From ce3d18e9f3f7e505af86a32c33fbffd966867dd3 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Date: Thu, 22 Feb 2018 11:52:14 +0100 Subject: [PATCH] Bug 19633: Use alphanumeric error codes in upload The error codes 1 to 7 are used in Uploader.pm or tools/upload.pl. It would be nice to use alphanumeric codes instead. No behavior change expected. Test plan: [1] Run t/db_dependent/Upload.t [2] Verify that a regular upload with tools/upload.pl still works. [3] Rename upload_path in your koha-conf.xml. Restart Plack, flush the cache and try to upload to a category. Correct error message? [4] Upload the same file twice to the same category. Correct error message the second time? Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Koha/Uploader.pm | 14 ++++--- .../intranet-tmpl/prog/en/modules/tools/upload.tt | 43 +++++++++++++++------- t/db_dependent/Upload.t | 2 +- tools/upload.pl | 10 +++-- 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/Koha/Uploader.pm b/Koha/Uploader.pm index e1b34c9e0e..dd60d51934 100644 --- a/Koha/Uploader.pm +++ b/Koha/Uploader.pm @@ -58,8 +58,12 @@ Koha::Uploader - Facilitate file uploads (temporary and permanent) =cut -use constant KOHA_UPLOAD => 'koha_upload'; +use constant KOHA_UPLOAD => 'koha_upload'; use constant BYTES_DIGEST => 2048; +use constant ERRCODE_1 => 'UPLERR_ALREADY_EXISTS'; +use constant ERRCODE_2 => 'UPLERR_CANNOT_WRITE'; +use constant ERRCODE_3 => 'UPLERR_NO_ROOT_DIR'; +use constant ERRCODE_4 => 'UPLERR_NO_TEMP_DIR'; use Modern::Perl; use CGI; # no utf8 flag, since it may interfere with binary uploads @@ -220,9 +224,9 @@ sub _create_file { $self->{files}->{$filename}->{errcode} ) { #skip } elsif( !$self->{temporary} && !$self->{rootdir} ) { - $self->{files}->{$filename}->{errcode} = 3; #no rootdir + $self->{files}->{$filename}->{errcode} = ERRCODE_3; #no rootdir } elsif( $self->{temporary} && !$self->{tmpdir} ) { - $self->{files}->{$filename}->{errcode} = 4; #no tempdir + $self->{files}->{$filename}->{errcode} = ERRCODE_4; #no tempdir } else { my $dir = $self->_dir; my $hashval = $self->{files}->{$filename}->{hash}; @@ -235,7 +239,7 @@ sub _create_file { hashvalue => $hashval, uploadcategorycode => $self->{category}, })->count ) { - $self->{files}->{$filename}->{errcode} = 1; #already exists + $self->{files}->{$filename}->{errcode} = ERRCODE_1; #already exists return; } @@ -244,7 +248,7 @@ sub _create_file { $fh->binmode; $self->{files}->{$filename}->{fh}= $fh; } else { - $self->{files}->{$filename}->{errcode} = 2; #not writable + $self->{files}->{$filename}->{errcode} = ERRCODE_2; #not writable } } return $fh; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt index a4e6147f6e..7972f0162b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt @@ -249,18 +249,6 @@ [% MACRO jsinclude BLOCK %] [% Asset.js("js/tools-menu.js") %] [% INCLUDE 'datatables.inc' %] - <script type="text/javascript"> - var errMESSAGES = [ - "Error 0: Not in use", - _("This file already exists (in this category)."), - _("File could not be created. Check permissions."), - _("Your koha-conf.xml does not contain a valid upload_path."), - _("No temporary directory found."), - _("File could not be read."), - _("File has been deleted."), - _("File or upload record could not be deleted."), - ]; - </script> [% Asset.js("js/file-upload.js") %] <script type="text/javascript"> function StartUpload() { @@ -318,13 +306,42 @@ var str = ''; for( var file in err ) { str= str + '<p>' + file + ': ' + - errMESSAGES[ err[file].code ] + '</p>'; + errMESSAGES( err[file].code ) + '</p>'; } if( str ) { $('#myalerts').html(str); $('#myalerts').show(); } } + function errMESSAGES(code) { + var rv; + switch(code) { + case 'UPLERR_ALREADY_EXISTS': + rv = _("This file already exists (in this category)."); + break; + case 'UPLERR_CANNOT_WRITE': + rv = _("File could not be created. Check permissions."); + break; + case 'UPLERR_NO_ROOT_DIR': + rv = _("Your koha-conf.xml does not contain a valid upload_path."); + break; + case 'UPLERR_NO_TEMP_DIR': + rv = _("No temporary directory found."); + break; + case 'UPLERR_FILE_NOT_READ': + rv = _("File could not be read."); + break; + case 'UPL_FILE_DELETED': // An alert, no error + rv = _("File has been deleted."); + break; + case 'UPLERR_FILE_NOT_DELETED': + rv = _("File or upload record could not be deleted."); + break; + default: + rv = code; + } + return rv; + } function CheckSearch() { if( $("#term").val()=="" ) { alert( _("Please enter a search term.") ); diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t index 729bb86248..8b4f0ba7c4 100644 --- a/t/db_dependent/Upload.t +++ b/t/db_dependent/Upload.t @@ -145,7 +145,7 @@ subtest 'Add same file in same category' => sub { is( $upl->count, 0, 'Upload 4 failed as expected' ); is( $upl->result, undef, 'Result is undefined' ); my $e = $upl->err; - is( $e->{file2}->{code}, 1, "Errcode 1 [already exists] reported" ); + is( $e->{file2}->{code}, Koha::Uploader::ERRCODE_1, "Errcode [already exists] reported" ); }; subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub { diff --git a/tools/upload.pl b/tools/upload.pl index 9b10b4226c..20820b8c47 100755 --- a/tools/upload.pl +++ b/tools/upload.pl @@ -25,6 +25,10 @@ use C4::Auth; use C4::Output; use Koha::UploadedFiles; +use constant ERRCODE_5 => 'UPLERR_FILE_NOT_READ'; +use constant ALRCODE_6 => 'UPL_FILE_DELETED'; # alert, no error +use constant ERRCODE_7 => 'UPLERR_FILE_NOT_DELETED'; + my $input = CGI::->new; my $op = $input->param('op') // 'new'; my $plugin = $input->param('plugin'); @@ -87,9 +91,9 @@ if ( $op eq 'new' ) { my $delete = $rec ? $rec->delete : undef; #TODO Improve error handling my $msg = $delete - ? JSON::to_json({ $fn => { code => 6 }}) + ? JSON::to_json({ $fn => { code => ALRCODE_6 }}) : $id - ? JSON::to_json({ $fn || $id, { code => 7 }}) + ? JSON::to_json({ $fn || $id, { code => ERRCODE_7 }}) : ''; $template->param( mode => 'deleted', @@ -104,7 +108,7 @@ if ( $op eq 'new' ) { if ( !$rec || !$fh ) { $template->param( mode => 'new', - msg => JSON::to_json({ $id => { code => 5 }}), + msg => JSON::to_json({ $id => { code => ERRCODE_5 }}), ); output_html_with_http_headers $input, $cookie, $template->output; } else { -- 2.11.0