From 05e25dbc188d9d6340d03750a47cb1cce130715f 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 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 Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- Koha/UploadedFiles.pm | 9 ++++++--- .../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 3d28b25563..b75aee8927 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 9403951e2e..7ec9f60ad5 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 4e2c243fa5..b39d9a7315 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 136a9d77e7..d752426d91 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.39.5