[Bug 22052] New: DeleteExpiredOpacRegistrations should skip bad borrowers
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22052 Bug ID: 22052 Summary: DeleteExpiredOpacRegistrations should skip bad borrowers Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Target Milestone: --- If this job hits a patron with a checkout/fine it just dies To recreate: 1 - Add some self reg patrons 2 - checkout some items to one 3 - fine another 4 - backdate their registration dates (in the db) 5 - run this job 6 - note the error and fail -- 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=22052 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- 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=22052 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 83545 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83545&action=edit Bug 22052: Unit tests -- 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=22052 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 83546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83546&action=edit Bug 22052: Refactor DeleteExpiredOpacReigstrations This patch changes the sub to use Koha::Objects and updates the tests. Previously the sub would die on borrowers with checkouts and would delete borrowers if they had fines To test: 1 - prove -v t/db_dependent/ 2 - Set your selfreg preferences: PatronSelfRegistration: Allow PatronSelfRegistrationDefaultCategory: SELFREG (or of your choice) PatronSelfRegistrationExpireTemporaryAccountsDelay: 30 3 - Register a patron into SELFREG or how you set above 4 - Set their date enrolled to two months ago 5 - Checkout an item to the patron 6 - Issue a fine to that patron 7 - perl misc/cronjobs/cleanup_database.pl --del-exp-selfreg -v 8 - The job should die with an error 9 - Check in the item 10 - run the corn again - patron is deleted, oops 11 - Apply patch 12 - Create another patron in the same way 13 - Checkout and fine the patron 14 - run the cron 15 - they are not deleted, and no error 16 - checkin the item 17 - run the cron 18 - they are not deleted and no error 19 - clear the fine 20 - run the cron 21 - patron is deleted, huzzah -- 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=22052 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21065 -- 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=22052 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83545|0 |1 is obsolete| | --- Comment #3 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83660&action=edit Bug 22052: Unit tests Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- 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=22052 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83660|0 |1 is obsolete| | --- Comment #4 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83661&action=edit Bug 22052: Unit tests Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- 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=22052 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83546|0 |1 is obsolete| | --- Comment #5 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83662&action=edit Bug 22052: Refactor DeleteExpiredOpacReigstrations This patch changes the sub to use Koha::Objects and updates the tests. Previously the sub would die on borrowers with checkouts and would delete borrowers if they had fines To test: 1 - prove -v t/db_dependent/ 2 - Set your selfreg preferences: PatronSelfRegistration: Allow PatronSelfRegistrationDefaultCategory: SELFREG (or of your choice) PatronSelfRegistrationExpireTemporaryAccountsDelay: 30 3 - Register a patron into SELFREG or how you set above 4 - Set their date enrolled to two months ago 5 - Checkout an item to the patron 6 - Issue a fine to that patron 7 - perl misc/cronjobs/cleanup_database.pl --del-exp-selfreg -v 8 - The job should die with an error 9 - Check in the item 10 - run the corn again - patron is deleted, oops 11 - Apply patch 12 - Create another patron in the same way 13 - Checkout and fine the patron 14 - run the cron 15 - they are not deleted, and no error 16 - checkin the item 17 - run the cron 18 - they are not deleted and no error 19 - clear the fine 20 - run the cron 21 - patron is deleted, huzzah Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- 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=22052 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |charles.farmer@inlibro.com Status|Needs Signoff |Signed Off --- Comment #6 from Charles Farmer <charles.farmer@inlibro.com> --- Followed test plan, got the "DBD::mysql::st execute failed: Cannot delete or update a parent row..." has expected. Patch did what it advertised: self-reg'd borrower couldn't be deleted unless it had no checked out items, no fines, and all this without crashing the cron in the mean time. Signed off. -- 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=22052 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.arnaud@biblibre.com QA Contact|testopia@bugs.koha-communit |alex.arnaud@biblibre.com |y.org | -- 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=22052 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83661|0 |1 is obsolete| | --- Comment #7 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 83678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83678&action=edit Bug 22052: Unit tests Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> -- 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=22052 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83662|0 |1 is obsolete| | --- Comment #8 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 83679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83679&action=edit Bug 22052: Refactor DeleteExpiredOpacReigstrations This patch changes the sub to use Koha::Objects and updates the tests. Previously the sub would die on borrowers with checkouts and would delete borrowers if they had fines To test: 1 - prove -v t/db_dependent/ 2 - Set your selfreg preferences: PatronSelfRegistration: Allow PatronSelfRegistrationDefaultCategory: SELFREG (or of your choice) PatronSelfRegistrationExpireTemporaryAccountsDelay: 30 3 - Register a patron into SELFREG or how you set above 4 - Set their date enrolled to two months ago 5 - Checkout an item to the patron 6 - Issue a fine to that patron 7 - perl misc/cronjobs/cleanup_database.pl --del-exp-selfreg -v 8 - The job should die with an error 9 - Check in the item 10 - run the corn again - patron is deleted, oops 11 - Apply patch 12 - Create another patron in the same way 13 - Checkout and fine the patron 14 - run the cron 15 - they are not deleted, and no error 16 - checkin the item 17 - run the cron 18 - they are not deleted and no error 19 - clear the fine 20 - run the cron 21 - patron is deleted, huzzah Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> -- 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=22052 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- 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=22052 --- Comment #9 from Alex Arnaud <alex.arnaud@biblibre.com> --- Patch does the job. Test are ok. Code review is ok. So passed QA. However (and that doesn't put these patches in discussion), i'm wondering if should not move the self registered patrons in the deletedborrower table before deleting. Does that make sense to create an other bug ? -- 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=22052 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Alex Arnaud from comment #9)
Patch does the job. Test are ok. Code review is ok. So passed QA.
However (and that doesn't put these patches in discussion), i'm wondering if should not move the self registered patrons in the deletedborrower table before deleting. Does that make sense to create an other bug ?
Bug 22078 -- 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=22052 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- 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=22052 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=22052 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22052 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jesse@bywatersolutions.com --- Comment #13 from Jesse Maseto <jesse@bywatersolutions.com> --- Pushed to 18.05.x for 18.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22052 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #14 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.11.x for 17.11.14 Thanks a lot for the unit test, it makes life sweeter ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22052 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org