[Bug 28943] New: cleanup_database: Can we make del-exp-selfreg less disastrous ?
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Bug ID: 28943 Summary: cleanup_database: Can we make del-exp-selfreg less disastrous ? Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz See the discussion on bug 27395. If you do not want to change the patron category for self registrations, you should not use this flag. But can we do something to prevent accidents which seemed to have happened already? -- 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=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27395 -- 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=28943 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- My first idea (biased a bit by what we do ourselves..): If you have enabled Verification by mail and you would set a new pref like PreventAccidentalRemoval [needs a better name], then cleanup_database will not remove patrons enrolled in category X before certain date. -- 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=28943 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #1)
My first idea (biased a bit by what we do ourselves..):
If you have enabled Verification by mail and you would set a new pref like PreventAccidentalRemoval [needs a better name], then cleanup_database will not remove patrons enrolled in category X before certain date.
This comes down to moving the cmd line flag to a preference. I have seen that transition happen much more already. -- 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=28943 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, we already have PatronSelfRegistrationExpireTemporaryAccountsDelay for the number of days. It seems like a double up to introduce an additional pref to secure another one. Maybe we need to make sure that this one can also be used to deactivate? -- 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=28943 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #3)
Hm, we already have PatronSelfRegistrationExpireTemporaryAccountsDelay for the number of days.
It seems like a double up to introduce an additional pref to secure another one. Maybe we need to make sure that this one can also be used to deactivate?
my $delay = C4::Context->preference('PatronSelfRegistrationExpireTemporaryAccountsDelay'); return 0 if not $category_code or not defined $delay or $delay eq q||; -- 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=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=28943 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124579&action=edit Bug 28943: Lower the risk of accidental patron deletion If you do not use a temporary self registration patron category, you should actually make the preference PatronSelfRegistrationExpireTemporaryAccountsDelay empty. This patch adds a comment in the subroutine that does the deletion to not remove the test on empty delay. It also adds a test in Members.t for the same reason. It corrects the wrong explanation and default value in the sysprefs.sql file. Also it adds further explanation to both related sysprefs. Test plan: Run t/db_dependent/Members. Check in Administration the two adjusted OPAC pref descriptions. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- 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=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- 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=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124579|0 |1 is obsolete| | --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124580 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124580&action=edit Bug 28943: Lower the risk of accidental patron deletion If you do not use a temporary self registration patron category, you should actually make the preference PatronSelfRegistrationExpireTemporaryAccountsDelay empty. This patch adds a comment in the subroutine that does the deletion to not remove the test on empty delay. It also adds a test in Members.t for the same reason. It corrects the wrong explanation and default value in the sysprefs.sql file. Also it adds further explanation to both related sysprefs. Test plan: Run t/db_dependent/Members.t Check in Administration the two adjusted OPAC pref descriptions. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|cleanup_database: Can we |The del-exp-selfreg flag in |make del-exp-selfreg less |cleanup_database can be |disastrous ? |really disastrous -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- NOTE: I have intentionally set the importance to Normal since I do not only consider this as an enhancement. See the other bug. It refers to a case where accidentally patrons were deleted. Note that correcting the default of the syspref here may prevent accidents in new libraries. And the adjusted pref descriptions may help too. Could we do more? Probably, but not per se on this report. I was thinking about some limit to be used in the delete sub to not inadvertently remove a large number of patrons. If you agree, you are invited to open up a new report and add me in CC ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- For discussion: Do we need a preference that indicates that a library uses a temporary patron category for self registration or not? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #8)
For discussion: Do we need a preference that indicates that a library uses a temporary patron category for self registration or not?
Or should that be a flag on patron categories ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |If you use self release notes| |registration but you do not | |use a temporary self | |registration patron | |category, | |you should | |actually make the | |preference | |PatronSelfRegistrationExpir | |eTemporaryAccountsDelay | |EMPTY (and do not set it to | |zero). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #10 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- "A zero will remove all patrons; but if you make the pref EMPTY, it will not delete any patron." This sentence needs to be reworked for specificity and to keep the style consistent with other similar sysprefs. I'd recommend: "Leave this field empty if you don't want to activate this feature. A zero in this system preference will delete all patrons in the associated category." I'd be inclined not to add new wording to PatronSelfRegistrationDefaultCategory as well, that seems a little redundant. If you maintain a new message there, it should also be updated to not use all caps for "UNLESS" and "EMPTY." CCing Katrin on this for wording and style guidance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Reconsidering and looking at wording Thx Andrew -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|The del-exp-selfreg flag in |Lower the risk of |cleanup_database can be |accidental patron deletion |really disastrous |by cleanup_database.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124580|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 126038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126038&action=edit Bug 28943: Lower the risk of accidental patron deletion If you do not use a temporary self registration patron category, you should actually make the preference PatronSelfRegistrationExpireTemporaryAccountsDelay empty. As the comment in sysprefs.sql already said, we should not let a zero value in the pref delete patrons too. The module is changed now, the test adjusted and the description of both related sysprefs modified. Test plan: Run t/db_dependent/Members.t Check in Administration the two adjusted OPAC pref descriptions. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|If you use self |If you use self release notes|registration but you do not |registration but you do not |use a temporary self |use a temporary self |registration patron |registration patron |category, |category, |you should |you should |actually make the |actually clear the |preference |preference |PatronSelfRegistrationExpir |PatronSelfRegistrationExpir |eTemporaryAccountsDelay |eTemporaryAccountsDelay. |EMPTY (and do not set it to | |zero). | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 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=28943 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126038|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 128652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128652&action=edit Bug 28943: Lower the risk of accidental patron deletion If you do not use a temporary self registration patron category, you should actually make the preference PatronSelfRegistrationExpireTemporaryAccountsDelay empty. As the comment in sysprefs.sql already said, we should not let a zero value in the pref delete patrons too. The module is changed now, the test adjusted and the description of both related sysprefs modified. Test plan: Run t/db_dependent/Members.t Check in Administration the two adjusted OPAC pref descriptions. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=28943 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Isn't supporting "0" actually a feature? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #14)
Isn't supporting "0" actually a feature?
Hmm. Theoretically perhaps? I would have said No in absolute sense, were it not for the code part stripping the time by ->ymd. So yes, I you run with 0 at e.g. 23:00, you only remove things from yesterday (still a potential disaster); which might be what you want when using the temp category. If you run at 01:00, you remove everything except the last hour; which might not be what you like (too quick). But I think that the risk of deleting almost all patrons (no temp category) outweighs the gain of running 0 at 23:00 with temp category. So, in short: No we should not support 0 as a feature. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128652|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 129593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129593&action=edit Bug 28943: Lower the risk of accidental patron deletion If you do not use a temporary self registration patron category, you should actually make the preference PatronSelfRegistrationExpireTemporaryAccountsDelay empty. As the comment in sysprefs.sql already said, we should not let a zero value in the pref delete patrons too. The module is changed now, the test adjusted and the description of both related sysprefs modified. Test plan: Run t/db_dependent/Members.t Check in Administration the two adjusted OPAC pref descriptions. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=28943 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think Marcle argues the point well and I'm inclined to agree.. As such, I've worked through and tested this. Everyone works as expected and the QA scripts and tests are all happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Jonathan Druart from comment #14)
Isn't supporting "0" actually a feature?
Looks like when feature was added, zero was for disable. Also since zero is default value, it makes sens to be a not dangerous value.
From sysprefs.sql : ('PatronSelfRegistrationExpireTemporaryAccountsDelay','0',NULL,'If PatronSelfRegistrationDefaultCategory is enabled, this system preference controls how long a patron can have a temporary status before the account is deleted automatically. It is an integer value representing a number of days to wait before deleting a temporary patron account. Setting it to 0 disables the deleting of temporary accounts.','Integer'),
Looks like the testing of the preference has been added by : https://git.koha-community.org/Koha-community/Koha/commit/fbc4e94d7f389d6bac... So I agree with this change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Oh I see now it was already said in test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #20 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 129711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129711&action=edit Bug 28943: (QA follow-up) Fix opac.pref Fix YAML syntax of opac.pref, we must use dash for new lines. Also removes duplicate text before syspref. No need to say "code" since it is now a list of categories. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Command-line Utilities |Patrons CC| |gmcharlt@gmail.com, | |kyle.m.hall@gmail.com Severity|normal |major --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- I move to component "Patrons", makes more sens -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 --- Comment #22 from Fridolin Somers <fridolin.somers@biblibre.com> --- And rise to major since one can delete a lot of patrons -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27564 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.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=28943 --- Comment #23 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, 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=28943 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Keywords| |Manual -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable Version(s)|22.05.00 |22.05.00,21.11.03 released in| | --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|22.05.00,21.11.03 |22.05.00,21.11.03,21.05.14 released in| | --- Comment #25 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.14 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00,21.11.03,21.05.14 |22.05.00,21.11.03,21.05.14, released in| |20.11.18 Status|Pushed to oldstable |Pushed to oldoldstable CC| |victor@tuxayo.net --- Comment #26 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported: Pushed to 20.11.x branch for 20.11.18 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldstable |Pushed to oldoldoldstable CC| |chris@bigballofwax.co.nz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28943 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|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=28943 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldoldoldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org