[Bug 25683] New: update_patron_categories.pl should recognize no fine history = 0 outstanding fines
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Bug ID: 25683 Summary: update_patron_categories.pl should recognize no fine history = 0 outstanding fines Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz Bug 17168 introduced update_patron_categories.pl, which can change patron categories based on fine totals. But it doesn't understand that an absence of fines is the same as $0 owed. To replicate: - Have a patron with nothing in accountlines - run update_patron_categories to find patrons with fines under $5 (-fu=5) - Your patron is not found - Give your patron a manual charge of $1 - rerun the cron, your patron is found - pay off your patron's fine, putting their balance at $0 - rerun the cron, your patron is found -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17168 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17168 [Bug 17168] Add a command line script for updating patron category based on status -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105620&action=edit Bug 25683: (bug 17168 follow-up) Don't consider negative balance Test plan: - Have a patron with nothing in accountlines - run update_patron_categories to find patrons with fines under $5 (-fu=5) - Your patron is not found - Give your patron a manual charge of $1 - rerun the cron, your patron is found - pay off your patron's fine, putting their balance at $0 - rerun the cron, your patron is not found -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org Status|NEW |In Discussion CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Isn't it actually a hidden feature to search for negative balance (credit)? If not you can test this patch! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 105660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105660&action=edit Bug 25683: [alternate] Patron with no accountlines should have 0 outstanding (not NULL) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #2)
Isn't it actually a hidden feature to search for negative balance (credit)?
If not you can test this patch!
I think the issue is that sum(amountoutstanding) is 'NULL' for patrons with no account history, it should be treated as 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105660|0 |1 is obsolete| | --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 105662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105662&action=edit Bug 25683: [alternate] Patron with no accountlines should have 0 outstanding (not NULL) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Nick, both changes are valid I think. My patch is fixing what is describe in comment 0. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #7 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- I don't seem to have explained the issue well up front. Nick's patch addresses what I was looking at. Jonathan, I don't entirely follow what your patch is doing or why. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- There is a difference between no accountlines and sum(accountlines)=0. If I am dealing with the situation you described in comment 0: A patron had fines, but paied all of them: their balance is 0. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #8)
There is a difference between no accountlines and sum(accountlines)=0.
If I am dealing with the situation you described in comment 0: A patron had fines, but paied all of them: their balance is 0.
So your patch makes it be that a patron with 0 fines will not be returned if only a fine max is supplied? That is, it adds an assumption that you only want patrons with any fines if you supply a fine under amount? I do not think this is desired behaviour. I think the final step in comment 0 was a correct return and step 3 was the error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105620|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|jonathan.druart@bugs.koha-c |nick@bywatersolutions.com |ommunity.org | --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I obsoleted my patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #11 from David Nind <david@davidnind.com> --- I'm a bit confused with this one, sorry! Using J patron category and Bobbie Middleton in koha-testing-docker - no transactions to start with. Before applying the patch, results are: perl update_patrons_category.pl -f J -t J -fu=5 -v No actions will be taken (test mode) Will update patrons from J to J with conditions below (if any) Conditions: Total fines less than 5 0 found, 0 updated After adding a charge of $1.00: perl update_patrons_category.pl -f J -t J -fu=5 -v No actions will be taken (test mode) Will update patrons from J to J with conditions below (if any) Conditions: Total fines less than 5 WOULD HAVE Updated Bobbie Middleton from J to J 1 found, 0 updated After paying off charge (balance is $0, two transactions with charge and payment) - two lines appear for patron, even though the their balance is $0: perl update_patrons_category.pl -f J -t J -fu=5 -v No actions will be taken (test mode) Will update patrons from J to J with conditions below (if any) Conditions: Total fines less than 5 WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Bobbie Middleton from J to J 1 found, 0 updated After applying the patch: perl update_patrons_category.pl -f J -t J -fu=5 -v No actions will be taken (test mode) Will update patrons from J to J with conditions below (if any) Conditions: Total fines less than 5 WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Gordon Peterson from J to J WOULD HAVE Updated Mae Kinney from J to J 3 found, 0 updated After applying the patch and adding another charge - Bobbie Middleton appears three times: perl update_patrons_category.pl -f J -t J -fu=5 -v No actions will be taken (test mode) Will update patrons from J to J with conditions below (if any) Conditions: Total fines less than 5 WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Bobbie Middleton from J to J WOULD HAVE Updated Gordon Peterson from J to J WOULD HAVE Updated Mae Kinney from J to J 3 found, 0 updated Every transaction added increases the number of times they appear. If the above is what is expected then I'm happy to sign off! ==> Outcome after applying the patch is that all patrons for the category with charges under 5 should appear, even if they have a 0 balance -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 105690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105690&action=edit Bug 25683: Fix grouping of results DBIX doesn't seem to work as I expected - if trying to group by we can only get the columns we grouped by, otherwise the queries are much more complicated and come out wrong This patch removes the 'amountoutstanding' from the query. Note that we do return patron object, however, to access all the columns we must discard_changes (or refetch) Also fixes a typo 'verbosse' To test: 1 - charge a $1 fine to a patron 2 - pay off the fine 3 - perl misc/cronjobs/update_patrons_category.pl -f J -t J -fu=5 -v 4 - Note the patron is returned twice 5 - export DBIC_TRACE=1 6 - repeat 3 - view the SQL query and see how odd it is 7 - Apply this patch 8 - repeat 3 - simpler query 9 - patron returned only once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105662|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 105704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105704&action=edit Bug 25683: [alternate] Patron with no accountlines should have 0 outstanding (not NULL) Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105690|0 |1 is obsolete| | --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 105705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105705&action=edit Bug 25683: Fix grouping of results DBIX doesn't seem to work as I expected - if trying to group by we can only get the columns we grouped by, otherwise the queries are much more complicated and come out wrong This patch removes the 'amountoutstanding' from the query. Note that we do return patron object, however, to access all the columns we must discard_changes (or refetch) Also fixes a typo 'verbosse' To test: 1 - charge a $1 fine to a patron 2 - pay off the fine 3 - perl misc/cronjobs/update_patrons_category.pl -f J -t J -fu=5 -v 4 - Note the patron is returned twice 5 - export DBIC_TRACE=1 6 - repeat 3 - view the SQL query and see how odd it is 7 - Apply this patch 8 - repeat 3 - simpler query 9 - patron returned only once Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can you please check the QA fail? FAIL Koha/Patrons.pm OK critic FAIL forbidden patterns forbidden pattern: IFNULL (must be replaced by COALESCE) (line 402) forbidden pattern: IFNULL (must be replaced by COALESCE) (line 403) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 107792 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107792&action=edit Bug 25683: (follow-up) Use COALESCE instead of IFNULL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 107850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107850&action=edit Bug 25683: Patron with no accountlines should have 0 outstanding (not NULL) Test plan: - Have a patron with nothing in accountlines - run update_patron_categories to find patrons with fines under $5 (-fu=5) - Your patron is not found - Give your patron a manual charge of $1 - rerun the cron, your patron is found - pay off your patron's fine, putting their balance at $0 - rerun the cron, your patron is found Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105705|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 107851 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107851&action=edit Bug 25683: Fix grouping of results DBIX doesn't seem to work as I expected - if trying to group by we can only get the columns we grouped by, otherwise the queries are much more complicated and come out wrong This patch removes the 'amountoutstanding' from the query. Note that we do return patron object, however, to access all the columns we must discard_changes (or refetch) Also fixes a typo 'verbosse' To test: 1 - charge a $1 fine to a patron 2 - pay off the fine 3 - perl misc/cronjobs/update_patrons_category.pl -f J -t J -fu=5 -v 4 - Note the patron is returned twice 5 - export DBIC_TRACE=1 6 - repeat 3 - view the SQL query and see how odd it is 7 - Apply this patch 8 - repeat 3 - simpler query 9 - patron returned only once Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107792|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 107852 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107852&action=edit Bug 25683: (follow-up) Use COALESCE instead of IFNULL Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Attachment #105704|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Works as expected, no regressions found, passes qa scripts. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00 |20.11.00, 20.05.03 released in| | Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #22 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x for 20.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00, 20.05.03 |20.11.00, 20.05.03, released in| |19.11.09 Status|Pushed to stable |Pushed to oldstable CC| |aleisha@catalyst.net.nz --- Comment #23 from Aleisha Amohia <aleisha@catalyst.net.nz> --- backported to 19.11.x for 19.11.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25683 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #24 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Missing dependencies for 19.05.x, can't backport. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org