Bugs 2541 and 2587 - AddIssue must return date object as intended.

SIP actually relied on the AddIssue return that was not reliable.
AddRenew also updated to return C4::Dates object for datedue.

Please note, any running SIPServer will have to be restarted
*immediately* after applying this patch, because although Koha
C4 behaves as normal, the SIP server runs as a Net::Server with
components cached.  Changes will not be applied until SIPServer
restarts, and so checkout actions may fail until then.

Signed-off-by: Galen Charlton <galen.charlton@liblime.com>

C4/Circulation.pm

@@ -19,7 +19,7 @@ package C4::Circulation;
 
 
 use strict;
-require Exporter;
+#use warnings;  # soon!
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
@@ -48,8 +48,8 @@ use Data::Dumper;
 use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 BEGIN {
-	# set the version for version checking
+	require Exporter;
-	$VERSION = 3.01;
+	$VERSION = 3.02;	# for version checking
 	@ISA    = qw(Exporter);
 
 	# FIXME subs that should probably be elsewhere
@@ -811,19 +811,21 @@ sub CanBookBeIssued {
 
 Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this sub, it means the user confirmed if needed.
 
-&AddIssue($borrower,$barcode,$date)
+&AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])
 
 =over 4
 
-=item C<$borrower> hash with borrower informations (from GetMemberDetails)
+=item C<$borrower> is a hash with borrower informations (from GetMemberDetails).
 
-=item C<$barcode> is the bar code of the book being issued.
+=item C<$barcode> is the barcode of the item being issued.
 
-=item C<$date> contains the max date of return. calculated if empty.
+=item C<$datedue> is a C4::Dates object for the max date of return, i.e. the date due (optional).
+Calculated if empty.
 
-=item C<$cancelreserve>
+=item C<$cancelreserve> is 1 to override and cancel any pending reserves for the item (optional).
 
-=item C<$issuedate> the date to issue the item in iso format (YYYY-MM-DD). Defaults to today.
+=item C<$issuedate> is the date to issue the item in iso (YYYY-MM-DD) format (optional).
+Defaults to today.  Unlike C<$datedue>, NOT a C4::Dates object, unfortunately.
 
 AddIssue does the following things :
 - step 01: check that there is a borrowernumber & a barcode provided
@@ -876,15 +878,14 @@ sub AddIssue {
 		#
 		# check if we just renew the issue.
 		#
-		if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} ) {
+		if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
-			AddRenewal(
+			$datedue = AddRenewal(
 				$borrower->{'borrowernumber'},
 				$item->{'itemnumber'},
 				$branch,
 				$datedue,
                 $issuedate, # here interpreted as the renewal date
 			);
-
 		}
 		else {
         # it's NOT a renewal
@@ -959,33 +960,30 @@ sub AddIssue {
                     (borrowernumber, itemnumber,issuedate, date_due, branchcode)
                 VALUES (?,?,?,?,?)"
           );
-        my $dateduef;
+        unless ($datedue) {
-        if ($datedue) {
-            $dateduef = $datedue;
-        } else {
             my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
             my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );
-            $dateduef = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch );
+            $datedue = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch );
 
             # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
-            if ( C4::Context->preference('ReturnBeforeExpiry') && $dateduef->output('iso') gt $borrower->{dateexpiry} ) {
+            if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {
-                $dateduef = C4::Dates->new( $borrower->{dateexpiry}, 'iso' );
+                $datedue = C4::Dates->new( $borrower->{dateexpiry}, 'iso' );
             }
         }
-                        $sth->execute(
+        $sth->execute(
-                            $borrower->{'borrowernumber'},      # borrowernumber
+            $borrower->{'borrowernumber'},      # borrowernumber
-                            $item->{'itemnumber'},              # itemnumber
+            $item->{'itemnumber'},              # itemnumber
-                            $issuedate,                         # issuedate
+            $issuedate,                         # issuedate
-                            $dateduef->output('iso'),           # date_due
+            $datedue->output('iso'),            # date_due
-                            C4::Context->userenv->{'branch'}    # branchcode
+            C4::Context->userenv->{'branch'}    # branchcode
-                        );
+        );
         $sth->finish;
         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
                   holdingbranch    => C4::Context->userenv->{'branch'},
                   itemlost         => 0,
                   datelastborrowed => C4::Dates->new()->output('iso'),
-                  onloan           => $dateduef->output('iso'),
+                  onloan           => $datedue->output('iso'),
                 }, $item->{'biblionumber'}, $item->{'itemnumber'});
         ModDateLastSeen( $item->{'itemnumber'} );
         
@@ -1013,8 +1011,8 @@ sub AddIssue {
     
     logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'}) 
         if C4::Context->preference("IssueLog");
-    return ($datedue);
   }
