From c39ea95e3806e55bd12a4f5e993b4f79ba87ba4a Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Fri, 23 Feb 2018 10:33:49 +0100
Subject: [PATCH] Bug 13287: (QA follow-up) Final polishing

Cosmetic changes.
And: adding a confirm flag (see earlier comment too). Without this flag but
with having a filled pref, the script would purge when you do not pass any
parameter. This might not be appreciated.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 misc/cronjobs/crontab.example      |  2 +-
 misc/cronjobs/purge_suggestions.pl | 41 +++++++++++-------------------
 2 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/misc/cronjobs/crontab.example b/misc/cronjobs/crontab.example
index f3dad73f15..db1c703c3c 100644
--- a/misc/cronjobs/crontab.example
+++ b/misc/cronjobs/crontab.example
@@ -90,7 +90,7 @@ KOHA_CRON_PATH = /usr/share/koha/bin/cronjobs
 16 1 * * * __KOHA_USER__ $KOHA_CRON_PATH/cleanup_database.pl --sessions --zebraqueue 10 --list-invites --temp-uploads
 
 # delete old purchase suggestions weekly. Replace XX with a number to define the age of suggestions to delete.
-@weekly	__KOHA_USER__  $KOHA_CRON_PATH/purge_suggestions.pl --days XX > /dev/null 2>&1
+@weekly	__KOHA_USER__  $KOHA_CRON_PATH/purge_suggestions.pl --confirm --days XX > /dev/null 2>&1
 
 # share_usage_with_koha_community.pl every months
 0 0 1 * *  __KOHA_USER__ $KOHA_CRON_PATH/share_usage_with_koha_community.pl
diff --git a/misc/cronjobs/purge_suggestions.pl b/misc/cronjobs/purge_suggestions.pl
index 6dff5db086..543d629273 100755
--- a/misc/cronjobs/purge_suggestions.pl
+++ b/misc/cronjobs/purge_suggestions.pl
@@ -18,10 +18,8 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
-use utf8;
 
 BEGIN {
-
     # find Koha's Perl modules
     # test carefully before changing this
     use FindBin;
@@ -34,48 +32,39 @@ use C4::Suggestions;
 use C4::Log;
 use C4::Context;
 
-my ( $help, $days );
+my ( $help, $days, $confirm );
 
 GetOptions(
     'help|?' => \$help,
-    'days=s' => \$days,
+    'days:i' => \$days,
+    'confirm'=> \$confirm,
 );
 
 my $usage = << 'ENDUSAGE';
-This script delete old suggestions
+This script deletes old suggestions
 Parameters:
 -help|? This message
 -days TTT to define the age of suggestions to delete
+-confirm flag needed to confirm purge operation
+
+The days parameter falls back to the value of system preference
+PurgeSuggestionsOlderThan. Suggestions are deleted only for a positive
+number of days.
 
 Example:
 ENDUSAGE
-$usage .= $0 . " -days 30\n";
+$usage .= $0 . " -confirm -days 30\n";
 
 # If this script is called without the 'days' parameter, we use the system preferences value instead.
-if ( !defined($days) && !$help ) {
-    my $purge_sugg_days =
-      C4::Context->preference('PurgeSuggestionsOlderThan') || q{};
-    if ( $purge_sugg_days ne q{} and $purge_sugg_days >= 0 ) {
-        $days = $purge_sugg_days;
-    }
-}
+$days = C4::Context->preference('PurgeSuggestionsOlderThan') if !defined($days);
 
 # If this script is called with the 'help' parameter, we show up the help message and we leave the script without doing anything.
-if ($help) {
+if( !$confirm || $help || !defined($days) ) {
+    print "No confirm parameter passed!\n\n" if !$confirm && !$help;
     print $usage;
-    exit;
-}
-
-if ( defined($days) && $days ne q{} && $days > 0 ) {
+} elsif( $days > 0 ) {
     cronlogaction();
     DelSuggestionsOlderThan($days);
-}
-elsif (!defined($days)){
-    print $usage;
-}
-elsif ( $days == 0 ) {
-    warn "This script is not executed with 0 days. Aborted.\n";
-}
-else {
+} else {
     warn "This script requires a positive number of days. Aborted.\n";
 }
-- 
2.39.5