Bugzilla – Attachment 96872 Details for
Bug 21684
Koha::Object[s]->delete methods must behave identically as the corresponding DBIx::Class ones
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21684: Fix UploadedFile[s]->delete
Bug-21684-Fix-UploadedFiles-delete.patch (text/plain), 4.39 KB, created by
Jonathan Druart
on 2020-01-07 11:12:37 UTC
(
hide
)
Description:
Bug 21684: Fix UploadedFile[s]->delete
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-01-07 11:12:37 UTC
Size:
4.39 KB
patch
obsolete
>From 16a78a19a54df9348879f100df8146a26480aaca Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 7 Jan 2020 12:10:05 +0100 >Subject: [PATCH] Bug 21684: Fix UploadedFile[s]->delete > >Tests were failing with: > # Failed test 'Test delete via UploadedFile as well as UploadedFiles' > # at t/db_dependent/Upload.t line 193. > DBIx::Class::Row::delete(): Not in database at /kohadevbox/koha/Koha/Object.pm line 219 > >I am not sure this patch is perfect, a set of uploaded files should be >deleted in a transaction, which would be rollback if something is wrong. >But it will be tricky to restore the files after they have been deleted. >It seems that we should deal with that with a more complicated process >and should be part of a separate bug. >--- > Koha/UploadedFile.pm | 7 ------- > Koha/UploadedFiles.pm | 8 ++++---- > t/db_dependent/Upload.t | 13 ++++++++----- > 3 files changed, 12 insertions(+), 16 deletions(-) > >diff --git a/Koha/UploadedFile.pm b/Koha/UploadedFile.pm >index 20fc608c68..bc7d4a240f 100644 >--- a/Koha/UploadedFile.pm >+++ b/Koha/UploadedFile.pm >@@ -75,13 +75,6 @@ sub delete { > my $file = $self->full_path; > > my $retval = $self->SUPER::delete; >- if( !defined($retval) ) { # undef is Unknown (-1) >- $retval = -1; >- } elsif( $retval eq '0' ) { # 0 => 0E0 >- $retval = "0E0"; >- } elsif( $retval !~ /^(0E0|1)$/ ) { # Unknown too >- $retval = -1; >- } > return $retval if $params->{keep_file}; > > if( ! -e $file ) { >diff --git a/Koha/UploadedFiles.pm b/Koha/UploadedFiles.pm >index b6804f299b..d7023ce02d 100644 >--- a/Koha/UploadedFiles.pm >+++ b/Koha/UploadedFiles.pm >@@ -72,8 +72,8 @@ sub delete { > # We use the individual delete on each resultset record > my $rv = 0; > while( my $row = $self->next ) { >- my $delete= $row->delete( $params ); # 1, 0E0 or -1 >- $rv = ( $delete < 0 || $rv < 0 ) ? -1 : ( $rv + $delete ); >+ my $deleted = eval { $row->delete( $params ) }; >+ $rv++ if $deleted && !$@; > } > return $rv==0 ? "0E0" : $rv; > } >@@ -135,8 +135,8 @@ sub delete_missing { > # 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 ); >+ my $deleted = eval { $row->delete({ keep_file => 1 }) }; >+ $rv++ if $deleted && !$@; > } > return $rv==0 ? "0E0" : $rv; > } >diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t >index 802dc58da0..93dc5f500f 100644 >--- a/t/db_dependent/Upload.t >+++ b/t/db_dependent/Upload.t >@@ -4,6 +4,7 @@ use Modern::Perl; > use File::Temp qw/ tempdir /; > use Test::More tests => 13; > use Test::Warn; >+use Try::Tiny; > > use Test::MockModule; > use t::lib::Mocks; >@@ -171,7 +172,7 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub { > my $kohaobj = Koha::UploadedFiles->find( $upl->result ); > $path = $kohaobj->full_path; > $delete = $kohaobj->delete; >- ok( $delete=~/^-?1$/, 'Delete successful' ); >+ ok( $delete, 'Delete successful' ); > isnt( -e $path, 1, 'File no longer found after delete' ); > > # add another record with TestBuilder, so file does not exist >@@ -180,16 +181,18 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub { > warning_like { $delete = Koha::UploadedFiles->find( $upload01->{id} )->delete; } > qr/file was missing/, > 'delete warns when file is missing'; >- ok( $delete=~/^-?1$/, 'Deleting record was successful' ); >+ ok( $delete, 'Deleting record was successful' ); > is( Koha::UploadedFiles->count, 4, 'Back to four uploads now' ); > > # add another one with TestBuilder and delete twice (file does not exist) > $upload01 = $builder->build({ source => 'UploadedFile' }); > $kohaobj = Koha::UploadedFiles->find( $upload01->{id} ); > $delete = $kohaobj->delete({ keep_file => 1 }); >- $delete = $kohaobj->delete({ keep_file => 1 }); >- ok( $delete =~ /^(0E0|-1)$/, 'Repeated delete unsuccessful' ); >- # NOTE: Koha::Object->delete does not return 0E0 (yet?) >+ try { >+ $delete = $kohaobj->delete({ keep_file => 1 }); >+ } catch { >+ ok( $_->isa("DBIx::Class::Exception"), 'Repeated delete unsuccessful' ); >+ } > }; > > subtest 'Test delete_missing' => sub { >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21684
:
81317
|
81318
|
81319
|
90937
|
96218
|
96254
|
96270
|
96271
|
96272
|
96429
|
96430
|
96431
| 96872 |
96888
|
96895
|
96898