Bugzilla – Attachment 26884 Details for
Bug 6874
Attach files to bibliographic records
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[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 2014-04-07 19:43:49 UTC
(
hide
)
Description:
Bug 6874 - kohastructure.sql, jquery.js, refocus, and more
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-04-07 19:43:49 UTC
Size:
14.65 KB
patch
obsolete
>From 85c0539f52b4567cf668f3a65c3216ee4c8f8465 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 72489ea..6655375 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -3265,12 +3265,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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
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