[Bug 13287] New: Add a system preference to define the number of days used in purge_suggestions.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Bug ID: 13287 Summary: Add a system preference to define the number of days used in purge_suggestions.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Reports Assignee: gmcharlt@gmail.com Reporter: simith.doliveira@inlibro.com QA Contact: testopia@bugs.koha-community.org The system preferences value is used whenever purge_suggestions.pl is called without the 'days' parameter. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |simith.doliveira@inlibro.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #1 from simith.doliveira@inlibro.com --- Testing: I Apply the patch II Run updatedatabase.pl a) Run purge_suggestion.pl without the days parameter - validate error message b) Run purge_suggestion.pl with the days parameter - validate no error message c) Insert a number of days in the system variable PurgeSuggestionsOlderThan d) Run purge_suggestion.pl without the days parameter - validate no error message -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #2 from simith.doliveira@inlibro.com --- Created attachment 33661 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33661&action=edit Add a system preference to define the number of days used in purge_suggestions.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric.begin@inLibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |francois.charbonnier@inlibr | |o.com, | |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Change sponsored?|--- |Sponsored --- Comment #3 from simith.doliveira@inlibro.com --- Sponsored-by: CCSR ( http://www.ccsr.qc.ca ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Hi Simith, Thanks for writing your patch, but I do have a question: Although we have some cron jobs that use or even fall back to preferences, it is not the default solution. Normally, cronjobs should just get their parameters from the command line. If you do not pass a command line parameter and you do not expect the cronjob to use a pref as fallback, you could suddenly have lost your suggestions unwillingly. So, could you give a very good reason why the script still should fall back to the pref value? Isn't it true that you set this cmd line parameter once in your crontab and do not touch it again? Marcel -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #5 from Eric Bégin <eric.begin@inLibro.com> --- Thanks Marcel for the opportunity to clarify why a syspref is within scripts executed via a cronjobs. The main reason is to give the libraries a control to those parameters without having to constantly get back to their service provider in order to change a parameter on the cronjob command line. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mirko@abunchofthings.net --- Comment #6 from Mirko Tietgen <mirko@abunchofthings.net> --- How often is a value like this changed? I would expect this to be set once and probably never touched again? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think there is a use case for making the options easy to configure from the gui - multiple instances hosted on one server. Then every instance can decide on their defaults and you don't have to touch the cronjobs to make it happen. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #8 from Mirko Tietgen <mirko@abunchofthings.net> --- Right, I did not think about that. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #9 from Marc Véron <veron@veron.ch> --- I agree with Eric and Katrin. The patch works as expected, however I think the explanation in syspref needs some changes: --Original------- Keep purchase suggestions for a period of [ ] days. WARNING - Leave this field empty if you don't want to activate this automatic feature. Example: [30] Sets purgation of suggestions for those older than 30 days. (Used when the purge_suggestions.pl script is called with a specific number of days) ------------------ --Proposition----- Keep accepted or rejected purchase suggestions for a period of [ ] days. WARNING - Leave this field empty if you don't want to activate this automatic feature. Example: [30] Sets purgation of suggestions for those older than 30 days. (Used when the cronjob purge_suggestions.pl is active and called without a specific number of days) ------------------ Additionally, I would like to see some more explanation in the manual http://manual.koha-community.org/latest/en/cronjobsch.html#cronjobs 1.9.1. Clean up old suggestions -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 33661 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33661 Add a system preference to define the number of days used in purge_suggestions.pl Review of attachment 33661: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13287&attachment=33661) ----------------------------------------------------------------- ::: misc/cronjobs/purge_suggestions.pl @@ +47,5 @@
+-help|? This message +-days TTT to define the age of suggestions to delete + +Example: +$PERL5LIB/misc/cronjobs/purge_suggestions.pl -days 30
This line change seems wrong to me. It doesn't interpret $PERL5LIB, and this is a drastic change from what was already there. And though what was there is a little unclear to the average user, perhaps it is better? @@ +68,4 @@
DelSuggestionsOlderThan($days); } +elsif(defined($days) && $days == 0) { + print << 'ERROR';
Seriously, why? There is no need to << 'LABEL ... LABEL this. It's a waste of vertical space. @@ +72,5 @@
+ This script is not executed with 0 days. Aborted. +ERROR +} +else { + print << 'ERROR';
Why not use: print STDERR "This script requires a positive number of days. Aborted."; -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 33661 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33661 Add a system preference to define the number of days used in purge_suggestions.pl Review of attachment 33661: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13287&attachment=33661) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ +561,5 @@
+ class: integer + - days. + - <br>WARNING - Leave this field empty if you don't want to activate this automatic feature. + - "<br>Example: [30] Sets purgation of suggestions for those older than 30 days." + - <br>(Used when the purge_suggestions.pl script is called with a specific number of days)
As per comment #9. This is unclear. This feature is only used if the --days ### command line parameter is not passed. This last line makes it sound like it is used instead of the command line value. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com Status|Needs Signoff |Failed QA --- Comment #12 from M. Tompsett <mtompset@hotmail.com> --- Marking as 'Failed QA', since 'In Discussion' tends to head into 'Never get looked at again' land. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Rémi Mayrand-Provencher <remi.mayrand-provencher@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33661|0 |1 is obsolete| | --- Comment #13 from Rémi Mayrand-Provencher <remi.mayrand-provencher@inLibro.com> --- Created attachment 41219 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41219&action=edit Bug13287 - Add a system preference to define the number of days used in purge_suggestions.pl The system preferences value is used whenever purge_suggestions.pl is called without the 'days' parameter. This patch uses the preference description suggested by comment #9. This version should now be cleanly applicable. I Apply the patch II Run updatedatabase.pl a) Run purge_suggestions.pl without the days parameter - validate that there is an error message b) Run purge_suggestions.pl with the days parameter - validate that there is no error message c) Insert a number of days in the system variable PurgeSuggestionsOlderThan d) Run purge_suggestions.pl without the days parameter - validate that there is no error message -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Rémi Mayrand-Provencher <remi.mayrand-provencher@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |remi.mayrand-provencher@inL | |ibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41219|0 |1 is obsolete| | --- Comment #14 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44312 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44312&action=edit Bug13287 - Add a system preference to define the number of days used in purge_suggestions.pl The system preferences value is used whenever purge_suggestions.pl is called without the 'days' parameter. This patch uses the preference description suggested by comment #9. This version should now be cleanly applicable. I Apply the patch II Run updatedatabase.pl a) Run purge_suggestions.pl without the days parameter - validate that there is an error message b) Run purge_suggestions.pl with the days parameter - validate that there is no error message c) Insert a number of days in the system variable PurgeSuggestionsOlderThan d) Run purge_suggestions.pl without the days parameter - validate that there is no error message Signed-off-by: Liz Rea <liz@catalyst.net.nz> Tested per plan, all tests pass. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |enhancement --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Changing this to enh - it's a good one, but not a bug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13287 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- I would like to see this script die with errors if both the command line switch and the system preference are set. I can see a scenario where a sysadmin changes the value on the switch but it doesn't change the purge days! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org