[Bug 11352] New: Batch Patron Deletion/Anonmyzation deletes more than warning states it will delete
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Bug ID: 11352 Summary: Batch Patron Deletion/Anonmyzation deletes more than warning states it will delete Change sponsored?: --- Product: Koha Version: 3.12 Hardware: All OS: All Status: NEW Severity: major Priority: P3 Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: danielle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Created attachment 23346 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23346&action=edit Warning 1 Tested in versions 3.12.6, 3.14 and 3.15 (Bywater demo) When deleting by dateexpiry in batch deletion tool. The warning states a low number, and then when it comes back with a total of deletion, it deletes thousands more than it give a warning for. For instance it warned it would delete 102, the deleted 3661 in one system. We can't get the accurate count it is deleting patron's on either way. When queried with the same parameters (dateexpiry) the number is different. To recreate. Go to Batch Patron/Anonymzation Tool Select a dateexpiry date Warning should come up with count to delete Select to Permanently delete (not put in trash) Count of deleted items will show differently than warning-by thousands. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #1 from Chris Cormack <chris@bigballofwax.co.nz> --- Ouch sounds nasty, So the deleted count is actually correct? IE if you look in the db, it did actually delete that many? And just the warning count is incorrect? Is it deleting ones it shouldn't? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #2 from Danielle Elder <danielle@bywatersolutions.com> --- Yes, it is deleting ones it shouldn't and we couldn't get any of the numbers to show up: warning, count, or on the database. We even queried with the deletion parameters before deletion and could not get numbers to match up. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #3 from Joy Nelson <joy@bywatersolutions.com> --- To add insult to injury, this tool in some instances will completely delete the patrons and not move them to deleted_borrowers table. It just nukes the borrowers completely. (deleted permanently is the preselected option) (In reply to Danielle Elder from comment #2)
Yes, it is deleting ones it shouldn't and we couldn't get any of the numbers to show up: warning, count, or on the database. We even queried with the deletion parameters before deletion and could not get numbers to match up.
-- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11642 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Joy Nelson from comment #3)
To add insult to injury, this tool in some instances will completely delete the patrons and not move them to deleted_borrowers table. It just nukes the borrowers completely. (deleted permanently is the preselected option)
Remedied by bug 11642 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl --- Comment #5 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Chris Cormack from comment #1)
Is it deleting ones it shouldn't?
Yes; I can replicate this as well. It will delete (or move to trash) ones it shouldn't. Evidently there is a problem with how given parameter borrower_dateexpiry is (not) being properly converted when passed from step2 to step3 in tools/cleanborrowers.pl - somehow this parameter value gets mangled in the process. In step2 this script does: $filterdate1 = format_date_in_iso( $params->{'filterdate1'} ); $filterdate2 = format_date_in_iso( $params->{'filterdate2'} ); $borrower_dateexpiry = format_date_in_iso( $params->{'borrower_dateexpiry'} ); ... $template->param( filterdate1 => format_date($filterdate1), filterdate2 => format_date($filterdate2), borrower_dateexpiry => $borrower_dateexpiry, Note there is no format_date() call on $borrower_dateexpiry in line #100.. Changing line 100 to borrower_dateexpiry => format_date($borrower_dateexpiry), seems to fix this issue (at least for me). But: my comprehension of date handling in Koha is very far from perfect; I can't really guarantee this would be a proper fix. I guess the exact outcome may also depend on 'dateformat' setting in I18N/L10N preferences (we use dd/mm/yyyy; for yyyy-mm-dd unpatched script may actually work seemingly fine!). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #6 from Jacek Ablewicz <ablewicz@gmail.com> --- Created attachment 25594 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25594&action=edit Bug 11352 - Batch Patron Deletion/Anonmyzation deletes more than warning states it will delete Batch patron deletion/anonymization tool has some issues: 1) If 'dateformat' setting in I18N/L10N system preferences is set to anything other then 'iso' (eg. 'metric'), bulk deletion when using 'expiration date is before' criterion is not working properly. Date entered in this field will be efectivelly ignored (or possibly sometimes wrongly interpreted as different date, in other format) on the final patron deletion stage. This may result in deleting (or moving to trash) more borrower records then intended. 2) Bulk/batch patron deletion should skip borrowers with nonzero account balance (ones with oustanding fines or credits) 3) This tool shouldn't offer to choose as deletion criterion those patron categories which have category_type set to 'S' (= staff patron categories) This patch fixes abovementioned problems. It also adds an option to "test run" patron batch deletion, and makes this option the default choice in "warning" stage. Test plan: - prepare test database with some patron records (at least 2, the more the better) set up in such a way that they will be vulnerable to issues 1 & 2 - confirm issues 1,2 - restore test database - apply patch - ensure issues 1 & 2 are no longer present - first by using new "test run" option: for #1, record counts in "warning" stage and "final" stage should be now the same; for #2, observe that patron records with nonzero balance are now excluded from deletion - redo the tests, this time choosing "delete permanently" and "move to trash" instead of "test run" - test #3 by changing "Category type" to "S" in some test patron categories - after that, those categories should no longer be choosable as deletion criteria. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch Assignee|koha-bugs@lists.koha-commun |abl@biblos.pk.edu.pl |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25594|0 |1 is obsolete| | --- Comment #7 from Magnus Enger <magnus@enger.priv.no> --- Created attachment 26047 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26047&action=edit Bug 11352 - Batch Patron Deletion/Anonmyzation deletes more than warning states it will delete Batch patron deletion/anonymization tool has some issues: 1) If 'dateformat' setting in I18N/L10N system preferences is set to anything other then 'iso' (eg. 'metric'), bulk deletion when using 'expiration date is before' criterion is not working properly. Date entered in this field will be efectivelly ignored (or possibly sometimes wrongly interpreted as different date, in other format) on the final patron deletion stage. This may result in deleting (or moving to trash) more borrower records then intended. 2) Bulk/batch patron deletion should skip borrowers with nonzero account balance (ones with oustanding fines or credits) 3) This tool shouldn't offer to choose as deletion criterion those patron categories which have category_type set to 'S' (= staff patron categories) This patch fixes abovementioned problems. It also adds an option to "test run" patron batch deletion, and makes this option the default choice in "warning" stage. Test plan: - prepare test database with some patron records (at least 2, the more the better) set up in such a way that they will be vulnerable to issues 1 & 2 - confirm issues 1,2 - restore test database - apply patch - ensure issues 1 & 2 are no longer present - first by using new "test run" option: for #1, record counts in "warning" stage and "final" stage should be now the same; for #2, observe that patron records with nonzero balance are now excluded from deletion - redo the tests, this time choosing "delete permanently" and "move to trash" instead of "test run" - test #3 by changing "Category type" to "S" in some test patron categories - after that, those categories should no longer be choosable as deletion criteria. Signed-off-by: Magnus Enger <digitalutvikling@gmail.com> Tested with dateformat = dd/mm/yyyy. I tested with two expired patrons, one with fines and one without. Before the patch a lot of unexpected patrons were deleted along with the expected ones. After applying the patch only the expired patron was deleted, not the one with fines. The test run and the "real" run reported correct numbers. The patch also makes sure no patron categories with category_type = S are suggested for batch deletion. Note: The ergonomics of the "Batch delete/anonymize" tool is hardly optimal, but this patch fixes a real, data-loosing bug, so let's deal with the ergonomics later. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |magnus@enger.priv.no -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m QA Contact|testopia@bugs.koha-communit |jonathan.druart@biblibre.co |y.org |m Severity|major |new feature -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|new feature |major -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26047|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26488 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26488&action=edit Bug 11352 - Batch Patron Deletion/Anonmyzation deletes more than warning states it will delete Batch patron deletion/anonymization tool has some issues: 1) If 'dateformat' setting in I18N/L10N system preferences is set to anything other then 'iso' (eg. 'metric'), bulk deletion when using 'expiration date is before' criterion is not working properly. Date entered in this field will be efectivelly ignored (or possibly sometimes wrongly interpreted as different date, in other format) on the final patron deletion stage. This may result in deleting (or moving to trash) more borrower records then intended. 2) Bulk/batch patron deletion should skip borrowers with nonzero account balance (ones with oustanding fines or credits) 3) This tool shouldn't offer to choose as deletion criterion those patron categories which have category_type set to 'S' (= staff patron categories) This patch fixes abovementioned problems. It also adds an option to "test run" patron batch deletion, and makes this option the default choice in "warning" stage. Test plan: - prepare test database with some patron records (at least 2, the more the better) set up in such a way that they will be vulnerable to issues 1 & 2 - confirm issues 1,2 - restore test database - apply patch - ensure issues 1 & 2 are no longer present - first by using new "test run" option: for #1, record counts in "warning" stage and "final" stage should be now the same; for #2, observe that patron records with nonzero balance are now excluded from deletion - redo the tests, this time choosing "delete permanently" and "move to trash" instead of "test run" - test #3 by changing "Category type" to "S" in some test patron categories - after that, those categories should no longer be choosable as deletion criteria. Signed-off-by: Magnus Enger <digitalutvikling@gmail.com> Tested with dateformat = dd/mm/yyyy. I tested with two expired patrons, one with fines and one without. Before the patch a lot of unexpected patrons were deleted along with the expected ones. After applying the patch only the expired patron was deleted, not the one with fines. The test run and the "real" run reported correct numbers. The patch also makes sure no patron categories with category_type = S are suggested for batch deletion. Note: The ergonomics of the "Batch delete/anonymize" tool is hardly optimal, but this patch fixes a real, data-loosing bug, so let's deal with the ergonomics later. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- I'm wondering if this one is not a critical one. I have a QA followup but I will open a new bug report for that in order not to slow down the push of this one. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11975 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- I created and submitted a patch for my QA comments, see bug 11975. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Version|3.12 |master Severity|major |critical -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11983 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #11 from Galen Charlton <gmcharlt@gmail.com> --- I've opened another follow-up bug (bug 11983) regarding the fact that this patron introduces a /second/ implementation of logic to exclude patrons that have non-zero fine-balances (the other is in the patron deletion cronjob) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #12 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Jacek! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m Version|master |3.12 Severity|critical |major --- Comment #13 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patch pushed to 3.14.x, will be in 3.14.5 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical --- Comment #14 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- I've changed importance by mistake -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.12 |3.14 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 --- Comment #15 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.12.x will be in 3.12.13 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11352 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=15398 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org