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>
Signed-off-by: Henri-Damien LAURENT <henridamien.laurent@biblibre.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index acf3683..a1782af 100644 (file)
--- a/C4/Circulation.pm
+++ b/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
-       $VERSION = 3.01;
+       require Exporter;
+       $VERSION = 3.02;        # for version checking
        @ISA    = qw(Exporter);
 
        # FIXME subs that should probably be elsewhere
@@ -810,19 +810,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
@@ -875,15 +877,14 @@ sub AddIssue {
                #
                # check if we just renew the issue.
                #
-               if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} ) {
-                       AddRenewal(
+               if ($actualissue->{borrowernumber} eq $borrower->{'borrowernumber'}) {
+                       $datedue = AddRenewal(
                                $borrower->{'borrowernumber'},
                                $item->{'itemnumber'},
                                $branch,
                                $datedue,
                 $issuedate, # here interpreted as the renewal date
                        );
-
                }
                else {
         # it's NOT a renewal
@@ -958,33 +959,30 @@ sub AddIssue {
                     (borrowernumber, itemnumber,issuedate, date_due, branchcode)
                 VALUES (?,?,?,?,?)"
           );
-        my $dateduef;
-        if ($datedue) {
-            $dateduef = $datedue;
-        } else {
+        unless ($datedue) {
             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} ) {
-                $dateduef = C4::Dates->new( $borrower->{dateexpiry}, 'iso' );
+            if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {
+                $datedue = C4::Dates->new( $borrower->{dateexpiry}, 'iso' );
             }
         }
-                        $sth->execute(
-                            $borrower->{'borrowernumber'},      # borrowernumber
-                            $item->{'itemnumber'},              # itemnumber
-                            $issuedate,                         # issuedate
-                            $dateduef->output('iso'),           # date_due
-                            C4::Context->userenv->{'branch'}    # branchcode
-                        );
+        $sth->execute(
+            $borrower->{'borrowernumber'},      # borrowernumber
+            $item->{'itemnumber'},              # itemnumber
+            $issuedate,                         # issuedate
+            $datedue->output('iso'),            # date_due
+            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'} );
         
@@ -1012,8 +1010,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
@@ -2007,6 +2005,7 @@ sub AddRenewal {
     }
     # Log the renewal
     UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber);
+       return $datedue;
 }
 
 sub GetRenewCount {

diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm
index bf0e7a8..257f9e2 100644 (file)
--- a/C4/SIP/ILS/Patron.pm
+++ b/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

diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm
index d62d3c4..d0f9880 100644 (file)
--- a/C4/SIP/ILS/Transaction/Checkout.pm
+++ b/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.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;
@@ -91,7 +94,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;
 }