[Bug 25429] New: Cleanup Database - remove resolved claims returned from db after X days
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Bug ID: 25429 Summary: Cleanup Database - remove resolved claims returned from db after X days 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: joy@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. -- 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=25429 Joy Nelson <joy@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=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #1 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Maybe put the number of days in a syspref, make the cron check the syspref when run? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Andrew Fuerste-Henry from comment #1)
Maybe put the number of days in a syspref, make the cron check the syspref when run?
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104611 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104611&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Test cleanup_database with the new --return-claims DAYS option -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104612 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104612&action=edit Bug 25429: Fix typos, correct spacing, add missing periods for script help -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, did you see comments #1 and #2? I think it would be really great if we could have this as a system preference - it would make things much easier with multiple instances on a server. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104798 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104798&action=edit Bug 25429: Implement system preference, remove command line switch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Daniel Gaghan <daniel.gaghan@pueblolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |daniel.gaghan@pueblolibrary | |.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #7 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Kyle, can you please rebase this? Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104611|0 |1 is obsolete| | Attachment #104612|0 |1 is obsolete| | Attachment #104798|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 114601 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114601&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Test cleanup_database with the new --return-claims DAYS option -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 114602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114602&action=edit Bug 25429: Implement system preference, remove command line switch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- After applying this patch, I get the error "unknown option: return-claims" when I run "perl misc/cronjobs/cleanup_database.pl --return-claims 40 --confirm" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114601|0 |1 is obsolete| | Attachment #114602|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 115042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115042&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #12 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Can we make the cron require a --claims flag or something comparable? It seems likely to create confusion down the line to have this cron perform an action based on a syspref value without some obvious indicator in the cron line itself. Additionally, as it is now I am unable to remove resolved claims without *also* doing something else with the cron. If I run this with just "--confirm," the cron just spits out the help text and wants me to try again with something actually set. Not that anyone's likely to be running this with no other flags, but it's a confusion easily avoided. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Andrew Fuerste-Henry from comment #12)
Can we make the cron require a --claims flag or something comparable? It seems likely to create confusion down the line to have this cron perform an action based on a syspref value without some obvious indicator in the cron line itself.
Additionally, as it is now I am unable to remove resolved claims without *also* doing something else with the cron. If I run this with just "--confirm," the cron just spits out the help text and wants me to try again with something actually set. Not that anyone's likely to be running this with no other flags, but it's a confusion easily avoided.
I don't understand the purpose of moving the value to a syspref if it still requires a cli flag to run. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115042|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 115086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115086&action=edit Bug 25429: Script should be able to run with only return claims -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115086|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 115087 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115087&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 115088 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115088&action=edit Bug 25429: Script should be able to run with only return claims -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.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=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115087|0 |1 is obsolete| | --- Comment #17 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 115089 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115089&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch Signed-off-by: Andrew Fuerste-Henry <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=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115088|0 |1 is obsolete| | --- Comment #18 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 115090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115090&action=edit Bug 25429: Script should be able to run with only return claims Signed-off-by: Andrew Fuerste-Henry <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=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115089|0 |1 is obsolete| | --- Comment #19 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 115907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115907&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115090|0 |1 is obsolete| | --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 115908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115908&action=edit Bug 25429: Script should be able to run with only return claims Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115907|0 |1 is obsolete| | --- Comment #21 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 118880 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118880&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115908|0 |1 is obsolete| | --- Comment #22 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 118881 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118881&action=edit Bug 25429: Script should be able to run with only return claims Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Signed Off |Passed QA --- Comment #23 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works, tested with various values for edge cases. Code looks good QA script ok Passed! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Passed QA |Failed QA --- Comment #24 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 1. Please use the same syntax as the other entries. You need to use filter_by_last_update (inherited from Koha::Objects) to get the object set. It will offer a better verbose and confirm support. See the "$pOldIssues" block for a good example. 2. Why don't we add an option to the script, like the other flags? I would have expected a default number of days picked from the pref, and a flag passed to the script. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #24)
2. Why don't we add an option to the script, like the other flags? I would have expected a default number of days picked from the pref, and a flag passed to the script.
ok, I read the previous comments now. But... isn't how the whole script work for the other flags so far? for instance: 190 $pDebarments = DEFAULT_DEBARMENTS_PURGEDAYS if defined($pDebarments) && $pDebarments == 0; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 119219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119219&action=edit Bug 25429: Use filter_by_last_update -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #25)
(In reply to Jonathan Druart from comment #24)
2. Why don't we add an option to the script, like the other flags? I would have expected a default number of days picked from the pref, and a flag passed to the script.
ok, I read the previous comments now. But... isn't how the whole script work for the other flags so far?
for instance: 190 $pDebarments = DEFAULT_DEBARMENTS_PURGEDAYS if defined($pDebarments) && $pDebarments == 0;
This is what I was asked to implement, so I did. Koha is on a trajectory of moving away from cli paramters for cronjobs to sysprefs so that librarians can control how cronjobs behave without the need to have a sysadmin make changes for them. I fully expect over time all the parameters for this script will be moved to sysprefs. This is just the first step. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Kyle M Hall from comment #27)
(In reply to Jonathan Druart from comment #25)
(In reply to Jonathan Druart from comment #24)
2. Why don't we add an option to the script, like the other flags? I would have expected a default number of days picked from the pref, and a flag passed to the script.
ok, I read the previous comments now. But... isn't how the whole script work for the other flags so far?
for instance: 190 $pDebarments = DEFAULT_DEBARMENTS_PURGEDAYS if defined($pDebarments) && $pDebarments == 0;
This is what I was asked to implement, so I did. Koha is on a trajectory of moving away from cli paramters for cronjobs to sysprefs so that librarians can control how cronjobs behave without the need to have a sysadmin make changes for them. I fully expect over time all the parameters for this script will be moved to sysprefs. This is just the first step.
Hi, when I commented on wanting this to be a pref, I was thinking of making it work like the PurgeSuggestionsOlderThan preference. In this case a comment line switch is there, but the number of days and activation for the instance is handled by the pref. I like this solution best, because: - As an admin, you can turn off all deletions easily by removing the switch from the cron, without every library having to change their configuration. This might be handy sometimes (think COVID...) - The libraries have the ability to turn the feature off indidvidually by leaving it empty and set it to the desirable number of days if they want deletions. I think this speaks against a default value of days and I expect that there is not a common default value anyway that would make sense to most libraries. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #29 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Adjusting status regarding the last comment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 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=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118880|0 |1 is obsolete| | Attachment #118881|0 |1 is obsolete| | Attachment #119219|0 |1 is obsolete| | --- Comment #30 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120927&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #31 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120928&action=edit Bug 25429: Script should be able to run with only return claims Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #32 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120929&action=edit Bug 25429: Use filter_by_last_update -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #33 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120930&action=edit Bug 25429: Require command line switch to enable, control DAYS via syspref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28457 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #34 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 120930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120930 Bug 25429: Require command line switch to enable, control DAYS via syspref Review of attachment 120930: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=25429&attachment=120930) ----------------------------------------------------------------- ::: misc/cronjobs/cleanup_database.pl @@ +532,4 @@
} }
+$return_claims = C4::Context->preference('CleanUpDatabaseReturnClaims') if $return_claims;
This doesn't make sense to me. It means that you need to have a sysadmin change a cronjob for ALL instances if there's just 1 instance that wants to be able to use the system preference "CleanUpDatabaseReturnClaims". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #35 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #34)
This doesn't make sense to me.
It means that you need to have a sysadmin change a cronjob for ALL instances if there's just 1 instance that wants to be able to use the system preference "CleanUpDatabaseReturnClaims".
To me, that means a custom crontab for vendors, which is not nice from a management perspective. And it means that people who rely on volunteers or shared sysadmins (or no sysadmins) can't get the functionality. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion --- Comment #36 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- David, basically you are asking for another switch in koha-conf that would be off by default, that's it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA --- Comment #37 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to David Cook from comment #35)
(In reply to David Cook from comment #34)
This doesn't make sense to me.
It means that you need to have a sysadmin change a cronjob for ALL instances if there's just 1 instance that wants to be able to use the system preference "CleanUpDatabaseReturnClaims".
To me, that means a custom crontab for vendors, which is not nice from a management perspective.
And it means that people who rely on volunteers or shared sysadmins (or no sysadmins) can't get the functionality.
(In reply to David Cook from comment #34)
Comment on attachment 120930 [details] [review] Bug 25429: Require command line switch to enable, control DAYS via syspref
Review of attachment 120930 [details] [review]: -----------------------------------------------------------------
::: misc/cronjobs/cleanup_database.pl @@ +532,4 @@
} }
+$return_claims = C4::Context->preference('CleanUpDatabaseReturnClaims') if $return_claims;
This doesn't make sense to me.
It means that you need to have a sysadmin change a cronjob for ALL instances if there's just 1 instance that wants to be able to use the system preference "CleanUpDatabaseReturnClaims".
You are incorrect. It means that an administrator can disable that behavior for all instances. However, if the option is enabled on the command line, the system preference then takes over as the control. If the syspref is empty, the feature is disabled. If the syspref has a value, the feature is enabled. This specific behavior was requested by a member of the QA team. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Guess that was me and I like the description of it - not sure I understand the problem pointed out by Jonathan. Maybe you can explain a bit more? We can add the switch to new installations by default and we can still deactivate/activate on demand. It would work like the self reg one now does too (point for consistency). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- (In reply to Kyle M Hall from comment #37)
You are incorrect. It means that an administrator can disable that behavior for all instances. However, if the option is enabled on the command line, the system preference then takes over as the control. If the syspref is empty, the feature is disabled. If the syspref has a value, the feature is enabled.
The patch literally says 'Bug 25429: Require command line switch to enable, control DAYS via syspref" and the code supports that. That means that a sysadmin must add the CLI switch in order for the syspref to be usable, and this switch is not enabled out of the box. Can you show me how I'm incorrect because I'm not seeing it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #40 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #38)
Guess that was me and I like the description of it - not sure I understand the problem pointed out by Jonathan. Maybe you can explain a bit more? We can add the switch to new installations by default and we can still deactivate/activate on demand. It would work like the self reg one now does too (point for consistency).
I think the self-reg is a suboptimal implementation, which is why I've opened bug 28457. We shouldn't require sysadmins to change crontabs in order to get cronjobs to work using system preferences. It defeats the whole point of Bug 24968. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #41 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #36)
David, basically you are asking for another switch in koha-conf that would be off by default, that's it?
I'm not sure what you mean by "asking for another switch in koha-conf". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #42 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to David Cook from comment #40)
(In reply to Katrin Fischer from comment #38)
Guess that was me and I like the description of it - not sure I understand the problem pointed out by Jonathan. Maybe you can explain a bit more? We can add the switch to new installations by default and we can still deactivate/activate on demand. It would work like the self reg one now does too (point for consistency).
I think the self-reg is a suboptimal implementation, which is why I've opened bug 28457.
We shouldn't require sysadmins to change crontabs in order to get cronjobs to work using system preferences.
It defeats the whole point of Bug 24968.
Ok, I am not going to insist on the switch any longer. I've also added the topic to the dev meeting agenda a few days ago, because I think we need to reach a decision that will unlock the 2 waiting bugs and more to come for good. But... from our own experience: especially the self reg ones are quite dangerous. If you put a 'wrong' patron category type into the pref, it will delete a big chunk of your users without batting an eye (and they are delete-deleted, not moved). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #43 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #42)
But... from our own experience: especially the self reg ones are quite dangerous. If you put a 'wrong' patron category type into the pref, it will delete a big chunk of your users without batting an eye (and they are delete-deleted, not moved).
I think the "Delete unverified self registrations older than DAYS" should be safe either way? But "Delete expired self registration accounts" does sound dangerous as you describe. Personally, I don't see the merit of having that in cleanup_database.pl at all. Even with a sysadmin adding that flag, you can't control the behaviour of library staff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #44 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
But "Delete expired self registration accounts" does sound dangerous as you describe. Personally, I don't see the merit of having that in cleanup_database.pl at all. Even with a sysadmin adding that flag, you can't control the behaviour of library staff.
I think having all "delete" and clean-up things together is ok, especially when we move things into the GUI. But it requires good documentation for sure. We added a hint to the system preference, maybe we should add one to the script as well for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #41)
(In reply to Jonathan Druart from comment #36)
David, basically you are asking for another switch in koha-conf that would be off by default, that's it?
I'm not sure what you mean by "asking for another switch in koha-conf".
I don't want to add more confusion and I guess it wouldn't have answered the need anyway. I thought you were asking for more flexibility and so adding a new switch in koha-conf.xml to allow the syspref to have precedence over the script flag. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off --- Comment #46 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I would like another QA stamp and clarification on what we need here. I won't have time to dedicate to this in the next couple of weeks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |BLOCKED --- Comment #47 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #48 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- It might be completely obvious for people using this feature daily how it works, but since not everybody does, a little bit more test plan would have been helpful. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I read the discussion. Yes, there is a trend to put values in the prefs and no longer on the cmdline. We are in a hybrid situation now (as we are with many things). Asking to put something in koha-conf too would be rather tedious imo. This looks good to me. Adding just a small follow-up including my test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120927|0 |1 is obsolete| | --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125584 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125584&action=edit Bug 25429: Cleanup Database - remove resolved claims returned from db after X days Add option to cleanup_database script to removed 'resolved' return claims from the database after a specified number of days. Test Plan: 1) Apply this patch 2) Set the new syspref CleanUpDatabaseReturnClaims to a number of days 3) Run cleanup_database.pl 4) Note resolved claims older than the specified number of days are removed from the database Bug 25429: Implement system preference, remove command line switch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120928|0 |1 is obsolete| | --- Comment #51 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125585 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125585&action=edit Bug 25429: Script should be able to run with only return claims Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120929|0 |1 is obsolete| | --- Comment #52 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125586 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125586&action=edit Bug 25429: Use filter_by_last_update 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=25429 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120930|0 |1 is obsolete| | --- Comment #53 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125587&action=edit Bug 25429: Require command line switch to enable, control DAYS via syspref 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=25429 --- Comment #54 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125588&action=edit Bug 25429: (QA follow-up) Add extra variable for days I think it is a bit more clear to use another variable for the number of days from the preferences than overwriting the flag variable. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested by adding an authval for LOST. Filling prefs ClaimReturnedLostValue, CleanUpDatabaseReturnClaims. Claiming a return, resolving it. Setting date back via SQL. Running the script with -v --return-claims, toggling --confirm. The claim is counted and deleted with confirm. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 --- Comment #55 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, 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=25429 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Summary|Cleanup Database - remove |`cleanup_database.pl` |resolved claims returned |remove resolved claims |from db after X days |returned from db after X | |days -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|`cleanup_database.pl` |`cleanup_database.pl` |remove resolved claims |should remove resolved |returned from db after X |claims returned after X |days |days -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement adds the release notes| |new | |`CleanUpDatabaseReturnClaim | |s` system preference | |allowing administrators to | |specify how many days after | |resolution a claimed return | |record should be deleted | |from the database. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement adds the |This enhancement adds the release notes|new |new |`CleanUpDatabaseReturnClaim |`CleanUpDatabaseReturnClaim |s` system preference |s` system preference |allowing administrators to |allowing administrators to |specify how many days after |specify how many days after |resolution a claimed return |resolution a claimed return |record should be deleted |record should be deleted |from the database. |from the database. | | | |For | |this functionality to be | |enabled, the | |`cleanup_database.pl` must | |be scheduled to run | |regularly with the new | |`--returns_claims` | |parameter passed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement adds the |This enhancement adds the release notes|new |new |`CleanUpDatabaseReturnClaim |`CleanUpDatabaseReturnClaim |s` system preference |s` system preference |allowing administrators to |allowing administrators to |specify how many days after |specify how many days after |resolution a claimed return |resolution a claimed return |record should be deleted |record should be deleted |from the database. |from the database. | | |For |For |this functionality to be |this functionality to be |enabled, the |enabled, the |`cleanup_database.pl` must |`cleanup_database.pl` must |be scheduled to run |be scheduled to run |regularly with the new |regularly with the new |`--returns_claims` |`--return_claims` parameter |parameter passed. |passed. --- Comment #56 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Wondering: return-claims or return_claims? + --return-claims Purge all resolved return claims older than the number of days specified in + the system preference CleanUpDatabaseReturnClaims. vs. +$return_claims = C4::Context->preference('CleanUpDatabaseReturnClaims') if $return_claims; if ($return_claims) { print "Purging return claims older than $return_claims days.\n" if $verbose; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement adds the |This enhancement adds the release notes|new |new |`CleanUpDatabaseReturnClaim |`CleanUpDatabaseReturnClaim |s` system preference |s` system preference |allowing administrators to |allowing administrators to |specify how many days after |specify how many days after |resolution a claimed return |resolution a claimed return |record should be deleted |record should be deleted |from the database. |from the database. | | |For |For |this functionality to be |this functionality to be |enabled, the |enabled, the |`cleanup_database.pl` must |`cleanup_database.pl` must |be scheduled to run |be scheduled to run |regularly with the new |regularly with the new |`--return_claims` parameter |`--return-claims` parameter |passed. |passed. --- Comment #57 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- return-claims -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org