From baa734102ff20fd90ceaeb47c02c7a68e1802bb8 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 12 Jun 2013 13:12:51 +0000 Subject: [PATCH] Bug 6874: Move uploadPath syspref to koha-conf.xml --- C4/UploadedFiles.pm | 6 ++-- cataloguing/value_builder/upload.pl | 38 +++++++++++--------- etc/koha-conf.xml | 1 + installer/data/mysql/sysprefs.sql | 1 - installer/data/mysql/updatedatabase.pl | 9 +---- .../en/modules/admin/preferences/cataloguing.pref | 4 -- .../en/modules/cataloguing/value_builder/upload.tt | 24 ++++++++----- t/db_dependent/UploadedFiles.t | 8 ++--- t/lib/Mocks/Context.pm | 24 ++++++++++++- 9 files changed, 68 insertions(+), 47 deletions(-) diff --git a/C4/UploadedFiles.pm b/C4/UploadedFiles.pm index 1574e99..1d72015 100644 --- a/C4/UploadedFiles.pm +++ b/C4/UploadedFiles.pm @@ -61,7 +61,7 @@ use C4::Context; sub _get_file_path { my ($id, $dirname, $filename) = @_; - my $uploadPath = C4::Context->preference('uploadPath'); + my $uploadPath = C4::Context->config('uploadPath'); my $filepath = "$uploadPath/$dirname/${id}_$filename"; $filepath =~ s|/+|/|g; @@ -81,7 +81,7 @@ Hash keys are: =item * filename: name of the file -=item * dir: directory where file is stored (relative to syspref 'uploadPath') +=item * dir: directory where file is stored (relative to config variable 'uploadPath') =back @@ -123,7 +123,7 @@ Parameters: =item * $filename: name of the file -=item * $dir: directory where to store the file (path relative to syspref 'uploadPath' +=item * $dir: directory where to store the file (path relative to config variable 'uploadPath' =item * $io_handle: valid IO::Handle object, can be retrieved with $cgi->upload('uploaded_file')->handle; diff --git a/cataloguing/value_builder/upload.pl b/cataloguing/value_builder/upload.pl index 7eb15e5..f415464 100755 --- a/cataloguing/value_builder/upload.pl +++ b/cataloguing/value_builder/upload.pl @@ -108,22 +108,26 @@ sub plugin { } } } else { - my $filefield = CGI::filefield( - -name => 'uploaded_file', - -size => 50, - ); - - my $upload_path = C4::Context->preference('uploadPath'); - my $dirs_tree = [ { - name => '/', - value => '/', - dirs => finddirs($upload_path) - } ]; - - $template->param( - dirs_tree => $dirs_tree, - filefield => $filefield - ); + my $upload_path = C4::Context->config('uploadPath'); + if ($upload_path) { + my $filefield = CGI::filefield( + -name => 'uploaded_file', + -size => 50, + ); + + my $dirs_tree = [ { + name => '/', + value => '/', + dirs => finddirs($upload_path) + } ]; + + $template->param( + dirs_tree => $dirs_tree, + filefield => $filefield + ); + } else { + $template->param( error_upload_path_not_configured => 1 ); + } } $template->param( @@ -137,7 +141,7 @@ sub plugin { # Build a hierarchy of directories sub finddirs { my $base = shift; - my $upload_path = C4::Context->preference('uploadPath'); + my $upload_path = C4::Context->config('uploadPath'); my $found = 0; my @dirs; my @files = glob("$base/*"); diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index 22529ac..162e6ca 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -278,6 +278,7 @@ __PAZPAR2_TOGGLE_XML_POST__ 1 __PLUGINS_DIR__ 0 + __INTRANET_CGI_DIR__ __OPAC_CGI_DIR__/opac __OPAC_TMPL_DIR__ diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index e752f38..fbd0387 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -427,4 +427,3 @@ INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES(' INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES('UseCourseReserves', '0', 'Enable the course reserves feature.', NULL, 'YesNo'); INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES('OpacHoldNotes',0,'Show hold notes on OPAC','','YesNo'); INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('CalculateFinesOnReturn','1','Switch to control if overdue fines are calculated on return or not', '', 'YesNo'); -INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('uploadPath','','Sets the upload path for the upload.pl plugin. For security reasons, the upload path MUST NOT be a public, webserver accessible directory.','',''); diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 82f2053..fb9856d 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -6994,11 +6994,6 @@ if ( CheckVersion($DBversion) ) { $DBversion = "XXX"; if ( CheckVersion($DBversion) ) { $dbh->do(" - INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) - VALUES('uploadPath','','Sets the upload path for the upload.pl plugin','',''); - "); - - $dbh->do(" CREATE TABLE uploaded_files ( id CHAR(40) NOT NULL PRIMARY KEY, filename TEXT NOT NULL, @@ -7007,8 +7002,8 @@ if ( CheckVersion($DBversion) ) { "); print "Upgrade to $DBversion done (Bug 6874: New cataloging plugin upload.pl)\n"; - print "This plugin comes with a new syspref (uploadPath) and a new table (uploaded_files)\n"; - print "To use it, set 'uploadPath' and 'OPACBaseURL' system preferences and link this plugin to a subfield (856\$u for instance)\n"; + print "This plugin comes with a new config variable (uploadPath) and a new table (uploaded_files)\n"; + print "To use it, set 'uploadPath' config variable and 'OPACBaseURL' system preference and link this plugin to a subfield (856\$u for instance)\n"; SetVersion($DBversion); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref index 39e414f..96f3603 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref @@ -109,10 +109,6 @@ Cataloging: - pref: UNIMARCField100Language class: short - as default language in the UNIMARC field 100 when creating a new record or in the field plugin. - - - - Absolute path where to store files uploaded in MARC record (plugin upload.pl) - - pref: uploadPath - class: multi Display: - - 'Separate multiple displayed authors, series or subjects with ' 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 da40360..10f64f1 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 @@ -54,15 +54,21 @@ [% END %] [% END %] -

Please select the file to upload :

-
- [% filefield %] -

Choose where to upload file

- [% INCLUDE list_dirs dirs = dirs_tree %] - - - -
+ [% IF (error_upload_path_not_configured) %] +

Configuration error

+

Configuration variable 'uploadPath' is not configured.

+

Please configure it in your koha-conf.xml

+ [% ELSE %] +

Please select the file to upload :

+
+ [% filefield %] +

Choose where to upload file

+ [% INCLUDE list_dirs dirs = dirs_tree %] + + + +
+ [% END %] [% END %] [% END %] diff --git a/t/db_dependent/UploadedFiles.t b/t/db_dependent/UploadedFiles.t index 36605d2..ee8a82c 100644 --- a/t/db_dependent/UploadedFiles.t +++ b/t/db_dependent/UploadedFiles.t @@ -5,6 +5,8 @@ use File::Temp qw/ tempdir /; use Test::CGI::Multipart; use Test::More tests => 11; +use t::lib::Mocks::Context; + use C4::Context; use C4::UploadedFiles; @@ -17,10 +19,8 @@ $tcm->upload_file( ); my $cgi = $tcm->create_cgi; -# Save the value of uploadPath and set it to a temporary directory -my $uploadPath = C4::Context->preference('uploadPath'); my $tempdir = tempdir(CLEANUP => 1); -C4::Context->set_preference('uploadPath', $tempdir); +t::lib::Mocks::Context::mock_config('uploadPath', $tempdir); my $testfilename = $cgi->param('testfile'); my $testfile_fh = $cgi->upload('testfile'); @@ -38,7 +38,5 @@ ok(-e $file->{filepath}, "File $file->{filepath} exists"); ok(C4::UploadedFiles::DelUploadedFile($id), "DelUploadedFile($id) returned true"); ok(! -e $file->{filepath}, "File $file->{filepath} does not exist anymore"); -C4::Context->set_preference('uploadPath', $uploadPath); - is(C4::UploadedFiles::UploadFile($testfilename, '../', $testfile_fh->handle), undef, 'UploadFile with $dir containing ".." return undef'); is(C4::UploadedFiles::GetUploadedFile(), undef, 'GetUploadedFile without parameters returns undef'); diff --git a/t/lib/Mocks/Context.pm b/t/lib/Mocks/Context.pm index 185209a..28939ef 100644 --- a/t/lib/Mocks/Context.pm +++ b/t/lib/Mocks/Context.pm @@ -1,5 +1,7 @@ package t::lib::Mocks::Context; -use t::lib::Mocks::Context; + +use Modern::Perl; +use Test::MockModule; use C4::Context; sub MockPreference { @@ -10,4 +12,24 @@ sub MockPreference { $mock_object->mock('preference', sub { &t::lib::Mocks::Context::MockPreference( @_, $value, $mock_object ) }); return $r; } + +our %config; +sub mock_config { + my ($variable, $value) = @_; + + $config{$variable} = $value; + + my $module = new Test::MockModule('C4::Context'); + unless ($module->is_mocked('config')) { + $module->mock('config', sub { + my ($self, $variable) = @_; + if (exists $config{$variable}) { + return $config{$variable}; + } else { + return $module->original('config')->($self, $variable); + } + }); + } +} + 1; -- 1.7.2.5