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 <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index a7f686c..220b1ef 100644 (file)
--- 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 9e03a55..3b1305f 100644 (file)
--- 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 ) %]<b>ERROR:</b> Image not imported because Koha was unable to open the image for reading.
                                         [% ELSIF ( filerror.OVRSIZ ) %]<b>ERROR:</b> Image not imported because the image file is too big (see online help for maximum size).
                                         [% ELSIF ( filerror.CRDFIL ) %]<b>ERROR:</b> Image not imported ([% filerror.CRDFIL %] missing).
+                                        [% ELSIF ( filerror.CARDNUMBER_DOES_NOT_EXIST ) %]<b>ERROR:</b> Image not imported because this patron does not exist in the database.
                                         [% ELSE %]<b>ERROR:</b> 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 (executable)
--- 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;