From 7822223b7268f6ee318aefabe1dd7a7e31cb77bd Mon Sep 17 00:00:00 2001 From: "Joe Atzberger (siptest" Date: Wed, 10 Sep 2008 22:30:04 -0500 Subject: [PATCH] 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 Signed-off-by: Henri-Damien LAURENT --- C4/Circulation.pm | 55 +++++++++++++++--------------- C4/SIP/ILS/Patron.pm | 13 ++++--- C4/SIP/ILS/Transaction/Checkout.pm | 13 +++++-- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index acf3683899..a1782afa44 100644 --- 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 bf0e7a8166..257f9e23ab 100644 --- 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 d62d3c46b0..d0f988009e 100644 --- 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; } -- 2.39.5