From 64079ccaa3284f9bf12945aad082e22c1fe4d90d Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Thu, 26 Sep 2013 11:45:19 -0400 Subject: [PATCH] Bug 6874: kohastructure.sql, jquery.js, refocus, and more Two problems were discovered while doing a fresh install of Koha. These problems in the kohastructure.sql file are addressed with this patch. Clicking the plug-in icon should cause the popup window to refocus. This adds the refocus code to the upload.pl file. The path to the jquery.js script was wrong in the upload_delete_file.tt file. Changed [% themelang %] to [% interface %]. If a user clones 856$u after uploading a file, deletes the file, and then clicks the plugin icon on the first 856$u, this will go immediately to the upload screen with an informative error message. After some validation was added, it was extended to include other cases. This serves to patch 6874 to a state where sign off should be possible. Signed-off-by: Bernardo Gonzalez Kriegel Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- C4/UploadedFiles.pm | 109 ++++++++++++++---- cataloguing/value_builder/upload.pl | 42 +++++-- installer/data/mysql/kohastructure.sql | 2 +- .../cataloguing/value_builder/upload.tt | 55 ++++++--- .../value_builder/upload_delete_file.tt | 2 +- t/db_dependent/UploadedFiles.t | 14 ++- 6 files changed, 173 insertions(+), 51 deletions(-) diff --git a/C4/UploadedFiles.pm b/C4/UploadedFiles.pm index 456bf4e63d..a47b5197fc 100644 --- a/C4/UploadedFiles.pm +++ b/C4/UploadedFiles.pm @@ -183,44 +183,109 @@ sub UploadFile { return; } +=head2 DanglingEntry + + C4::UploadedFiles::DanglingEntry($id,$isfileuploadurl); + +Determine if a entry is dangling. + +Returns: 2 == no db entry + 1 == no file + 0 == both a file and db entry. + -1 == N/A (undef id / non-file-upload URL) + +=cut + +sub DanglingEntry { + my ($id,$isfileuploadurl) = @_; + my $retval; + + if (defined($id)) { + my $file = GetUploadedFile($id); + if($file) { + my $file_path = $file->{filepath}; + my $file_deleted = 0; + unless( -f $file_path ) { + $retval = 1; + } else { + $retval = 0; + } + } + else { + if ( $isfileuploadurl ) { + $retval = 2; + } + else { + $retval = -1; + } + } + } + else { + $retval = -1; + } + return $retval; +} + =head2 DelUploadedFile C4::UploadedFiles::DelUploadedFile($id); Remove a previously uploaded file, given its id. -Returns a false value if an error occurs. +Returns: 1 == file deleted + 0 == file not deleted + -1== no file to delete / no meaninful id passed =cut sub DelUploadedFile { my ($id) = @_; - - my $file = GetUploadedFile($id); - if($file) { - my $file_path = $file->{filepath}; - my $file_deleted = 0; - unless( -f $file_path ) { - warn "Id $file->{id} is in database but not in filesystem, removing id from database"; - $file_deleted = 1; - } else { - if(unlink $file_path) { + my $retval; + + if ($id) { + my $file = GetUploadedFile($id); + if($file) { + my $file_path = $file->{filepath}; + my $file_deleted = 0; + unless( -f $file_path ) { + warn "Id $file->{id} is in database but not in filesystem, removing id from database"; $file_deleted = 1; + } else { + if(unlink $file_path) { + $file_deleted = 1; + } } - } - unless($file_deleted) { - warn "File $file_path cannot be deleted: $!"; - } + unless($file_deleted) { + warn "File $file_path cannot be deleted: $!"; + } - my $dbh = C4::Context->dbh; - my $query = qq{ - DELETE FROM uploaded_files - WHERE id = ? - }; - my $sth = $dbh->prepare($query); - return $sth->execute($id); + my $dbh = C4::Context->dbh; + my $query = qq{ + DELETE FROM uploaded_files + WHERE id = ? + }; + my $sth = $dbh->prepare($query); + my $numrows = $sth->execute($id); + # if either a DB entry or file was deleted, + # then clearly we have a deletion. + if ($numrows>0 || $file_deleted==1) { + $retval = 1; + } + else { + $retval = 0; + } + } + else { + warn "There was no file for id=($id)"; + $retval = -1; + } + } + else { + warn "DelUploadFile called with no id."; + $retval = -1; } + return $retval; } 1; diff --git a/cataloguing/value_builder/upload.pl b/cataloguing/value_builder/upload.pl index 966f0b36ac..a2de3eb711 100755 --- a/cataloguing/value_builder/upload.pl +++ b/cataloguing/value_builder/upload.pl @@ -46,10 +46,15 @@ sub plugin_javascript { function Clic$function_name(index) { var id = document.getElementById(index).value; + var IsFileUploadUrl=0; + if (id.match(/opac-retrieve-file/)) { + IsFileUploadUrl=1; + } if(id.match(/id=([0-9a-f]+)/)){ id = RegExp.\$1; } - window.open(\"../cataloguing/plugin_launcher.pl?plugin_name=upload.pl&index=\"+index+\"&id=\"+id, 'upload', 'width=600,height=400,toolbar=false,scrollbars=no'); + var newin=window.open(\"../cataloguing/plugin_launcher.pl?plugin_name=upload.pl&index=\"+index+\"&id=\"+id+\"&from_popup=0\"+\"&IsFileUploadUrl=\"+IsFileUploadUrl, 'upload', 'width=600,height=400,toolbar=false,scrollbars=no'); + newin.focus(); } @@ -64,10 +69,16 @@ sub plugin { my $id = $input->param('id'); my $delete = $input->param('delete'); my $uploaded_file = $input->param('uploaded_file'); - - my $template_name = ($id || $delete) - ? "upload_delete_file.tt" - : "upload.tt"; + my $from_popup = $input->param('from_popup'); + my $isfileuploadurl = $input->param('IsFileUploadUrl'); + my $dangling = C4::UploadedFiles::DanglingEntry($id,$isfileuploadurl); + my $template_name; + if ($delete || ($id && ($dangling==0 || $dangling==1))) { + $template_name = "upload_delete_file.tt"; + } + else { + $template_name = "upload.tt"; + } my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { template_name => "cataloguing/value_builder/$template_name", @@ -79,6 +90,10 @@ sub plugin { } ); + if ($dangling==2) { + $template->param( dangling => 1 ); + } + # Dealing with the uploaded file my $dir = $input->param('dir'); if ($uploaded_file and $dir) { @@ -97,14 +112,14 @@ sub plugin { } else { $template->param(error => 1); } - } elsif ($delete || $id) { + } elsif ($delete || ($id && ($dangling==0 || $dangling==1))) { # If there's already a file uploaded for this field, # We handle its deletion if ($delete) { - if(C4::UploadedFiles::DelUploadedFile($id)) {; - $template->param(success => 1); - } else { + if(C4::UploadedFiles::DelUploadedFile($id)==0) {; $template->param(error => 1); + } else { + $template->param(success => 1); } } } else { @@ -129,14 +144,21 @@ sub plugin { $template->param( error_upload_path_not_configured => 1 ); } + if (!$uploaded_file && !$dir && $from_popup) { + $template->param(error_nothing_selected => 1); + } + elsif (!$uploaded_file && $dir) { + $template->param(error_no_file_selected => 1); + } if ($uploaded_file and not $dir) { $template->param(error_no_dir_selected => 1); } + } $template->param( index => $index, - id => $id + id => $id, ); output_html_with_http_headers $input, $cookie, $template->output; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 269531a5d5..1210ca49b3 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -3364,7 +3364,7 @@ CREATE TABLE IF NOT EXISTS `borrower_modifications` ( -- Table structure for table uploaded_files -- -DROP TABLE IF EXISTS uploaded_files +DROP TABLE IF EXISTS uploaded_files; CREATE TABLE uploaded_files ( id CHAR(40) NOT NULL PRIMARY KEY, filename TEXT NOT NULL, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt index 63849f8d68..2f002af676 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt @@ -7,18 +7,32 @@ @@ -73,17 +87,28 @@

Configuration variable 'upload_path' is not configured.

Please configure it in your koha-conf.xml

[% ELSE %] + [% IF (error_nothing_selected) %] +

Error: You have to choose the file to upload and select where to upload the file.

+ [% END %] + [% IF (error_no_file_selected) %] +

Error: You have to choose the file to upload.

+ [% END %] [% IF (error_no_dir_selected) %] -

Error: You have to select the destination of uploaded file.

+

Error: You have to select where to upload the file.

+ [% END %] + [% IF (dangling) %] +

Error: The URL has no file to retrieve.

[% END %]

Please select the file to upload :

-
+ [% filefield %] -

Choose where to upload file

+

Choose where to upload the file

[% INCLUDE list_dirs dirs = dirs_tree %] + - + +
[% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt index 7817bbedd4..928b954b79 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt @@ -4,7 +4,7 @@ Upload plugin - +