Attachment 61743 Details for Bug 17669 – Bug 17669: [QA Follow-up] Allow zero in temp-uploads-days

[patch] Bug 17669: [QA Follow-up] Allow zero in temp-uploads-days

Bug-17669-QA-Follow-up-Allow-zero-in-temp-uploads-.patch (text/plain), 5.20 KB, created by Marcel de Rooy on 2017-03-31 06:44:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2017-03-31 06:44:28 UTC

Size: 5.20 KB

patch

obsolete

>From b2184a944ba87cebea15dc4269c0ff54aab999fb Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 31 Mar 2017 08:31:10 +0200
>Subject: [PATCH] Bug 17669: [QA Follow-up] Allow zero in temp-uploads-days
>Content-Type: text/plain; charset=utf-8
>
>As requested by QA on comment33.
>If the pref is 0 or the overriding command line parameter is 0, all
>temporary files will be deleted. But if the pref is NULL or empty string,
>we will not delete files.
>
>Also adjusted the description of the preference in this regard.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/UploadedFiles.pm                                 |  9 ++++++---
> .../prog/en/modules/admin/preferences/tools.pref      |  2 +-
> misc/cronjobs/cleanup_database.pl                     |  7 ++++---
> t/db_dependent/Upload.t                               | 19 ++++++++-----------
> 4 files changed, 19 insertions(+), 18 deletions(-)
>
>diff --git a/Koha/UploadedFiles.pm b/Koha/UploadedFiles.pm
>index 3d28b25..b75aee8 100644
>--- a/Koha/UploadedFiles.pm
>+++ b/Koha/UploadedFiles.pm
>@@ -86,9 +86,12 @@ Returns true if no errors occur. (Even when no files had to be deleted.)
> 
> sub delete_temporary {
>     my ( $self, $params ) = @_;
>-    my $days = $params->{override_pref} ||
>-        C4::Context->preference('UploadPurgeTemporaryFilesDays');
>-    return 1 if !$days;
>+    my $days = C4::Context->preference('UploadPurgeTemporaryFilesDays');
>+    if( exists $params->{override_pref} ) {
>+        $days = $params->{override_pref};
>+    } elsif( !defined($days) || $days eq '' ) { # allow 0, not NULL or ""
>+        return 1;
>+    }
>     my $dt = dt_from_string();
>     $dt->subtract( days => $days );
>     my $parser = Koha::Database->new->schema->storage->datetime_parser;
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref
>index 9403951..7ec9f60 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref
>@@ -31,4 +31,4 @@ Tools:
>             - Automatically delete temporary uploads older than
>             - pref: UploadPurgeTemporaryFilesDays
>               class: integer
>-            - "days in cleanup_database cron job. NOTE: If you leave this field empty (zero), the cron job will not delete any files."
>+            - "days in cleanup_database cron job. NOTE: If you leave this field empty, the cron job will not delete any files. On the other hand a value of 0 means: delete all temporary files."
>diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl
>index 4e2c243..b39d9a7 100755
>--- a/misc/cronjobs/cleanup_database.pl
>+++ b/misc/cronjobs/cleanup_database.pl
>@@ -311,11 +311,12 @@ if ($special_holidays_days) {
> }
> 
> if( $temp_uploads ) {
>-    # Delete temporary uploads, governed by a pref.
>-    # If the pref is empty, nothing happens (unless you override).
>+    # Delete temporary uploads, governed by a pref (unless you override)
>     print "Purging temporary uploads.\n" if $verbose;
>     Koha::UploadedFiles->delete_temporary({
>-        override_pref => $temp_uploads_days,
>+        defined($temp_uploads_days)
>+            ? ( override_pref => $temp_uploads_days )
>+            : ()
>     });
>     print "Done purging temporary uploads.\n" if $verbose;
> }
>diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t
>index 136a9d7..d752426 100644
>--- a/t/db_dependent/Upload.t
>+++ b/t/db_dependent/Upload.t
>@@ -245,7 +245,7 @@ subtest 'Testing allows_add_by' => sub {
> };
> 
> subtest 'Testing delete_temporary' => sub {
>-    plan tests => 7;
>+    plan tests => 6;
> 
>     # Add two temporary files: result should be 3 + 3
>     Koha::Uploader->new({ tmp => 1 })->cgi; # add file6 and file7
>@@ -268,21 +268,18 @@ subtest 'Testing delete_temporary' => sub {
>     $recs[1]->dtcreated($dt)->store;
>     $recs[2]->dtcreated($dt)->store;
> 
>-    # Now call delete_temporary with 0, 6, 5 and 1 (via override)
>-    t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 0 );
>-    Koha::UploadedFiles->delete_temporary;
>-    is( Koha::UploadedFiles->search->count, 6, 'Delete with pref==0' );
>-
>+    # Now call delete_temporary with 6, 5 and 0
>     t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 6 );
>     Koha::UploadedFiles->delete_temporary;
>     is( Koha::UploadedFiles->search->count, 6, 'Delete with pref==6' );
> 
>-    t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 5 );
>-    Koha::UploadedFiles->delete_temporary;
>-    is( Koha::UploadedFiles->search->count, 4, 'Delete with pref==5 makes 4' );
>+    # use override parameter
>+    Koha::UploadedFiles->delete_temporary({ override_pref => 5 });
>+    is( Koha::UploadedFiles->search->count, 4, 'Delete with override==5' );
> 
>-    Koha::UploadedFiles->delete_temporary({ override_pref => 1 });
>-    is( Koha::UploadedFiles->search->count, 3, 'Delete override==1 makes 3' );
>+    t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 0 );
>+    Koha::UploadedFiles->delete_temporary;
>+    is( Koha::UploadedFiles->search->count, 3, 'Delete with pref==0 makes 3' );
>     is( Koha::UploadedFiles->search({ permanent => 1 })->count, 3,
>         'Still 3 permanent uploads' );
> };
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17669: 58176 | 58177 | 58178 | 58179 | 58810 | 58811 | 58812 | 61239 | 61240 | 61241 | 61242 | 61243 | 61465 | 61466 | 61467 | 61468 | 61469 | 61727 | 61729 | 61730 | 61742 | 61743 | 61805 | 61809 | 61818 | 61819 | 62486 | 62487 | 62488 | 62489 | 62490 | 62491 | 62492 | 62493 | 62494 | 62495