From d8980b60ee3144ab8ebf2d3bbf6847c622443425 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 21 Jan 2016 12:32:28 +0000 Subject: [PATCH] Bug 15635: Koha::Patron::Images - Remove PutPatronImage The C4::Members::PutPatronImage inserted/updated the image of a patron. This can be done easily with ->find->set->store or ->new->store Test plan: 1/ Modify the image of a patron from the patron detail page 2/ Add an image to a new patron 3/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add or modify the image of a patron 4/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add or modify the image of several patrons, using a zip file. Stress the script trying to get as many errors as possible (wrong cardnumber, wrong mimetype, file does not exist, etc.) With this patch, if the cardnumber does not exist, you will get a specific error "Image not imported because this patron does not exist in the database" Signed-off-by: Josef Moravec Signed-off-by: Kyle M Hall Signed-off-by: Kyle M Hall --- C4/Members.pm | 21 ------ .../prog/en/modules/tools/picture-upload.tt | 1 + tools/picture-upload.pl | 64 ++++++++++++------- 3 files changed, 41 insertions(+), 45 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index a7f686cce8..220b1ef0b7 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -75,7 +75,6 @@ BEGIN { &GetTitles &GetPatronImage - &PutPatronImage &RmPatronImage &GetHideLostItemsPreference @@ -1850,26 +1849,6 @@ sub GetPatronImage { return $imagedata, $sth->errstr; } -=head2 PutPatronImage - - PutPatronImage($cardnumber, $mimetype, $imgfile); - -Stores patron binary image data and mimetype in database. -NOTE: This function is good for updating images as well as inserting new images in the database. - -=cut - -sub PutPatronImage { - my ($cardnumber, $mimetype, $imgfile) = @_; - warn "Parameters passed in: Cardnumber=$cardnumber, Mimetype=$mimetype, " . ($imgfile ? "Imagefile" : "No Imagefile") if $debug; - my $dbh = C4::Context->dbh; - my $query = "INSERT INTO patronimage (borrowernumber, mimetype, imagefile) VALUES ( ( SELECT borrowernumber from borrowers WHERE cardnumber = ? ),?,?) ON DUPLICATE KEY UPDATE imagefile = ?;"; - my $sth = $dbh->prepare($query); - $sth->execute($cardnumber,$mimetype,$imgfile,$imgfile); - warn "Error returned inserting $cardnumber.$mimetype." if $sth->errstr; - return $sth->errstr; -} - =head2 RmPatronImage my ($dberror) = RmPatronImage($borrowernumber); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt index 9e03a55e1e..3b1305f5e7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt @@ -79,6 +79,7 @@ [% ELSIF ( filerror.OPNERR ) %]ERROR: Image not imported because Koha was unable to open the image for reading. [% ELSIF ( filerror.OVRSIZ ) %]ERROR: Image not imported because the image file is too big (see online help for maximum size). [% ELSIF ( filerror.CRDFIL ) %]ERROR: Image not imported ([% filerror.CRDFIL %] missing). + [% ELSIF ( filerror.CARDNUMBER_DOES_NOT_EXIST ) %]ERROR: Image not imported because this patron does not exist in the database. [% ELSE %]ERROR: Image not imported because of an unknown error. Please refer to the error log for more details. [% END %] [% END %] diff --git a/tools/picture-upload.pl b/tools/picture-upload.pl index 8afccf6dbf..ea058dc021 100755 --- a/tools/picture-upload.pl +++ b/tools/picture-upload.pl @@ -313,31 +313,47 @@ sub handle_file { $debug and warn "Image is of mimetype $mimetype"; 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 - $count{count}++; - push @{ $count{filenames} }, - { source => $filename, cardnumber => $cardnumber }; - } - elsif ($dberror) { - warn "Database returned error: $dberror"; - ( $dberror =~ /patronimage_fk1/ ) - ? $filerrors{'IMGEXISTS'} = 1 - : $filerrors{'DBERR'} = 1; - push my @filerrors, \%filerrors; - push @{ $count{filenames} }, - { - filerrors => \@filerrors, - source => $filename, - cardnumber => $cardnumber - }; - $template->param( ERRORS => 1 ); + my $patron = Koha::Patrons->find({ cardnumber => $cardnumber }); + if ( $patron ) { + my $image = $patron->image; + $image ||= Koha::Patron::Image->new({ borrowernumber => $patron->borrowernumber }); + $image->set({ + mimetype => $mimetype, + imagefile => $imgfile, + }); + eval { $image->store }; + if ( $@ ) { + # 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 + warn "Database returned error: $@"; + $filerrors{'DBERR'} = 1; + push my @filerrors, \%filerrors; + push @{ $count{filenames} }, + { + filerrors => \@filerrors, + source => $filename, + cardnumber => $cardnumber + }; + $template->param( ERRORS => 1 ); + } else { + $count{count}++; + push @{ $count{filenames} }, + { source => $filename, cardnumber => $cardnumber }; + } + } else { + warn "Patron with the cardnumber '$cardnumber' does not exist"; + $filerrors{'CARDNUMBER_DOES_NOT_EXIST'} = 1; + push my @filerrors, \%filerrors; + push @{ $count{filenames} }, + { + filerrors => \@filerrors, + source => $filename, + cardnumber => $cardnumber + }; + $template->param( ERRORS => 1 ); + } } - elsif ( !$mimetype ) { + else { warn "Unable to determine mime type of $filename. Please verify mimetype."; $filerrors{'MIMERR'} = 1; push my @filerrors, \%filerrors; -- 2.39.5