Bug 24532 - Some account types are converted to debits when they shouldn't be
Summary: Some account types are converted to debits when they shouldn't be
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Martin Renvoize (ashimema)
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on: 23049
Blocks: 29559
  Show dependency treegraph
 
Reported: 2020-01-29 14:55 UTC by Nick Clemens (kidclamp)
Modified: 2021-12-03 00:29 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.04
Circulation function:


Attachments
Bug 24532: Fix original DB update for bug 23049 (1.21 KB, patch)
2020-01-30 14:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 24532: Fix misidentified credit_types from bug 23049 (1.73 KB, patch)
2020-01-30 14:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 24532: Fix original DB update for bug 23049 (1.28 KB, patch)
2020-02-13 11:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24532: Fix misidentified credit_types from bug 23049 (1.80 KB, patch)
2020-02-13 11:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2020-01-29 14:55:56 UTC
Bug 23049 converts some credit types to debit types

The logic seems to be:
1 - Get all unique types where the accountline is a debit
2 - Convert all lines with these types to have debit codes

But in step 1 if we have any type 'Pay' or 'W' for example, where the amount is greater than 0 (pathological credits which have debit amounts) then 'Pay' is added as a debit type and all 'Pay' lines get that as a debit code.
Comment 1 Martin Renvoize (ashimema) 2020-01-30 13:12:17 UTC
Hmm.. this is a hard one.

I can't easily see how 'credit' type accountlines have gotten their 'amounts' inverted in the first place and as such I'm not entirely confident in switching the sign on them.

Any thoughts on what we should do here..?
Comment 2 Martin Renvoize (ashimema) 2020-01-30 13:15:05 UTC
yikes.. what we certainly should do however is update that second step to ensure it only catches the weird cases rather than updating all matching codes!
Comment 3 Martin Renvoize (ashimema) 2020-01-30 14:40:22 UTC
Created attachment 98161 [details] [review]
Bug 24532: Fix original DB update for bug 23049

Pathalogical cases of Payments and Writeoffs being converted to debits
are identified during the DB update for bug 23049, but we are not
correctly limiting to those types accountlines when subsequently
populating the debit_type field.
Comment 4 Martin Renvoize (ashimema) 2020-01-30 14:40:25 UTC
Created attachment 98162 [details] [review]
Bug 24532: Fix misidentified credit_types from bug 23049
Comment 5 Kyle M Hall (khall) 2020-02-13 11:18:51 UTC
Created attachment 98811 [details] [review]
Bug 24532: Fix original DB update for bug 23049

Pathalogical cases of Payments and Writeoffs being converted to debits
are identified during the DB update for bug 23049, but we are not
correctly limiting to those types accountlines when subsequently
populating the debit_type field.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall (khall) 2020-02-13 11:18:59 UTC
Created attachment 98812 [details] [review]
Bug 24532: Fix misidentified credit_types from bug 23049

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2020-02-19 17:56:42 UTC
I read this one over, looked at code and looked at an affected site - this should resolve the issue and prevent further. I didn't test with actual data but feel confident this should do what is needed
Comment 8 Martin Renvoize (ashimema) 2020-02-20 08:37:44 UTC
I've tested on a series of sites here now and am confident in the change.. with Kyle and Nick having both taken a look too I think we're safe to PQA.
Comment 9 Martin Renvoize (ashimema) 2020-02-20 08:46:16 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 10 Joy Nelson 2020-03-13 01:12:27 UTC
Pushed to 19.11.x branch for 19.11.04
Comment 11 David Cook 2021-12-03 00:29:13 UTC
I wonder if we should rename this bug and open a new one with this name, since this technically is still an on-going problem...