http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> 2011-09-16 10:32:12 UTC --- Created attachment 5439 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5439 Patch Plugin for uploading files and link it into 856$u field -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #3 from Julian Maurice <julian.maurice@biblibre.com> 2011-09-16 10:42:17 UTC --- To test: - Create the directory where uploaded files will go mkdir koha-tmpl/uploads - Set the 2 sysprefs uploadPath: "KOHA_ROOT/koha-tmpl/uploads" uploadWebPath: "/uploads" - Set the MARC framework to use "upload.pl" plugin for 856$u - Modify an existing biblio and use the plugin on this field - Save the modified biblio, verify that the url is well displayed at catalogue/detail.pl and try to download the file. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 |PATCH-Sent Patch Status|--- |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |robin@catalyst.net.nz --- Comment #6 from Robin Sheat <robin@catalyst.net.nz> 2011-09-18 23:28:17 UTC --- A couple of things: How will this handle installations that have multiple instances? I'm quite wary of allowing files to be uploaded into a webserver accessible directory, it seems there's too much that can go wrong unless you're really careful (whitelisting extensions, etc.) My preferences would be an out-of-server-tree location, and download requests go through a program that does the actual transfer. The upload.pl has no copyright header, or useful perldoc. "plugin_parameters : other parameters added when the plugin is called by the dopop function" is meaningless. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #5440|0 |1 is obsolete| | --- Comment #7 from Julian Maurice <julian.maurice@biblibre.com> 2011-10-07 10:20:25 UTC --- Created attachment 5764 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5764 Patch New rebased patch with some fixes. @Sébastien: 1/ Fixed. 2/ If you are talking about this line: my $template_name = $result || $delete ? "upload_delete_file.tt" : "upload.tt"; The '||' operator have higher precedence than '?' so in reality it's equivalent to my $template_name = ($result || $delete) ? "upload_delete_file.tt" : "upload.tt"; I added parentheses to avoid confusion 3/ '../' strings are now removed from filename ($result) 4/ Added chmod 0644 on created files to avoid script execution, and removed leading dot on filename to avoid uploading files like .htaccess 5/ err... copy/paste error. The second elsif condition is 'subfiel.is_file' @Robin: Not sure to understand your first question. What do you mean when you say "multiple instances"? You can set uploadPath to be external to the server tree location, and change the apache configuration to add a directory alias. Added copyright header and a brief perldoc section. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

Multiple instances is when you're running more than one koha instance on a server.

Having the files accessible directly through apache strikes me as problematic. Adding an alias doesn't solve that really, and I don't think chmod 0644 will either. For a simple example, what if someone uploads a PHP file and you have mod_php enabled? can't it be solved just by an apache config (like "noexec" or something like

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #9 from Paul Poulain <paul.poulain@biblibre.com> 2011-10-13 13:16:11 UTC --- (In reply to comment #8) that ?)

The patch has a lot of 'warn's in it, looks like debug code.

Agreed, Julian, please update unconditional warns to "$debug and warn"

Most of the functions are undocumented, which is bad. If you're speaking of upload.pl, I fully agree, Julian, please add some documentation to this script.

Putting HTML/Javascript in .pl files is something that should really really be avoided, it's violating separation of concerns (I know that other parts of Koha have that, that upsets me too :) It should be pushed into a template.

If you speak of upload.pl, it's how it's made for all plugins and I don't think this should be changed for this one. I agree we should improve the cataloguing plugin system (& probably completely rewrite the cataloguing tool in fact), but that's outside the scope of this patch. For consistency reason I think upload.pl should continue to have this structure & embeed code & html & javascript.

I note that you search through to find a free filename. I can't tell what the base name will be (or perhaps it's provided by the uploader), but that a) risks a (very unlikely) race condition, and b) could get slow if there are many files. Perhaps include a timestamp as well? I don't understand what you mean here, so won't say anything.

If the OPAC is set so that it requires a valid login to access, then this will still allow those files to be accessible, which is bad. True, but how could we avoid this ? should we obfuscate all files URL by providing a pass-through perl script? Something like : <a href="/cgi-bin/koha/opac-retrieve.pl?filename=blabla">Download file</a> and opac-retrieve.pl: 1- check for Opac requiring login 2- send the file

