From e0674dde81210342f1efeeada0d554db6d3541fc 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. Signed-off-by: Marcel de Rooy --- Koha/UploadedFiles.pm | 9 ++++++--- misc/cronjobs/cleanup_database.pl | 7 ++++--- t/db_dependent/Upload.t | 19 ++++++++----------- 3 files changed, 18 insertions(+), 17 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/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