From 1a12cfd6d80bee6b70785b6d0b52f899e824f859 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 --- 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 e11f97a..7d295a4 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -78,7 +78,6 @@ BEGIN { &GetTitles &GetPatronImage - &PutPatronImage &RmPatronImage &GetHideLostItemsPreference @@ -1917,26 +1916,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 ef22fe1..d3e4a08 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 @@ -68,6 +68,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 8afccf6..ea058dc 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.1.4