gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 18300: [QA Follow-up] Fix return value inconsistency 

As noted on bug report 17669, the return values of delete (both singular
and plural), delete_missing and delete_temporary should be consistent.

Removed the if-construction around full_path. We do not need it; in the
very exceptional case that full_path would be empty, we should delete
the record since the file is missing.

Adjusted POD and unit tests accordingly.

Test plan:
Run t/db_dependent/Upload.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2017-04-03 21:43:48 +02:00 committed by Kyle M Hall
parent 8c80c9b8e1
commit 4eb53eeee9
2 changed files with 21 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
Koha/UploadedFiles.pm
View File

@ -115,24 +115,30 @@ Deletes all records where the actual file is not found.
Supports a keep_record hash parameter to do a check only.
Returns the number of missing files (and/or deleted records).
Return value: If you pass keep_record, it returns the number of records where
the file is not found, or 0E0. Otherwise it returns a number, 0E0 or -1 just
as delete does.
=cut
sub delete_missing {
my ( $self, $params ) = @_;
$self = Koha::UploadedFiles->new if !ref($self); # handle class call
my $cnt = 0;
my $rv = 0;
while( my $row = $self->next ) {
if( my $file = $row->full_path ) {
next if -e $file;
# We are passing keep_file since we already know that the file
# is missing and we do not want to see the warning
$row->delete({ keep_file => 1 }) if !$params->{keep_record};
$cnt++;
my $file = $row->full_path;
next if -e $file;
if( $params->{keep_record} ) {
$rv++;
next;
}
# We are passing keep_file since we already know that the file
# is missing and we do not want to see the warning
# Apply the same logic as in delete for the return value
my $delete = $row->delete({ keep_file => 1 }); # 1, 0E0 or -1
$rv = ( $delete < 0 || $rv < 0 ) ? -1 : ( $rv + $delete );
}
return $cnt;
return $rv==0 ? "0E0" : $rv;
}
=head3 search_term

9
t/db_dependent/Upload.t
View File

@ -193,20 +193,23 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub {
};
subtest 'Test delete_missing' => sub {
plan tests => 4;
plan tests => 5;
# If we add files via TestBuilder, they do not exist
my $upload01 = $builder->build({ source => 'UploadedFile' });
my $upload02 = $builder->build({ source => 'UploadedFile' });
# dry run first
my $deleted = Koha::UploadedFiles->delete_missing({ keep_record => 1 });
is( $deleted, 2, 'Expect two missing files' );
is( $deleted, 2, 'Expect two records with missing files' );
isnt( Koha::UploadedFiles->find( $upload01->{id} ), undef, 'Not deleted' );
$deleted = Koha::UploadedFiles->delete_missing;
is( $deleted, 2, 'Deleted two missing files' );
ok( $deleted =~ /^(2|-1)$/, 'Deleted two records with missing files' );
is( Koha::UploadedFiles->search({
id => [ $upload01->{id}, $upload02->{id} ],
})->count, 0, 'Records are gone' );
# Repeat it
$deleted = Koha::UploadedFiles->delete_missing;
is( $deleted, "0E0", "Return value of 0E0 expected" );
};
subtest 'Call search_term with[out] private flag' => sub {