Bug 7643 - Can't upload and import zipped file of patron images
Summary: Can't upload and import zipped file of patron images
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal
Assignee: Ivan Brown
QA Contact: Mason James
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-05 15:35 UTC by Owen Leonard
Modified: 2013-12-05 20:01 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
proposed patch (954 bytes, patch)
2012-09-25 15:16 UTC, Ivan Brown
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7643 - Can't upload and import zipped file of patron images (1.02 KB, patch)
2012-09-28 13:00 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2012-03-05 15:35:50 UTC
As reported on the mailing list, import of zipped patron images is not working. I created a zip file my patron images and a text file with data looking like this:

0056464,patron1.jpg
0077822,patron2.jpg
0019166,patron3.jpg
0013890,patron4.jpg

I tried comma-separated and tab-separated. I have confirmed that the text file is plain text. I've tried using "DATALINK.TXT" and "IDLINK.TXT." Each time I get the same error:

Patron Image Failed to Upload

    Unpacking completed
    1 directorie(s) scanned.
    1 directorie(s) processed.

    - Cardnumber:
    WARNING: This image not imported because the cardnumber and filename is missing.

In the log:

picture-upload.pl: Missing cardnumber and filename at /kohaclone/tools/picture-upload.pl line 296, <fh00001patron-images.zip> line 112., referer: http://localhost:8888/cgi-bin/koha/tools/picture-upload.pl
Comment 1 Liz Rea 2012-03-05 15:40:28 UTC
I confirm this bug - a slight variation, I had a single image, single patron in similar format to Owen's.

Mine came back

Unpacking completed
4 directorie(s) scanned.
4 directorie(s) processed.

Now, I didn't have 4 directories in my zip file, very odd.
Comment 2 Ata ur Rehman 2012-03-20 18:06:29 UTC
Dear All

I'm also getting the same error in Koha 3.6.4

Ata ur Rehman
LISolutions
Comment 3 Albert Oller 2012-06-19 15:58:22 UTC
For 3.6 I found that unzip hadn't been installed on a site.  Once I installed that I got errors.  I installed an earlier version of picture-upload.pl and that resolved the problem.

http://git.koha-community.org/gitweb/?p=koha.git;a=blob_plain;f=tools/picture-upload.pl;hb=44201a54e15f06d13c2f40d93bd5a76f6493d7d2

I'm seeing a  similar problem on 3.8, and am going to test that now.
Comment 4 Albert Oller 2012-06-19 16:58:58 UTC
Hi, 

same problem with 3.8

missing unzip fixed with : sudo apt-get install unzip

current 3.8 picture-upload.pl gives this error:

Patron image failed to upload

    Unpacking completed
    1 directories scanned.
    1 directories processed.

    - Cardnumber:
    WARNING: This image not imported because the cardnumber and filename is missing.

installed the older picture-upload.pl and it now works fine.


Here's the diff of the two:  (picture-upload.pl.orig = 3.8 version)

kohaclone/tools$ diff picture-upload.pl picture-upload.pl.orig 
61c61
< picture-upload.p. - Script for handling uploading of both single and bulk patronimages and importing them into the database.
---
> picture-upload.pl - Script for handling uploading of both single and bulk patronimages and importing them into the database.
81c81,84
<     my $filesuffix = $1 if $uploadfilename =~ m/(\..+)$/i;
---
>     my $filesuffix;
>     if ( $uploadfilename =~ m/(\..+)$/i ) {
>         my $filesuffix = $1;
>     }
98c101
<             unless (system("unzip $tempfile -d $dirname") == 0) {
---
>             unless (system("unzip", $tempfile,  '-d', $dirname) == 0) {
257,258c260,264
< 				my $dberror = PutPatronImage($cardnumber,$mimetype, $imgfile) if $mimetype;
< 				if ( !$dberror && $mimetype ) { # Errors from here on are fatal only to the import of a particular image, so don't bail, just note the error and keep going
---
>                 my $dberror;
>                 if ($mimetype) {
>                     $dberror = PutPatronImage( $cardnumber, $mimetype, $imgfile );
>                 }
>                 if ( !$dberror && $mimetype ) { # Errors from here on are fatal only to the import of a particular image, so don't bail, just note the error and keep going
298,299d303
< 
< =back
Comment 5 Ivan Brown 2012-09-25 11:21:11 UTC
I think I have managed to fix this.

In the new code:

my $filesuffix;
if ( $uploadfilename =~ m/(\..+)$/i ) {
    my $filesuffix = $1;
}

$filesuffix is being declared as local (with my) twice.

If the second my is deleted, the upload works.
Comment 6 Chris Nighswonger 2012-09-25 12:04:38 UTC
(In reply to comment #5)
> I think I have managed to fix this.
> 
> In the new code:
> 
> my $filesuffix;
> if ( $uploadfilename =~ m/(\..+)$/i ) {
>     my $filesuffix = $1;
> }
> 
> $filesuffix is being declared as local (with my) twice.
> 
> If the second my is deleted, the upload works.

Good catch indeed! And here is the commit which broke it:

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=b5b71339f3e3dfd639609ecc4b6912435d29704b

Maybe you would consider submitting a patch to fix this bug? In return, you'll get your name added to the list of those who make Koha what it is.
Comment 7 Ivan Brown 2012-09-25 15:16:50 UTC Comment hidden (obsolete)
Comment 8 Chris Nighswonger 2012-09-25 15:46:39 UTC
Updating status
Comment 9 Kyle M Hall (khall) 2012-09-28 13:00:42 UTC
Created attachment 12577 [details] [review]
[SIGNED-OFF] Bug 7643 - Can't upload and import zipped file of patron images

Fixed problem with re-declaration of $filesuffix

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Mason James 2012-10-11 07:44:25 UTC
(In reply to comment #9)
> Created attachment 12577 [details] [review]
> [SIGNED-OFF] Bug 7643 - Can't upload and import zipped file of patron images
> 
> Fixed problem with re-declaration of $filesuffix
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

patch looks good, passing QA...
thanks for the patch Ivan! :p

$ koha-qa.pl -c 1

testing 1 commit(s) (applied to commit 0c75d66)
 * 7ab41ad Bug 7643 - Can't upload and import zipped file of patron images
      tools/picture-upload.pl

* tools/picture-upload.pl                                                  OK
Comment 11 Paul Poulain 2012-10-12 13:43:30 UTC
Patch pushed to master
Comment 12 Chris Cormack 2012-10-14 07:23:21 UTC
Pushed to 3.8.x will be in 3.8.6.

Thanks Ivan and welcome to the Koha contributors family!