From f8274600933ad3e8c91200e0fe8d8a02cd3d357f 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. --- C4/UploadedFiles.pm | 109 ++++++++++++++++----- cataloguing/value_builder/upload.pl | 42 ++++++-- installer/data/mysql/kohastructure.sql | 2 +- .../en/modules/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 456bf4e..a47b519 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 966f0b3..a2de3eb 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 0405f01..4776e8d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -3351,7 +3351,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 63849f8..2f002af 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 7817bbe..928b954 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 - +