Note that is would also solve your php uploaded concern right ? (Julian : if Robin confirms, ask stephane_n for what he did for Electre, it's that kind of thing) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|master |Rel_3_8 --- Comment #11 from Paul Poulain <paul.poulain@biblibre.com> 2011-10-24 11:38:09 UTC --- Updating Version : This ENH will be for Koha 3.8 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire.hernandez@biblibre.c | |om Patch Status|Needs Signoff |Does not apply --- Comment #13 from claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> 2011-12-02 13:41:33 UTC --- $ git-bz apply 6874 Bug 6874 - File upload in MARC Patch Apply? [yn] y Applying: Bug 6874: Adds the ability to upload a file for a subfield /home/claire/dev/community_repo/koha/.git/rebase-apply/patch:329: trailing whitespace. return $string; /home/claire/dev/community_repo/koha/.git/rebase-apply/patch:555: trailing whitespace. <script type="text/javascript" src="[% yuipath %]/utilities/utilities.js"></script> /home/claire/dev/community_repo/koha/.git/rebase-apply/patch:564: trailing whitespace. var doc = opener.document; /home/claire/dev/community_repo/koha/.git/rebase-apply/patch:611: trailing whitespace. <script type="text/javascript" src="[% yuipath %]/utilities/utilities.js"></script> /home/claire/dev/community_repo/koha/.git/rebase-apply/patch:620: trailing whitespace. var doc = opener.document; error: patch failed: catalogue/detail.pl:104 error: catalogue/detail.pl: patch does not apply error: patch failed: installer/data/mysql/sysprefs.sql:318 error: installer/data/mysql/sysprefs.sql: patch does not apply error: patch failed: kohaversion.pl:16 error: kohaversion.pl: patch does not apply error: patch failed: opac/opac-detail.pl:223 error: opac/opac-detail.pl: patch does not apply Patch failed at 0001 Bug 6874: Adds the ability to upload a file for a subfield When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Patch left in /tmp/Patch-69WGpm.patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #5764|0 |1 is obsolete| | --- Comment #15 from Julian Maurice <julian.maurice@biblibre.com> 2012-01-12 17:10:10 UTC --- Created attachment 7114 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7114 Reviewed patch This patch add a new table 'uploaded_files' where are stored the filename and the md5 of the file as an id. This id is stored in corresponding MARC subfield (so maybe 856$u is not the right place to store it?) and is used to download the file with the new script opac-retrieve-file.pl. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #16 from Robin Sheat <robin@catalyst.net.nz> 2012-02-01 00:29:08 UTC --- That looks good, my only quibble is about the use of MD5. There's no real reason to use it any more, and while it's not a particularly critical situation, it should probably still be avoided. See also: http://marc-stevens.nl/research/md5-1block-collision/ -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7114|0 |1 is obsolete| | --- Comment #18 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 7980 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7980&action=edit File upload in MARC Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly -- What's new: - SHA-1 is now used to build the file hash instead of MD5 - uploadPath syspref is now displayed in the syspref interface (module Cataloging) - documentation added to C4::UploadedFiles - Value in MARC subfield is now the full URL instead of the id (needs OPACBaseURL syspref) - Patch rebased on current master - kohaversion.pl changes removed from the patch - updatedatabase.pl now prints a short explanation on how to make the plugin working - If 'uploadPath' directory contains other directories, a directory tree is displayed to allow the user to choose in which directory he wants to store his file -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #19 from Frère Sébastien Marie <semarie-koha@latrappe.fr> --- Hi, I only do a "visual review" of the code, but just some suggestions... In "C4/UploadedFiles.pm" - line 147:

if( -f $file_path ) { warn "Id $id not in database, but present in filesystem, do nothing"; return; }

Use "-e" instead of "-f": the difference is when $file_path is an existing link. -f say "not a file", but -e "a node exists" (may be a directory, a plain file, a socket, a pipe, a special block device, a symbolic link...) - line 153:

unless(open $out_fh, '>', $file_path) { warn "Failed to open file '$file_path': $!"; return; }

a race condition is possible between the check of file existence and the open of file. You could use sysopen function instead of file-check + open.

use IO::Handle; unless( sysopen($out_fh, $path, O_WRONLY|O_CREAT|O_EXCL) ) { warn "Failed to open file '$file_path': $!"; return; }

note: the file is created only if don't exist, due to O_CREAT|O_EXCL. There is not time between check and open because all is done by system kernel. In ".../cataloguing/value_builder/upload.tt" (and others templates): please use "html" filter. It is (generally) always to use it, a prevent XSS (here only "uploaded_file" seems to be build with user-input)

The file [% uploaded_file | html %] has been successfully uploaded.

In the syspref message, perhaps it should be added that the uploadPath MUST NOT be a public directory, accessible from the webserver (because else, the server could be exposed to arbitrary code execution by evil uploader). Else, the patch seems promising. Thanks -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7980|0 |1 is obsolete| | --- Comment #21 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 8234 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8234&action=edit Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly -- What's new: - Use sysopen instead of open - Use html filter on 'uploaded_file' var - Change the syspref description - Rebased on master -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #22 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Julian, I don't think this works right. I have uploadPath set to /tmp, and I get the following result: http://screencast.com/t/x9ihzYSpWC Uploading doesn't seem to work to any directory. Am I missing something? Last time I tested it, the uploading worked, and there weren't all those radio boxes. Regards, Jared -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #24 from Julian Maurice <julian.maurice@biblibre.com> --- Jared, this hierarchy of radio buttons allow you to choose in which sub-directory you want to save your file. It normally displays only directories under uploadPath and where it have permission to write. So you should only have subdirectories of /tmp. I will take a look at this. Pierre, you must enter the absolute path to directory you want to save files. This can be any directory you want, but for security reasons it should not be reachable by webserver (so a directory outside of Koha source tree should be ok) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Matthias Meusburger <matthias.meusburger@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matthias.meusburger@biblibr | |e.com --- Comment #26 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- I've tried to reproduce the bug described by Jared as well, but it did work correctly for me. Using /tmp, I had (root) and the only www-data writable directory (and it's subdirectories). Non www-data writable directories were hidden. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10664|0 |1 is obsolete| | --- Comment #28 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 10673 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10673&action=edit Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |6274 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |mtj@kohaaloha.com --- Comment #31 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #29)

The problem I reported earlier seemed to have something to do with setting the syspref *prior* to the revised patches. I tested it on an installation which had not had the syspref set previously, and it worked great.

QA comment, passing QA... all perl-critic, perl syntax, and tt-template tests pass OK $ koha-qa.pl starting perlcritic-progressive tests... OK starting 'perl -c' syntax tests... OK starting xt/tt_valid.t tests... OK starting xt/author/vaild-template.t tests... OK rebased patch on 60b6c187c46614431f109512c740a3bcf077f342 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #33 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 10982 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10982&action=edit Bug 6874: Follow-up: fix filename encoding To test, try to upload a file with diacritics in its name and retrieve it by clicking on its link in catalogue/detail.pl page. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Frère Sébastien Marie <semarie-koha@latrappe.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #34 from Frère Sébastien Marie <semarie-koha@latrappe.fr> --- I mark the bug "Failed QA" for a security problem (or perhaps two). in C4/UploadedFiles.pm, sub UploadFile, the regex for sanitize "../" is invalid.

if($filename =~ m#/(^|/)\.\.(/|$)# or $dir =~ m#(^|/)\.\.(/|$)#) {

for $filename, a "/" was inserted at the beginning... so a filename like "../foo" is ok. That line should be corrected like:

if($filename =~ m#(^|/)\.\.(/|$)# or $dir =~ m#(^|/)\.\.(/|$)#) {

And a second (possible) problem is using

$filename = decode_utf8($filename); *after* the previous check.

I think it may be possible to pass the "../" check and with decode_utf8 obtain a "../". To be sure, it should be investigated, but it should be ok to have the "decode_utf8" *before* the "../" check. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #37 from Julian Maurice <julian.maurice@biblibre.com> --- The follow-up does not intend to remove accents but to fix encoding. If the name of your uploaded file is the same as the original file, and does not contain "é"-like characters, it's ok. To be sure, you may try to download it. I'm going to submit a patch for the 2nd point. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #40 from Julian Maurice <julian.maurice@biblibre.com> --- At the end, filenames are just filenames. I think this is needed if you want to access uploaded files 'manually' (not with Koha). -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #42 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- (In reply to comment #40)

At the end, filenames are just filenames. I think this is needed if you want to access uploaded files 'manually' (not with Koha).

I thought we switched to using SHA1 as identifier to avoid clobbering files with the same name? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11098|0 |1 is obsolete| | --- Comment #44 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 12356 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12356&action=edit Bug 6874: Follow-up: fix filename encoding and sanitization Rebased on master -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #46 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 12358 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12358&action=edit Bug 6874: Change filename construction method. Use <SHA-1>_<filename>. SHA-1 to avoid name clash, filename to make it easier to find a file by its name, when not using Koha (direct filesystem access). -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #47 from Julian Maurice <julian.maurice@biblibre.com> --- Jared, I finally choose to use both SHA-1 and name for filename. So we have the advantages of both sides, I think. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12356|0 |1 is obsolete| | --- Comment #49 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 14131 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14131&action=edit Bug 6874: Follow-up: fix filename encoding and sanitization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12358|0 |1 is obsolete| | --- Comment #51 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 14133 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14133&action=edit Bug 6874: Change filename construction method. Use <SHA-1>_<filename>. SHA-1 to avoid name clash, filename to make it easier to find a file by its name, when not using Koha (direct filesystem access). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16343|0 |1 is obsolete| | --- Comment #53 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16354 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16354&action=edit Bug 6874: Follow-up: fix filename encoding and sanitization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #55 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16356 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16356&action=edit Bug 6874: Change filename construction method. Use <SHA-1>_<filename>. SHA-1 to avoid name clash, filename to make it easier to find a file by its name, when not using Koha (direct filesystem access). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA QA Contact|koha-bugs@lists.koha-commun |paul.poulain@biblibre.com |ity.org | --- Comment #57 from Paul Poulain <paul.poulain@biblibre.com> --- QA comment: sorry, another failed QA, because of koha-qa/perlcritic complaining: * opac/opac-retrieve-file.pl FAIL critic FAIL Bareword file handle opened at line 35, column 5. See pages 202,204 of PBP. * cataloguing/value_builder/upload.pl FAIL critic FAIL Glob written as <...> at line 143, column 17. See page 167 of PBP. However, i've uploaded rebased patches with a 3rd perlcritic error fixed, I let you fix those 2. Once follow-up uploaded, switch back to signed off -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #58 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 16370 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16370&action=edit Bug 6874: Follow-up for PBP fixes -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14130|0 |1 is obsolete| | Attachment #16354|0 |1 is obsolete| | Attachment #16355|0 |1 is obsolete| | Attachment #16356|0 |1 is obsolete| | Attachment #16357|0 |1 is obsolete| | Attachment #16370|0 |1 is obsolete| | --- Comment #59 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16693 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16693&action=edit [PASSED QA] Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #61 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16695 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16695&action=edit [PASSED QA] Bug 6874: Allow to reupload a file immediately after deletion This patch adds a button "Upload a new file" after the "sucessful deletion" message to avoid user to close and reopen the window. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #63 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16697 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16697&action=edit [PASSED QA] QA follow-up return with explicit undef removed Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> http://bugs.koha-community.org/show_bug.cgi?id=6874 Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #65 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16699 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16699&action=edit bug 6874 follow-up fix POD syntax -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #67 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #66)

Paul, isn't it kinda late where you are? ;)

yep, but I cannot sleep, so I do some QA ;-) QA comment: I've added a follow-up to fix POD, it now passes koha-qa.pl QUESTION : trying to use the feature, I think I made something wrong or there's a problem. Here is what I do: * I've defined the UploadPath syspref to /tmp/bz6874 & created the directory * I've attached "upload.pl" plugin to 856$u * I've opened and edited an existing biblio * I click on the ... (on the right of 856) * I select a file, and put it in root of UploadPath * Koha says "The file logo2.jpg has been successfully uploaded." * I click "close" * i'm back to cataloguing, but the 856$u is empty shouldn't it contain the file I just uploaded ? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #69 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16713 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16713&action=edit [PASSED QA] Bug 6874: Follow-up: fix filename encoding and sanitization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #71 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16715 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16715&action=edit [PASSED QA] Bug 6874: Change filename construction method. Use <SHA-1>_<filename>. SHA-1 to avoid name clash, filename to make it easier to find a file by its name, when not using Koha (direct filesystem access). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #73 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16717 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16717&action=edit [PASSED QA] Bug 6874: Follow-up for PBP fixes Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #75 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16719 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16719&action=edit [PASSED QA] Bug 6874 follow-up jquery has moved The jquery library has been removed from themelang directory and put into /intranet-tmpl/lib/jquery This patch fixes the path -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #78 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #77)

and at first glance does not look Plack-safe: my $upload_path = C4::Context->preference('uploadPath'); is at a file level, but $upload_path is used in subroutines.

I QAed it using Plack, and it worked well. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #80 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 17368 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17368&action=edit Bug 6874: Add unit tests for C4::UploadedFiles and move a variable declaration at subroutine level instead of file level. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #82 from Julian Maurice <julian.maurice@biblibre.com> --- Oops. Forgot to reset status. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #84 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 18768 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18768&action=edit Bug 6874: Follow-up: fix filename encoding and sanitization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #86 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 18770 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18770&action=edit Bug 6874: Change filename construction method. Use <SHA-1>_<filename>. SHA-1 to avoid name clash, filename to make it easier to find a file by its name, when not using Koha (direct filesystem access). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #88 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 18772 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18772&action=edit Bug 6874: Follow-up for PBP fixes Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #90 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 18774 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18774&action=edit Bug 6874 follow-up jquery has moved The jquery library has been removed from themelang directory and put into /intranet-tmpl/lib/jquery This patch fixes the path Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #92 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 18776 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18776&action=edit Bug 6874: Add dependency to Test::CGI::Multipart Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #94 from Robin Sheat <robin@catalyst.net.nz> --- (In reply to comment #83)

New cataloging plugin upload.pl and new system preference 'uploadPath'. ... uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path

This should not be a system preference. It should be something that's set in the koha-conf.xml. Library staff should not be modifying it, as they can't ensure that directories are set up with the right permissions. If you're running a hosted setup without permission separation, you could upload into someone else's koha space (probably not terrible, but not ideal.) Finally, it's hard to do an automated configuration of a system preference, and it's easy to do one for koha-conf.xml. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #96 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #95)

...

This should not be a system preference. It should be something that's set in the koha-conf.xml. Library staff should not be modifying it, as they can't ensure that directories are set up with the right permissions. This is the same for all other system preferences that contain a path. So why this one is different? At 1st glance, I had the same reaction. But there is a difference : writing ! Other systempreferences only require reading (opacusercss for example), this one will write, and there is no other syspref in this case I think.

Moreover, I don't think a preference for a cataloguing plugin should be in koha-conf.xml (its place is not in global system preferences either but there is no other place) I hesitate... what I think is that this preference don't need to be setup during installation, because it's an additional feature that most libraries won't use. (like for ldap for example : there are default values, but the koha-conf.xml must be edited manually if you want to enable it)

I'm not opposed to move this pref in koha-conf.xml, I just want to understand :) Also I would like to have someone else's opinion before writing a patch.

my conclusion would be: * move the uploadPath to koha-conf.xml * add <uploadPath><uploadPath> (without any value) to default koha-conf * add a disclaimer on the plugin page if uploadPath is empty, saying something like "WARNING : your server is not properly setup, it won't work. uploadPath must be set in koha configuration file" (wait for another feedback before writing the patch) PS: will this patch make it's way one day ? :\ -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18767|0 |1 is obsolete| | Attachment #18768|0 |1 is obsolete| | Attachment #18769|0 |1 is obsolete| | Attachment #18770|0 |1 is obsolete| | --- Comment #98 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 18891 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18891&action=edit [SIGNED-OFF] Bug 6874: File upload in MARC Rebased on master Squash of 4 signed-off patches -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18775|0 |1 is obsolete| | Attachment #18777|0 |1 is obsolete| | --- Comment #100 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 18893 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18893&action=edit [SIGNED-OFF] Bug 6874: Add unit tests for C4::UploadedFiles and move a variable declaration at subroutine level instead of file level. Add dependency to Test::CGI::Multipart Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Still works, and the newly-provided unit tests have good test coverage: C4/UploadedFiles.pm 90.7 65.0 66.7 100.0 100.0 0.2 86.4 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #102 from Julian Maurice <julian.maurice@biblibre.com> --- The last patch introduces a subroutine in t/lib/Mocks/Context.pm that is also introduced by Bug 10298. But there are quite similar and resolving the conflicts will be very easy. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18894|0 |1 is obsolete| | --- Comment #103 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20059 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20059&action=edit Bug 6874: Move uploadPath syspref to koha-conf.xml Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #104 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20060 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20060&action=edit Bug 6874 - File upload in MARC - QA Followup * Renames uploadPath to upload_path to follow the standard naming conventions in koha-conf which use underscores rather than camel case * Remove reference to intranet-tmpl and replace with [% interface %] required to pass qa -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #105 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Unfortunately the fourth patch, "Move uploadPath syspref to koha-conf.xml" will not apply at all because of missing sha1 information for sysprefs.sql. Could you please rebase this patchset and reupload? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #107 from M. Tompsett <mtompset@hotmail.com> --- Okay, I accidentally messed up and got what I think is this working for master. However, I have some concerns. misc/strip_value_from_tag.pl The patches containing this were obsoleted (see first 5 ones, I think). Is this module not useful? t/lib/Mocks/Context.pm This exists in part in the non-obsoleted patches ([SIGNED-OFF] Bug 6874: Move uploadPath syspref to koha-conf.xml), but I think something substituting for t::lib::Mocks would be sufficient based on my reading of the code. 3-5 lines of code are missing in the patch, so I can't reconstruct what I think is the original file. I hunted through all the obsoleted patches too. I would very much prefer the original developer/author to rebase their patch, but am willing to provide what I have reconstructed. I know that my colleagues have asked for this patch, and so I'll make sure to back port to 3.12.x too. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #109 from M. Tompsett <mtompset@hotmail.com> --- As far as I can tell: - configure the koha-conf.xml with the appropriate values - make sure the directories exist and have the correct permissions <pluginsdir>/home/mtompset/koha-dev/var/lib/plugins</pluginsdir> <enable_plugins>1</enable_plugins> <upload_path>/home/mtompset/kohaclone/koha-tmpl/uploads</upload_path> - link 856$u to the upload.pl plugin. - test. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18891|0 |1 is obsolete| | --- Comment #111 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 21432 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21432&action=edit [SIGNED-OFF] Bug 6874: File upload in MARC Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18893|0 |1 is obsolete| | --- Comment #113 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 21434 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21434&action=edit [SIGNED-OFF] Bug 6874: Add unit tests for C4::UploadedFiles Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20060|0 |1 is obsolete| | --- Comment #115 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 21436 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21436&action=edit Bug 6874 - File upload in MARC - QA Followup Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20779|0 |1 is obsolete| | --- Comment #117 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 20779 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20779 Reconstructed attempt for the upload plugin feature. Original author rebased patches. No need for my intermediate attempt. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #119 from M. Tompsett <mtompset@hotmail.com> --- YIKES! " ) ENGINE=InnoDB DEFAULT CHARSET=utf8;" missing from above the uploaded_files table in kohastructure.sql -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #123 from M. Tompsett <mtompset@hotmail.com> --- $ giz bz apply 6874 Had to fix up installer/data/mysql/updatedatabase.pl Had to install new dependency for this. $ koha_perl_deps.pl -m -u Listed new requirement (hence revised PerlDependencies.pm checked) $ sudo apt-get install libtest-cgi-multipart-perl Did the database upgrade by starting staff client. $ prove -v t/db_dependent/UploadedFiles.t everything passed. $ ~/qa-test-tools/koha-qa.pl -v 2 -c 10 (I forgot how many patches were applied, so I just said 10) No problems with this set of patches. Noticed POD updates unrelated to patches (C4::Biblio) Probably required to get C4::Biblio to pass QA tool tests. New files include: C4/UploadedFiles.pm cataloguing/value_builder/upload.pl koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt opac/opac-retrieve-file.pl t/db_dependent/UploadedFiles.t Did a opac-detail view, which means the "use C4::Biblio" isn't broken. There are 1,2,3,4,5,6,6a,6b,7,8,9 changes in C4::Biblio $ perldoc C4::Biblio This triggers 2,7,8,9 Refresh of opac-detail page triggers 3,4 Home -> administration -> Marc frameworks -> BKS framework structure -> tag 856 subfield structure -> edit subfields contraint -> advanced -> plugin = upload.pl Attempted to use plug in without configuring koha-conf.xml Correctly gave feedback. $ mkdir ~/uploads $ sudo chown www-data.www-data ~/uploads add in <upload_path>/home/mtompset/uploads</upload_path> Link a file into some biblio entry. Go to opac detail for it: http://demo.library.kohasystem.ca/cgi-bin/koha/opac-detail.pl?biblionumber=1... Triggers 1,3,4,5 In order to trigger 6, 6a, and 6b, GetMarcUrls needs to be called with something other than MARC21. I couldn't figure out how to do that. I'm attempting a UNIMARC installation. I used https://www.dropbox.com/s/idw98d7jiy5e5xd/dump_unimarc.zip as mentioned by Mathieu Saby in his August 20th email "pending bugs about UNIMARC marc flavor". Linked upload.pl uploaded a dummy file for a known biblio record. went to opac and did an opac detail view of it. (6, 6a) unlinked upload.pl refreshed opac detail page (6, 6b) testing to continue later. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #127 from M. Tompsett <mtompset@hotmail.com> --- I hope that clarifies the complete testing of the C4::Biblio changes that I have done. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #129 from M. Tompsett <mtompset@hotmail.com> --- Fear not. :) I'll be getting to less programmer oriented testing instructions. The programmer oriented instructions are a matter of proving that the code changes introduced do not cause any problems by triggering all the code points. Now, back to testing... -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #131 from M. Tompsett <mtompset@hotmail.com> --- Set up VM from scratch (include qa-test-tools, git-bz [fishsoup branch], koha origin/master branch) $ git checkout -b bug_6874 origin/master $ git bz apply 6874 $ perl Makefile.PL $ make $ make test $ make install Remember to set up some useful data after you get a working Koha. :) Configure patch - edit koha-conf.xml and make sure to fill in <upload_path></upload_path> (e.g. <upload_path>/home/mtompset/uploads</upload_path>) - make sure to give that directory correct permissions (e.g. sudo chown www-data.www-data /home/mtompset/uploads) - attach the plug-in to the 856$u subfield (e.g. - log in to staff client - click 'Koha administration' - click 'MARC bibliographic framework' - click 'MARC structure' for a framework - Find the 856 tag - click 'Subfields' - Find the 'u' subfield - click 'Edit' - click 'Other options (choose one)' - select 'upload.pl' in the Plugin drop-down - click 'Save changes') You will want to test in both MARC21 and UNIMARC systems. $ ./koha_perl_deps -m -u $ perldoc C4::Biblio $ perldoc C4::UploadedFiles This will trigger all the functions in C4::UploadedFiles $ prove -v t/db_dependent/UploadedFiles.t Once you have a bilio with 856$u linked up to a file, go to the same biblio on the OPAC. (e.g. http://demo.library.kohasystem.ca/cgi-bin/koha/opac-detail.pl?biblionumber=1...) Then unlink the upload.pl plugin and refresh the OPAC. This latter step is necessary to test a secondary code point under UNIMARC. In the staff client, find a biblio. Add it to your cart. Click the cart at the top of the page. This will test opac-retrieve-file.pl fully: While at the OPAC detail page for the item added - Click the 'Online resources:' URL. 1) It should work. - Delete the file from the filesystem. (e.g. sudo rm /home/mtompset/uploads/*) - Click the 'Online resources:' URL. 2) It should fail. - Open up MySQL and delete the record from the uploaded_files table. 3) It should fail. - Make sure to clear the 856$u entry for that biblio manually and save. opac-detail.pl change is tested just by viewing the details of the biblio with an 856$u link. opac-basket.pl change is tested by adding the item to your cart and clicking the "Cart" button. To test the template files, it is a matter of doing a lot of upload, delete, upload, whatever type tests in the staff client. This has not been fully tested, but there are some issues that I want to test to the point of being able to say what is wrong and how I was expecting it to be handled. I'm beginning to run short of testing time today again. However, I am pretty confident that I have tested the following thoroughly: C4/Biblio.pm C4/Installer/PerlDependencies.pm C4/UploadedFiles.pm basket/basket.pl etc/koha-conf.xml installer/data/mysql/kohastructure.sql installer/data/mysql/updatedatabase.pl opac/opac-basket.pl opac/opac-detail.pl opac/opac-retrieve-file.pl t/db_dependent/UploadedFiles.t Patches with updatedatabase.pl in them need to be rebased. I have already attached an extra patch to correct issues in kohastructure.sql I will continue my testing later of: catalogue/MARCdetail.pl catalogue/detail.pl cataloguing/value_builder/upload.pl koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload_delete_file.tt -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #136 from M. Tompsett <mtompset@hotmail.com> --- Seeing as I can't sign off on my own patches. Could you squash these into a single patch, re-test, and re-post, Julian? Thanks. I'll continue testing once reposted. :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #138 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 21546 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21546&action=edit Bug 6874 - kohastructure.sql & jquery.js fix with refocus 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 %]. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21543|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #140 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 21635 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21635&action=edit Bug 6874: Force user to select a directory This patch adds a javascript and a server-side checks Test plan: 1/ Choose a file, leave the radios not selected and submit 2/ You have a javascript alert which prevents form to be submitted 3/ Disable javascript and repeat step 1 4/ Form is submitted but form is redisplayed with an error message telling you to choose a directory. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #141 from M. Tompsett <mtompset@hotmail.com> --- First patch needs to be rebased. uploaddatabase.pl patch issues. Still going ahead with testing. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #143 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 21798 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21798&action=edit 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 %]. 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21546|0 |1 is obsolete| | --- Comment #145 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 21546 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21546 Bug 6874 - kohastructure.sql & jquery.js fix with refocus squashed my patches into one on the end. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21800|0 |1 is obsolete| | --- Comment #147 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 21801 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21801&action=edit [SIGNED OFF] Bug 6874 - File upload in MARC - QA Followup * Renames uploadPath to upload_path to follow the standard naming conventions in koha-conf which use underscores rather than camel case * Remove reference to intranet-tmpl and replace with [% interface %] required to pass qa Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21799|0 |1 is obsolete| | --- Comment #149 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 21803 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21803&action=edit 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 %]. 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #152 from Dobrica Pavlinusic <dpavlin@rot13.org> --- I also have conflict in sysprefs: CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #154 from M. Tompsett <mtompset@hotmail.com> --- Shoot... My librarian colleague came up with a scenario that breaks this. 1) Upload and set everything for a 856$u. 2) Clone the subfield. 3) Click the plugin icon. This is where the problem comes in, because now the two subfields you can end up deleting the file for the current 856$u, but the other 856$u still has a reference to the deleted file in it. Any ideas? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #156 from M. Tompsett <mtompset@hotmail.com> --- I have some other related questions: - should validate URLs, since not all 856$u urls will be uploaded file urls. - external urls will generate the same apparent error deleting as the cloning scenario I mentioned in comment #154. - is telling a user to check errors logs reasonable? They may not have shell access to the system. - If a valid external url is detected, then a different response is needed than if we discover a url to a deleted file. - How will the user know to delete the text field before clicking the plugin icon in order to continue onto the upload screen to put a different file in place? The signed off patches need a rebase. Feedback on these questions would be useful. I'm leaving the status at In Discussion, because even if my patch was signed off, the problem with respect to external urls and deleted files, due to a clone, would still exist. And that seems like enough to merit not just running for QA's approval. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #158 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to M. Tompsett from comment #156)

I have some other related questions: - should validate URLs, since not all 856$u urls will be uploaded file urls. Not sure I understand well the question, but isn't misc/cronjobs/check-url-quick.pl what you're asking ?

- external urls will generate the same apparent error deleting as the cloning scenario I mentioned in comment #154. see my previous answer.

- is telling a user to check errors logs reasonable? They may not have shell access to the system. No, it isn't

- If a valid external url is detected, then a different response is needed than if we discover a url to a deleted file. I think check-url-quick.pl achieve the goal, and it's better because it detects when an URL becomes invalid at any time.

- How will the user know to delete the text field before clicking the plugin icon in order to continue onto the upload screen to put a different file in place? I don't understand this one.

The signed off patches need a rebase. Feedback on these questions would be useful. I'm leaving the status at In Discussion, because even if my patch was signed off, the problem with respect to external urls and deleted files, due to a clone, would still exist. And that seems like enough to merit not just running for QA's approval. Are my answer OK and is it back to go to signed-off ? (I have not tested the patches still applies cleanly)

-- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #160 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23588 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23588&action=edit Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #161 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23589 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23589&action=edit [SIGNED OFF] Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #163 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23590 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23590&action=edit [SIGNED OFF] Bug 6874: QA follow-up Squash of the following commits: return with explicit undef removed Follow-up for PBP fixes follow-up fix POD syntax follow-up jquery has moved Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #165 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23591 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23591&action=edit [SIGNED OFF] Bug 6874: Add unit tests for C4::UploadedFiles and move a variable declaration at subroutine level instead of file level. Add dependency to Test::CGI::Multipart Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Still works, and the newly-provided unit tests have good test coverage: C4/UploadedFiles.pm 90.7 65.0 66.7 100.0 100.0 0.2 86.4 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #167 from M. Tompsett <mtompset@hotmail.com> --- I've almost got something nice working. I fixed DelUploadedFile and added DanglingEntry. Hopefully something will be finished in 24 hours. :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23590|0 |1 is obsolete| | --- Comment #169 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23608 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23608&action=edit [SIGNED OFF] Bug 6874: QA follow-up Squash of the following commits: return with explicit undef removed Follow-up for PBP fixes follow-up fix POD syntax follow-up jquery has moved Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #171 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23610 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23610&action=edit [SIGNED OFF] Bug 6874: Move uploadPath syspref to koha-conf.xml Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21802|0 |1 is obsolete| | --- Comment #173 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23612 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23612&action=edit [SIGNED OFF] Bug 6874: Force user to select a directory This patch adds a javascript and a server-side checks Test plan: 1/ Choose a file, leave the radios not selected and submit 2/ You have a javascript alert which prevents form to be submitted 3/ Disable javascript and repeat step 1 4/ Form is submitted but form is redisplayed with an error message telling you to choose a directory. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21435|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23607|0 |1 is obsolete| | --- Comment #176 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26418 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26418&action=edit [SIGNED OFF] Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23609|0 |1 is obsolete| | --- Comment #178 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26420 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26420&action=edit [SIGNED OFF] Bug 6874: Add unit tests for C4::UploadedFiles Add unit tests for C4::UploadedFiles and move a variable declaration at subroutine level instead of file level. Add dependency to Test::CGI::Multipart Still works, and the newly-provided unit tests have good test coverage: C4/UploadedFiles.pm 90.7 65.0 66.7 100.0 100.0 0.2 86.4 Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23611|0 |1 is obsolete| | --- Comment #180 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26422 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26422&action=edit [SIGNED OFF] Bug 6874 - File upload in MARC - QA Followup * Renames uploadPath to upload_path to follow the standard naming conventions in koha-conf which use underscores rather than camel case * Remove reference to intranet-tmpl and replace with [% interface %] required to pass qa Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23613|0 |1 is obsolete| | --- Comment #182 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26424 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26424&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #184 from M. Tompsett <mtompset@hotmail.com> --- Someone asked about how to test and why I have my name on so many things. I thought perhaps posting this might help people find the test plan and some background details.

Also, why many commits have only your sign? Why is your sign at all?

Because I didn’t write all the first part. I tested it, because we wanted the same functionality.

It's also difficult to test something without a clear test plan.

Yes, yes, it is. I started... Comment 109. Comment 123. Katrin makes similar comments regarding the test plan being difficult or lacking in Comment 128. Comment 131. Comment 142. Which all cumulated into what you are looking for: Comment 175 I only wrote the last patch, which is not signed off. And in my mind, it is the only thing stopping this from reaching QA.

Could you add a description of what to test, before and after patch

Before, Koha does not host files, and 856$u needs to point to some external source. After, Koha’s tag editor lets you upload a file and have a URL automatically generated to point to that newly hosted file. I hope this helps someone decide to take the plunge and test this. :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874 --- Comment #186 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26878 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26878&action=edit Bug 6874: File upload in MARC New cataloging plugin upload.pl and new system preference 'uploadPath'. upload.pl provide a way to upload files on the server and store a link (url) to it in MARC uploadPath is the absolute path on the server where the files will be stored. It's possible to have a hierarchy of directories under this path and the plugin will allow to choose in which directory to store the file. Stored value in MARC subfield looks like this: <OPACBaseURL>/cgi-bin/koha/opac-retrieve-file.pl?id=<SHA-1 of the file> So both 'uploadPath' and 'OPACBaseURL' sysprefs have to be set for this plugin to work correctly Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.