+  return ($datedue);	# not necessarily the same as when it came in!
 }
 
 =head2 GetLoanLength
@@ -2030,6 +2028,7 @@ sub AddRenewal {
     }
     # Log the renewal
     UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber);
+	return $datedue;
 }
 
 sub GetRenewCount {

C4/SIP/ILS/Patron.pm

@@ -25,7 +25,7 @@ use Digest::MD5 qw(md5_base64);
 use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
 
 BEGIN {
-	$VERSION = 2.01;
+	$VERSION = 2.02;
 	@ISA = qw(Exporter);
 	@EXPORT_OK = qw(invalid_patron);
 }
@@ -37,19 +37,19 @@ sub new {
     my $type = ref($class) || $class;
     my $self;
 	$kp = GetMember($patron_id,'cardnumber');
-	$debug and warn "new Patron: " . Dumper($kp);
+	$debug and warn "new Patron (GetMember): " . Dumper($kp);
     unless (defined $kp) {
 		syslog("LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id);
 		return undef;
 	}
 	$kp = GetMemberDetails(undef,$patron_id);
-	$debug and warn "new Patron: " . Dumper($kp);
+	$debug and warn "new Patron (GetMemberDetails): " . Dumper($kp);
 	my $pw = $kp->{password};    ## FIXME - md5hash -- deal with . 
 	my $dob= $kp->{dateofbirth};
 	my $fines_out = GetMemberAccountRecords($kp->{borrowernumber});
 	my $flags = $kp->{flags}; # or warn "Warning: No flags from patron object for '$patron_id'"; 
 	my $debarred = $kp->{debarred}; ### 1 if ($kp->{flags}->{DBARRED}->{noissues});
-	$debug and warn "Debarred: $debarred = " . Dumper(%{$kp->{flags}});
+	$debug and warn sprintf("Debarred = %s : ",($debarred||'undef')) . Dumper(%{$kp->{flags}});
 	my %ilspatron;
 	my $adr     = $kp->{streetnumber} || '';
 	my $address = $kp->{address}      || ''; 
@@ -58,6 +58,7 @@ sub new {
 	no warnings;	# any of these $kp->{fields} being concat'd could be undef
 	$dob =~ s/\-//g;
 	%ilspatron = (
+	  getmemberdetails_object => $kp,
 		name => $kp->{firstname} . " " . $kp->{surname},
 		  id => $kp->{cardnumber},			# to SIP, the id is the BARCODE, not userid
 		  password => $pw,
@@ -222,6 +223,10 @@ sub too_many_billed {
     my $self = shift;
     return $self->{too_many_billed};
 }
+sub getmemberdetails_object {
+    my $self = shift;
+    return $self->{getmemberdetails_object};
+}
 
 #
 # List of outstanding holds placed

C4/SIP/ILS/Transaction/Checkout.pm

@@ -18,13 +18,13 @@ use ILS::Transaction;
 use C4::Context;
 use C4::Circulation;
 use C4::Members;
+use C4::Debug;
 
 use vars qw($VERSION @ISA $debug);
 
 BEGIN {
-	$VERSION = 1.01;
+	$VERSION = 1.02;
 	@ISA = qw(ILS::Transaction);
-	$debug = 0;
 }
 
 # Most fields are handled by the Transaction superclass
@@ -53,7 +53,10 @@ sub do_checkout {
 	my $barcode        = $self->{item}->id;
 	my $patron_barcode = $self->{patron}->id;
 	$debug and warn "do_checkout: patron (" . $patron_barcode . ")";
-	my $borrower = GetMember( $patron_barcode, 'cardnumber' );
+	# my $borrower = GetMember( $patron_barcode, 'cardnumber' );
+	# my $borrower = $self->{patron};
+	# my $borrower = GetMemberDetails(undef, $patron_barcode);
+	my $borrower = $self->{patron}->getmemberdetails_object();
 	$debug and warn "do_checkout borrower: . " . Dumper $borrower;
 	my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( $borrower, $barcode );
 	my $noerror=1;
@@ -83,7 +86,11 @@ sub do_checkout {
 	$debug and warn "do_checkout: calling AddIssue(\$borrower,$barcode, undef, 0)\n"
 		# . "w/ \$borrower: " . Dumper($borrower)
 		. "w/ C4::Context->userenv: " . Dumper(C4::Context->userenv);
-	$self->{'due'} = AddIssue( $borrower, $barcode, undef, 0 );
+	my $c4due  = AddIssue( $borrower, $barcode, undef, 0 );
+	my $due  = $c4due->output('iso') || undef;
+	$debug and warn "Item due: $due";
+	$self->{'due'} = $due;
+	$self->{item}->due_date($due);
 	$self->ok(1);
 	return $self;
 }