From b2184a944ba87cebea15dc4269c0ff54aab999fb Mon Sep 17 00:00:00 2001 From: Marcel de Rooy 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 --- 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