Attachment 23613 Details for Bug 6874 – Bug 6874 - kohastructure.sql, jquery.js, refocus, and more

[patch] Bug 6874 - kohastructure.sql, jquery.js, refocus, and more

Bug-6874---kohastructuresql-jqueryjs-refocus-and-m.patch (text/plain), 14.65 KB, created by Mark Tompsett on 2013-12-17 20:34:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Tompsett

Created: 2013-12-17 20:34:55 UTC

Size: 14.65 KB

patch

obsolete

>From 9ed00462b36fc34e1104dedd9393a64dac9844cf Mon Sep 17 00:00:00 2001
>From: Mark Tompsett <mtompset@hotmail.com>
>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             |    3 +-
> .../en/modules/cataloguing/value_builder/upload.tt |   55 +++++++---
> .../value_builder/upload_delete_file.tt            |    2 +-
> t/db_dependent/UploadedFiles.t                     |   14 ++-
> 6 files changed, 174 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();
> 
>         }
>     </script>
>@@ -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 400e7f7..d6972b4 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -3275,12 +3275,13 @@ CREATE TABLE IF NOT EXISTS `borrower_modifications` (
>   PRIMARY KEY (`verification_token`,`borrowernumber`),
>   KEY `verification_token` (`verification_token`),
>   KEY `borrowernumber` (`borrowernumber`)
>+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
> 
> --
> -- 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 @@
>     <script type="text/javascript" src="[% interface %]/lib/jquery/jquery.js"></script>
>     <link rel="stylesheet" type="text/css" href="[% themelang %]/css/staff-global.css" />
>     <script type="text/javascript">
>-      function _(s) { return s; }
>-      $(document).ready(function() {
>+      function ValidateForm() {
>+        var filename = document.forms["UploadForm"]["uploaded_file"].value;
>+        var selected = 0;
>+        var value;
>         $('form').each(function() {
>-          $(this).submit(function() {
>-            var value = $(this).find('input[type="radio"][name="dir"]:checked').val();
>-            if (!value) {
>-              alert(_("Please select the destination of file"));
>-              return false;
>-            }
>-          });
>-        })
>-      });
>+          value = $(this).find('input[type="radio"][name="dir"]:checked').val();
>+          if (value) {
>+            selected = 1;
>+          }
>+        });
>+        if (!filename && !selected) {
>+          alert("Please select a file and its destination.");
>+          return false;
>+        }
>+        else if (!filename) {
>+          alert("Please select a file to upload.");
>+          return false;
>+        }
>+        else if (!selected) {
>+          alert("Please select a file destination.");
>+          return false;
>+        }
>+        else {
>+          return true;
>+        }
>+      }
>     </script>
> 
> </head>
>@@ -73,17 +87,28 @@
>           <p>Configuration variable 'upload_path' is not configured.</p>
>           <p>Please configure it in your koha-conf.xml</p>
>         [% ELSE %]
>+          [% IF (error_nothing_selected) %]
>+              <p class="error">Error: You have to choose the file to upload and select where to upload the file.</p>
>+          [% END %]
>+          [% IF (error_no_file_selected) %]
>+              <p class="error">Error: You have to choose the file to upload.</p>
>+          [% END %]
>           [% IF (error_no_dir_selected) %]
>-              <p class="error">Error: You have to select the destination of uploaded file.<p>
>+              <p class="error">Error: You have to select where to upload the file.</p>
>+          [% END %]
>+          [% IF (dangling) %]
>+              <p class="error">Error: The URL has no file to retrieve.</p>
>           [% END %]
>           <h2>Please select the file to upload : </h2>
>-          <form method="post" enctype="multipart/form-data" action="/cgi-bin/koha/cataloguing/plugin_launcher.pl">
>+          <form name="UploadForm" method="post" enctype="multipart/form-data" action="/cgi-bin/koha/cataloguing/plugin_launcher.pl" onsubmit="return ValidateForm()">
>               [% filefield %]
>-              <h3>Choose where to upload file</h3>
>+              <h3>Choose where to upload the file</h3>
>               [% INCLUDE list_dirs dirs = dirs_tree %]
>+              <input type="hidden" name="from_popup" value="1" />
>               <input type="hidden" name="plugin_name" value="upload.pl" />
>               <input type="hidden" name="index" value="[% index %]" />
>-              <input type="submit">
>+              <input type="submit" value="Upload file">
>+              <input type="button" value="Cancel" onclick="window.close();" />
>           </form>
>         [% 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 @@
> <head>
>     <title>Upload plugin</title>
>     <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
>-    <script type="text/javascript" src="[% themelang %]/lib/jquery/jquery.js"></script>
>+    <script type="text/javascript" src="[% interface %]/lib/jquery/jquery.js"></script>
>     <link rel="stylesheet" type="text/css" href="[% themelang %]/css/staff-global.css" />
>     <script type="text/javascript">
>         //<![CDATA[
>diff --git a/t/db_dependent/UploadedFiles.t b/t/db_dependent/UploadedFiles.t
>index ff14317..8e04aa2 100644
>--- a/t/db_dependent/UploadedFiles.t
>+++ b/t/db_dependent/UploadedFiles.t
>@@ -3,7 +3,7 @@
> use Modern::Perl;
> use File::Temp qw/ tempdir /;
> use Test::CGI::Multipart;
>-use Test::More tests => 11;
>+use Test::More tests => 15;
> 
> use t::lib::Mocks;
> 
>@@ -35,7 +35,17 @@ foreach my $key (qw(id filename filepath dir)) {
> 
> ok(-e $file->{filepath}, "File $file->{filepath} exists");
> 
>-ok(C4::UploadedFiles::DelUploadedFile($id), "DelUploadedFile($id) returned true");
>+ok(C4::UploadedFiles::DanglingEntry()==-1, "DanglingEntry() returned -1 as expected.");
>+ok(C4::UploadedFiles::DanglingEntry($id)==0, "DanglingEntry($id) returned 0 as expected.");
>+unlink ($file->{filepath});
>+ok(C4::UploadedFiles::DanglingEntry($id)==1, "DanglingEntry($id) returned 1 as expected.");
>+
>+open my $fh,">",($file->{filepath});
>+print $fh "";
>+close $fh;
>+
>+ok(C4::UploadedFiles::DelUploadedFile($id)==1, "DelUploadedFile($id) returned 1.");
>+ok(C4::UploadedFiles::DelUploadedFile($id)==-1, "DelUploadedFile($id) returned -1 as expected.");
> ok(! -e $file->{filepath}, "File $file->{filepath} does not exist anymore");
> 
> is(C4::UploadedFiles::UploadFile($testfilename, '../', $testfile_fh->handle), undef, 'UploadFile with $dir containing ".." return undef');
>-- 
>1.7.9.5

Actions: View | Diff | Splinter Review

Attachments on bug 6874: 5439 | 5440 | 5764 | 7114 | 7980 | 8234 | 8468 | 10664 | 10673 | 10863 | 10982 | 11098 | 11634 | 12355 | 12356 | 12357 | 12358 | 14130 | 14131 | 14132 | 14133 | 16343 | 16354 | 16355 | 16356 | 16357 | 16370 | 16693 | 16694 | 16695 | 16696 | 16697 | 16698 | 16699 | 16712 | 16713 | 16714 | 16715 | 16716 | 16717 | 16718 | 16719 | 17368 | 17369 | 18767 | 18768 | 18769 | 18770 | 18771 | 18772 | 18773 | 18774 | 18775 | 18776 | 18777 | 18891 | 18892 | 18893 | 18894 | 20059 | 20060 | 20779 | 21432 | 21433 | 21434 | 21435 | 21436 | 21489 | 21543 | 21546 | 21635 | 21798 | 21799 | 21800 | 21801 | 21802 | 21803 | 23588 | 23589 | 23590 | 23591 | 23607 | 23608 | 23609 | 23610 | 23611 | 23612 | 23613 | 26418 | 26419 | 26420 | 26421 | 26422 | 26423 | 26424 | 26878 | 26879 | 26880 | 26881 | 26882 | 26883 | 26884 | 27114 | 27125 | 27154 | 27155 | 27225 | 30362 | 30363 | 30364 | 30365 | 30366 | 30367 | 30368 | 30369 | 30370 | 30371 | 32384 | 32385 | 32386 | 32387 | 32388 | 32389 | 32390 | 32391 | 32392 | 32393 | 36198 | 36199 | 36200 | 36201 | 36202 | 36203 | 36204 | 36205 | 36206 | 36207 | 36208 | 36209 | 36210 | 36211 | 36213 | 36214 | 36215 | 36216 | 36217 | 36218 | 36219 | 36220 | 36221 | 36222 | 36223 | 36224 | 36225 | 36226 | 36227 | 36228 | 36229 | 36230 | 36231 | 36232 | 37681 | 37682 | 37683 | 37684 | 37685 | 37686 | 37687 | 37688 | 37689 | 37690 | 37691 | 37692 | 37693 | 37694 | 37696 | 37697 | 37698 | 37699 | 37700 | 37701 | 37702 | 37703 | 37704 | 37705 | 37706 | 38911 | 38912 | 38913 | 38914 | 38915 | 38916 | 38917 | 38918 | 38919 | 38920 | 38921 | 38922 | 38923 | 38924 | 38925 | 38926 | 38927 | 38928 | 38929 | 38930 | 38931 | 38932 | 38933 | 38934 | 38935 | 38936 | 39113 | 39114 | 39115 | 39116 | 39117 | 39118 | 39119 | 39120 | 39121 | 39122 | 39123 | 39626 | 39627 | 39628 | 39629 | 39630 | 39631 | 39632 | 39633 | 39634 | 39635 | 39636 | 39637 | 39638 | 41275 | 41276 | 41277 | 41278 | 41279 | 41280 | 41281 | 41282 | 41283 | 41284 | 41285 | 41286 | 41287 | 41288 | 41448 | 41449 | 41450 | 41783