[Koha-patches] [PATCH] (bug #4002) fix MarkIssueReturned for multiples issues in one item

[Nahuel ANGELINETTI]

This delete borrowernumber argument from MarkIssueReturned, because it's not needed, only one "issue" is autorised by item in issues table.
If there is more than one, we move them to old_issues, else all circulation for this item is broken.
---
 C4/Accounts.pm                                  |    4 ++--
 C4/Circulation.pm                               |   20 +++++++++-----------
 offline_circ/process_koc.pl                     |    3 +--
 t/lib/KohaTest/Circulation/MarkIssueReturned.pm |    5 ++---
 4 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index c43741b..e8ce6b3 100644
--- a/C4/Accounts.pm
+++ b/C4/Accounts.pm
@@ -264,7 +264,7 @@ EOT
 
 sub returnlost{
     my ( $borrowernumber, $itemnum ) = @_;
-    C4::Circulation::MarkIssueReturned( $borrowernumber, $itemnum );
+    C4::Circulation::MarkIssueReturned($itemnum );
     my $borrower = C4::Members::GetMember( $borrowernumber, 'borrowernumber' );
     my @datearr = localtime(time);
     my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
@@ -312,7 +312,7 @@ sub chargelostitem{
         }
         #FIXME : Should probably have a way to distinguish this from an item that really was returned.
         warn " $issues->{'borrowernumber'}  /  $itemnumber ";
-        C4::Circulation::MarkIssueReturned($issues->{borrowernumber},$itemnumber);
+        C4::Circulation::MarkIssueReturned($itemnumber);
 	#  Shouldn't MarkIssueReturned do this?
         ModItem({ onloan => undef }, undef, $itemnumber);
     }
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index e084548..43c0e07 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1433,7 +1433,7 @@ sub AddReturn {
 				# don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
 				undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
 			}
-            MarkIssueReturned($borrower->{'borrowernumber'}, $iteminformation->{'itemnumber'},$circControlBranch);
+            MarkIssueReturned($iteminformation->{'itemnumber'},$circControlBranch);
             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  
             # continue to deal with returns cases, but not only if we have an issue
             
@@ -1530,7 +1530,7 @@ sub AddReturn {
 
 =over 4
 
-MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate);
+MarkIssueReturned($itemnumber, $dropbox_branch, $returndate);
 
 =back
 
@@ -1551,7 +1551,7 @@ routine in C<C4::Accounts>.
 =cut
 
 sub MarkIssueReturned {
-    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate ) = @_;
+    my ( $itemnumber, $dropbox_branch, $returndate ) = @_;
     my $dbh   = C4::Context->dbh;
     my $query = "UPDATE issues SET returndate=";
     my @bind;
@@ -1566,19 +1566,17 @@ sub MarkIssueReturned {
     } else {
         $query .= " now() ";
     }
-    $query .= " WHERE  borrowernumber = ?  AND itemnumber = ?";
-    push @bind, $borrowernumber, $itemnumber;
+    $query .= " WHERE itemnumber = ?";
+    push @bind, $itemnumber;
     # FIXME transaction
     my $sth_upd  = $dbh->prepare($query);
     $sth_upd->execute(@bind);
     my $sth_copy = $dbh->prepare("INSERT INTO old_issues SELECT * FROM issues 
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-    $sth_copy->execute($borrowernumber, $itemnumber);
+                                  WHERE itemnumber = ?");
+    $sth_copy->execute($itemnumber);
     my $sth_del  = $dbh->prepare("DELETE FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-    $sth_del->execute($borrowernumber, $itemnumber);
+                                  WHERE itemnumber = ?");
+    $sth_del->execute($itemnumber);
 }
 
 =head2 FixOverduesOnReturn
diff --git a/offline_circ/process_koc.pl b/offline_circ/process_koc.pl
index 87ae5c1..23b242d 100755
--- a/offline_circ/process_koc.pl
+++ b/offline_circ/process_koc.pl
@@ -328,8 +328,7 @@ sub kocReturnItem {
   my $borrowernumber = _get_borrowernumber_from_barcode( $circ->{'barcode'} );
   if ( $borrowernumber ) {
   my $borrower = GetMember( $borrowernumber, 'borrowernumber' );
-    C4::Circulation::MarkIssueReturned( $borrowernumber,
-                                      $item->{'itemnumber'},
+    C4::Circulation::MarkIssueReturned($item->{'itemnumber'},
                                       undef,
                                       $circ->{'date'} );
   
diff --git a/t/lib/KohaTest/Circulation/MarkIssueReturned.pm b/t/lib/KohaTest/Circulation/MarkIssueReturned.pm
index 5722bcf..d271fc4 100644
--- a/t/lib/KohaTest/Circulation/MarkIssueReturned.pm
+++ b/t/lib/KohaTest/Circulation/MarkIssueReturned.pm
@@ -26,7 +26,7 @@ sub basic_usage : Test( 4 ) {
     is( $after_issues->{'borrowernumber'}, $self->{'memberid'}, 'Our item is checked out to our borrower' )
       or diag( Data::Dumper->Dump( [ $after_issues ], [ 'after_issues' ] ) );
 
-    C4::Circulation::MarkIssueReturned( $self->{'memberid'}, $self->{'items'}[0]{'itemnumber'} );
+    C4::Circulation::MarkIssueReturned( $self->{'items'}[0]{'itemnumber'} );
 
     my $after_return = C4::Circulation::GetItemIssue( $self->{'items'}[0]{'itemnumber'} );
     ok( ! defined $after_return->{'borrowernumber'}, 'The item is no longer checked out' )
@@ -63,8 +63,7 @@ sub set_retundate : Test( 7 ) {
     my $returndate = $self->random_date();
     ok( $returndate, "return this item on $returndate" );
 
-    C4::Circulation::MarkIssueReturned( $self->{'memberid'},
-                                        $self->{'items'}[0]{'itemnumber'},
+    C4::Circulation::MarkIssueReturned( $self->{'items'}[0]{'itemnumber'},
                                         undef,
                                         $returndate );
 
-- 
1.6.3.3