| Summary: | Patron userid field can be overwritten by update_patron_categories when limiting by fines | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> | 
| Component: | Command-line Utilities | Assignee: | Nick Clemens (kidclamp) <nick> | 
| Status: | CLOSED FIXED | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> | 
| Severity: | critical | ||
| Priority: | P5 - low | CC: | fridolin.somers, lucas, robin, tomascohen | 
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 | 
| Patch complexity: | Small patch | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: | ||
| Version(s) released in: | 24.05.00,23.11.06,23.05.12 | Circulation function: | |
| Attachments: | Bug 36508: Refresh patron object when updating category Bug 36508: Unit tests Bug 36508: Refresh patron object when updating category Bug 36508: Unit tests Bug 36508: Refresh patron object when updating category Bug 36508: Unit tests Bug 36508: Refresh patron object when updating category | ||
| Created attachment 164391 [details] [review] Bug 36508: Refresh patron object when updating category To test: 1 - Find a ptron, I sued #45 in KTD, note their category 2 - Update them from one category to another using finesunder: perl misc/cronjobs/update_patrons_category.pl -f ST -t S --finesunder=5.00 --where "me.borrowernumber=45" -v -c 3 - Check their modification log (I told you to enabled BorrowersLog, right?) 4 - See many fields reported changed 5 - Apply patch 6 - Repeat, but change the to and from options perl misc/cronjobs/update_patrons_category.pl -f S -t ST --finesunder=5.00 --where "me.borrowernumber=45" -v -c 7 - Note only one column changed in the logs 8 - Ask for unit tests Created attachment 165571 [details] [review] Bug 36508: Unit tests Created attachment 165572 [details] [review] Bug 36508: Refresh patron object when updating category To test: 1 - Find a ptron, I sued #45 in KTD, note their category 2 - Update them from one category to another using finesunder: perl misc/cronjobs/update_patrons_category.pl -f ST -t S --finesunder=5.00 --where "me.borrowernumber=45" -v -c 3 - Check their modification log (I told you to enabled BorrowersLog, right?) 4 - See many fields reported changed 5 - Apply patch 6 - Repeat, but change the to and from options perl misc/cronjobs/update_patrons_category.pl -f S -t ST --finesunder=5.00 --where "me.borrowernumber=45" -v -c 7 - Note only one column changed in the logs 8 - Ask for unit tests Created attachment 165574 [details] [review] Bug 36508: Unit tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 165575 [details] [review] Bug 36508: Refresh patron object when updating category To test: 1 - Find a ptron, I sued #45 in KTD, note their category 2 - Update them from one category to another using finesunder: perl misc/cronjobs/update_patrons_category.pl -f ST -t S --finesunder=5.00 --where "me.borrowernumber=45" -v -c 3 - Check their modification log (I told you to enabled BorrowersLog, right?) 4 - See many fields reported changed 5 - Apply patch 6 - Repeat, but change the to and from options perl misc/cronjobs/update_patrons_category.pl -f S -t ST --finesunder=5.00 --where "me.borrowernumber=45" -v -c 7 - Note only one column changed in the logs 8 - Ask for unit tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 165784 [details] [review] Bug 36508: Unit tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 165785 [details] [review] Bug 36508: Refresh patron object when updating category To test: 1 - Find a ptron, I sued #45 in KTD, note their category 2 - Update them from one category to another using finesunder: perl misc/cronjobs/update_patrons_category.pl -f ST -t S --finesunder=5.00 --where "me.borrowernumber=45" -v -c 3 - Check their modification log (I told you to enabled BorrowersLog, right?) 4 - See many fields reported changed 5 - Apply patch 6 - Repeat, but change the to and from options perl misc/cronjobs/update_patrons_category.pl -f S -t ST --finesunder=5.00 --where "me.borrowernumber=45" -v -c 7 - Note only one column changed in the logs 8 - Ask for unit tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed for 24.05! Well done everyone, thank you! Pushed to 23.11.x for 23.11.06 Backported to 23.05.x for upcoming 23.05.12 | 
In Koha::Patrons->search_patrons_to_update_category we join to accountlines and sum when using the fines over and fines under parameters. The way this sql works requires a GROUP BY - which means we limit the columns to borrowernumber This means the object we use to update later loses many of the fields we expect to be there We end up with entries in modification log like: { "address" : { "after" : null, "before" : "5129 Library Rd." }, "auth_method" : { "after" : null, "before" : "password" }, "autorenew_checkouts" : { "after" : null, "before" : 1 }, "branchcode" : { "after" : null, "before" : "MPL" }, "cardnumber" : { "after" : null, "before" : "23529000050113" }, "categorycode" : { "after" : "S", "before" : "ST" }, "checkprevcheckout" : { "after" : null, "before" : "inherit" }, "city" : { "after" : null, "before" : "Centerville, CA" }, "dateenrolled" : { "after" : null, "before" : "1985-09-09" }, "dateexpiry" : { "after" : null, "before" : "2099-12-31" }, "dateofbirth" : { "after" : null, "before" : "1952-05-27" }, "firstname" : { "after" : null, "before" : "Jordan" }, "lang" : { "after" : null, "before" : "default" }, "phone" : { "after" : null, "before" : "(212) 555-1212" }, "privacy" : { "after" : null, "before" : 1 }, "sex" : { "after" : null, "before" : "M" }, "sort1" : { "after" : null, "before" : "0.54121395064631" }, "sort2" : { "after" : null, "before" : "0.90967674824379" }, "surname" : { "after" : null, "before" : "Alford" }, "userid" : { "after" : "1", "before" : "23529000050113" }, "zipcode" : { "after" : null, "before" : "44262" } } Most of these are ignored, null interpreting as not being updated, however, the cardnumber does overwrite as it is treated specially.