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

Bug 42656: (QA follow-up) Use _abort_operation() for Local's 'not configured' errors too 

_upload_file, _download_file, _list_files and _rename_file's 'no directory configured' checks still called add_message() directly, skipping the status persistence this bug added elsewhere in the file. Route them through _abort_operation() so all error paths in Local.pm stay consistent.

prove t/db_dependent/Koha/File/Transport/Local.t

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Pedro Amorim 2026-08-06 10:44:41 +00:00
parent 4deb199163
commit de230bc58b
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
2 changed files with 18 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
Koha/File/Transport/Local.pm
View File

@ -138,14 +138,10 @@ sub _upload_file {
my $upload_dir = $self->_working_directory('upload_directory'); my $upload_dir = $self->_working_directory('upload_directory');
unless ( defined $upload_dir ) { unless ( defined $upload_dir ) {
$self->add_message( return $self->_abort_operation(
{ $operation,
message => $operation, { error => 'No upload directory configured or set via change_directory()' }
type => 'error',
payload => { error => 'No upload directory configured or set via change_directory()' }
}
); );
return;
} }
my $destination = File::Spec->catfile( $upload_dir, $remote_file ); my $destination = File::Spec->catfile( $upload_dir, $remote_file );
@ -185,14 +181,10 @@ sub _download_file {
my $download_dir = $self->_working_directory('download_directory'); my $download_dir = $self->_working_directory('download_directory');
unless ( defined $download_dir ) { unless ( defined $download_dir ) {
$self->add_message( return $self->_abort_operation(
{ $operation,
message => $operation, { error => 'No download directory configured or set via change_directory()' }
type => 'error',
payload => { error => 'No download directory configured or set via change_directory()' }
}
); );
return;
} }
my $source = File::Spec->catfile( $download_dir, $remote_file ); my $source = File::Spec->catfile( $download_dir, $remote_file );
@ -296,14 +288,10 @@ sub _list_files {
my $directory = $self->_working_directory('download_directory'); my $directory = $self->_working_directory('download_directory');
unless ( defined $directory ) { unless ( defined $directory ) {
$self->add_message( return $self->_abort_operation(
{ $operation,
message => $operation, { error => 'No download directory configured or set via change_directory()' }
type => 'error',
payload => { error => 'No download directory configured or set via change_directory()' }
}
); );
return;
} }
unless ( -d $directory ) { unless ( -d $directory ) {
@ -387,14 +375,10 @@ sub _rename_file {
my $directory = $self->_working_directory('download_directory'); my $directory = $self->_working_directory('download_directory');
unless ( defined $directory ) { unless ( defined $directory ) {
$self->add_message( return $self->_abort_operation(
{ $operation,
message => $operation, { error => 'No download directory configured or set via change_directory()' }
type => 'error',
payload => { error => 'No download directory configured or set via change_directory()' }
}
); );
return;
} }
my $old_path = File::Spec->catfile( $directory, $old_name ); my $old_path = File::Spec->catfile( $directory, $old_name );

7
t/db_dependent/Koha/File/Transport/Local.t
View File

@ -119,7 +119,7 @@ subtest 'current_directory() tests' => sub {
}; };
subtest 'an unconfigured directory fails cleanly instead of falling back to "." (the Koha process cwd)' => sub { subtest 'an unconfigured directory fails cleanly instead of falling back to "." (the Koha process cwd)' => sub {
plan tests => 8; plan tests => 10;
$schema->storage->txn_begin; $schema->storage->txn_begin;
@ -151,6 +151,11 @@ subtest 'an unconfigured directory fails cleanly instead of falling back to "."
'error explains the directory is unconfigured, rather than silently listing an unrelated directory' 'error explains the directory is unconfigured, rather than silently listing an unrelated directory'
); );
my $reloaded = Koha::File::Transports->find( $transport->id );
my $persisted_status = decode_json( $reloaded->status );
is( $persisted_status->{status}, 'errors', 'a "not configured" error also persists an "errors" status' );
is( $persisted_status->{operations}[-1]{code}, 'list', 'persisted status records the failing operation' );
my $renamed = $transport->rename_file( 'a.txt', 'b.txt' ); my $renamed = $transport->rename_file( 'a.txt', 'b.txt' );
is( $renamed, undef, 'rename_file() fails the same way when no download_directory is configured' ); is( $renamed, undef, 'rename_file() fails the same way when no download_directory is configured' );