[Bug 25183] New: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Bug ID: 25183 Summary: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers 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: wizzyrea@gmail.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. -- 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=25183 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #1 from Liz Rea <wizzyrea@gmail.com> --- Maybe instead of deleting them wholly the first go, we put them in deleted borrowers for a time, and then purge them from deleted borrowers once a year? -- 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=25183 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It could be a privacy issue. If the patron never finished the registration process, there might be no legal base for being allowed to store their data. deleted_borrowers is difficult in that regard anyway, but you could argue that usually patrons in there have been properly registered before. Often that would mean they have signed something on their first library visit. We still have no Koha script to clean deleted_borrowers afaik. :( Long argument short... I'd like to keep the option to delete them right away instead of moving them. -- 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=25183 --- Comment #3 from Liz Rea <wizzyrea@gmail.com> --- That's fine by me, making it an option. :) -- 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=25183 Jessie Zairo <jzairo@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #4 from Joy Nelson <joy@bywatersolutions.com> --- The Koha tool (delete patrons) provides a way to send patrons to the trash or permanently delete. This would be a good option to include for self reg patrons Patron deletion options in Tools: Permanently delete these patrons Delete patrons directly from the database. Patron data will not be recoverable. Move these patrons to the trash Move patrons to the deleted patrons table. They can be deleted permanently by the cleanup_database script. -- 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=25183 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103603&action=edit Bug 25183: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. Test Plan: 1) Apply this patch 2) Try running cleanup_database.pl with the new -del-exp-selfreg-type parameters 'soft' and 'hard' 3) Param value 'soft' should move patrons to deletedborrowers 4) Param value 'hard' should follow existing behavior 5) Leave our -del-exp-selfreg-type, you should get the existing behavior -- 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=25183 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|NEW |Needs Signoff -- 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=25183 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@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=25183 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> --- What is the exact syntax for running it? I obviously forgot some detail because it keeps printing me the help again and again. Here are the things I tried: kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type=soft kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft --confirm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103603|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103784 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103784&action=edit Bug 25183: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. Test Plan: 1) Apply this patch 2) Try running cleanup_database.pl with the new -del-exp-selfreg-type parameters 'soft' and 'hard' 3) Param value 'soft' should move patrons to deletedborrowers 4) Param value 'hard' should follow existing behavior 5) Leave our -del-exp-selfreg-type, you should get the existing behavior -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Please try again! (In reply to Victor Grousset/tuxayo from comment #6)
What is the exact syntax for running it? I obviously forgot some detail because it keeps printing me the help again and again.
Here are the things I tried:
kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type=soft
kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft
kohadev-koha@54d1fbaadc5c:/kohadevbox/koha$ misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft --confirm
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 --- Comment #9 from Victor Grousset/tuxayo <victor@tuxayo.net> --- - created a self registration - I see it with - select * from borrowers order by borrowernumber DESC limit 1\G - Applied the patch - ./cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type=soft - woops, wrong syntax, got the help printed - ./cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft - it seems it did something! - self registration not here anymore - select * from borrowers order by borrowernumber DESC limit 1\G - it's not it deletedborrowers despite "soft" - select * from deletedborrowers order by borrowernumber DESC limit 1\G So, test failed. Hypothesis: I didn't get the param syntax right and the it's falling back to the default "del-exp-selfreg-type" value which is "hard" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103784|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 119019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119019&action=edit Bug 25183: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. Test Plan: 1) Apply this patch 2) Try running cleanup_database.pl with the new -del-exp-selfreg-type parameters 'soft' and 'hard' 3) Param value 'soft' should move patrons to deletedborrowers 4) Param value 'hard' should follow existing behavior 5) Leave our -del-exp-selfreg-type, you should get the existing behavior 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=25183 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Failed QA |Signed Off --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Worked as expected in testing for me.. hope the QA person can work out where Victor may have gone wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 --- Comment #12 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
Worked as expected in testing for me
Is the following the right syntax? ./cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type soft -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |julian.maurice@biblibre.com --- Comment #13 from Julian Maurice <julian.maurice@biblibre.com> --- QA comments: 1. This patch adds a parameter to subroutine DeleteExpiredOpacRegistrations. This new parameter should be tested. 2. The new constant DEFAULT_EXP_SELFREG_TYPE is not used 3. The script parameter --del-exp-selfreg-type is not correctly defined. As it is, it takes no value: % misc/cronjobs/cleanup_database.pl --del-exp-selfreg --del-exp-selfreg-type=soft Option del-exp-selfreg-type does not take an argument -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 --- Comment #14 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Martin Renvoize from comment #11)
Worked as expected in testing for me.. hope the QA person can work out where Victor may have gone wrong.
I don't know how that could work for you. Could that be a difference in Getopt::Long versions ? I'm using 2.5 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 --- Comment #15 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Julian Maurice from comment #14)
I'm using 2.5
I meant 2.50 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Kyle M Hall <kyle@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=25183 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 119593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119593&action=edit Bug 25183: Add option to cleanup-database.pl to move expired self registrations to deleted borrowers table The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. Test Plan: 1) Apply this patch 2) Try running cleanup_database.pl with the new -del-exp-selfreg-type parameters 'soft' and 'hard' 3) Param value 'soft' should move patrons to deletedborrowers 4) Param value 'hard' should follow existing behavior 5) Leave our -del-exp-selfreg-type, you should get the existing behavior 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=25183 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 119594 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119594&action=edit Bug 25183: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119019|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=25183 --- Comment #18 from Julian Maurice <julian.maurice@biblibre.com> --- Comment on attachment 119593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119593 Bug 25183: Add option to cleanup-database.pl to move expired self registrations to deleted borrowers table Review of attachment 119593: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=25183&attachment=119593) ----------------------------------------------------------------- ::: misc/cronjobs/cleanup_database.pl @@ +27,4 @@
use constant DEFAULT_SEARCHHISTORY_PURGEDAYS => 30; use constant DEFAULT_SHARE_INVITATION_EXPIRY_DAYS => 14; use constant DEFAULT_DEBARMENTS_PURGEDAYS => 30; +use constant DEFAULT_EXP_SELFREG_TYPE => 'hard';
This is not used anywhere @@ +167,4 @@
'restrictions:i' => \$pDebarments, 'all-restrictions' => \$allDebarments, 'del-exp-selfreg' => \$pExpSelfReg, + 'del-exp-selfreg-type:s' => \$pExpSelfRegType,
':s' means the value is optional. In this case the value should be mandatory. Use '=s' instead @@ +229,4 @@
|| $pPseudoTransactionsTo || $pMessages || defined $lock_days && $lock_days ne q{} + || $pExpSelfRegType !~ m/^hard|soft$/
I guess this line is supposed to avoid giving wrong value for the parameter, but it doesn't work. I was able to give --del-exp-selfreg-type foo without the script complaining -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25183 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org