From d95e1ce857714379a4faff88d380314d8715f885 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Tue, 10 Feb 2015 17:22:43 +0000 Subject: [PATCH] Bug 12820: Handle rental fees in Sip issue and renew Implement correct handling of fees associated with checking out an item. This is associated with fee acknowledged field (BO) To quote from the Sip2 document " If this field is N in a Checkout message and there is a fee associated with checking out the item, the ACS should tell the SC in the Checkout Response that there is a fee, and refuse to check out the item. If the SC and the patron then interact and the patron agrees to pay the fee, this field will be set to Y on a second Checkout message, indicating to the ACS that the patron has acknowledged the fee and checkout of the item should not be refused just because there is a fee associated with the item" So there are two Checkout requests the first with BO not set to Y is rejected but the fee amount is returned. The Second Checkout with BO set to Y should succeed. Added a debug log message indicating why we block a checkout when we dont otherwise indicate Signed-off-by: Brendan Gallagher Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi (cherry picked from commit fe179c737df8a56be3cac6098d5266becb2dfef2) Signed-off-by: Chris Cormack Conflicts: C4/SIP/ILS.pm --- C4/SIP/ILS.pm | 9 ++++++--- C4/SIP/ILS/Transaction.pm | 1 + C4/SIP/ILS/Transaction/Checkout.pm | 13 +++++++++++++ C4/SIP/ILS/Transaction/Renew.pm | 11 +++++++++++ C4/SIP/Sip/MsgType.pm | 22 +++++++++++++--------- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index 8086bc6bb3..3db9f6bb14 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -126,13 +126,16 @@ sub offline_ok { # the response. # sub checkout { - my ($self, $patron_id, $item_id, $sc_renew) = @_; + my ($self, $patron_id, $item_id, $sc_renew, $fee_ack) = @_; my ($patron, $item, $circ); $circ = new ILS::Transaction::Checkout; # BEGIN TRANSACTION - $circ->patron($patron = new ILS::Patron $patron_id); - $circ->item($item = new ILS::Item $item_id); + $circ->patron($patron = C4::SIP::ILS::Patron->new( $patron_id)); + $circ->item($item = C4::SIP::ILS::Item->new( $item_id)); + if ($fee_ack) { + $circ->fee_ack($fee_ack); + } if (!$patron) { $circ->screen_msg("Invalid Patron"); diff --git a/C4/SIP/ILS/Transaction.pm b/C4/SIP/ILS/Transaction.pm index eb8d71fcca..c824413241 100644 --- a/C4/SIP/ILS/Transaction.pm +++ b/C4/SIP/ILS/Transaction.pm @@ -21,6 +21,7 @@ my %fields = ( sip_currency => 'USD', # FIXME: why hardcoded? screen_msg => '', print_line => '', + fee_ack => 'N', ); our $AUTOLOAD; diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index edc1d046db..8413937265 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -91,9 +91,14 @@ sub do_checkout { } elsif ($confirmation eq 'HIGHHOLDS') { $overridden_duedate = $needsconfirmation->{$confirmation}->{returndate}; $self->screen_msg('Loan period reduced for high-demand item'); + } elsif ($confirmation eq 'RENTALCHARGE') { + if ($self->{fee_ack} ne 'Y') { + $noerror = 0; + } } else { $self->screen_msg($needsconfirmation->{$confirmation}); $noerror = 0; + syslog('LOG_DEBUG', "Blocking checkout Reason:$confirmation"); } } } @@ -106,6 +111,14 @@ sub do_checkout { $debug and warn "Item is on hold shelf for another patron."; $self->screen_msg("Item is on hold shelf for another patron."); $noerror = 0; + } + my ($fee, undef) = GetIssuingCharges($itemnumber, $self->{patron}->{borrowernumber}); + if ( $fee > 0 ) { + $self->{sip_fee_type} = '06'; + $self->{fee_amount} = sprintf '%.2f', $fee; + if ($self->{fee_ack} eq 'N' ) { + $noerror = 0; + } } unless ($noerror) { $debug and warn "cannot issue: " . Dumper($issuingimpossible) . "\n" . Dumper($needsconfirmation); diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm index aa300d4dd9..18509659e4 100644 --- a/C4/SIP/ILS/Transaction/Renew.pm +++ b/C4/SIP/ILS/Transaction/Renew.pm @@ -34,6 +34,17 @@ sub do_renew_for { my $self = shift; my $borrower = shift; my ($renewokay,$renewerror) = CanBookBeRenewed($borrower->{borrowernumber},$self->{item}->{itemnumber}); + if ($renewokay) { # ok so far check charges + my ($fee, undef) = GetIssuingCharges($self->{item}->{itemnumber}, $self->{patron}->{borrowernumber}); + if ($fee > 0) { + $self->{sip_fee_type} = '06'; + $self->{fee_amount} = sprintf '%.2f',$fee; + if ($self->{fee_ack} eq 'N') { + $renewokay = 0; + } + } + + } if ($renewokay){ $self->{due} = undef; my $due_date = AddIssue( $borrower, $self->{item}->id, undef, 0 ); diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 2aca72f866..f22648ab7a 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -511,6 +511,7 @@ sub handle_checkout { $patron_id = $fields->{(FID_PATRON_ID)}; $item_id = $fields->{(FID_ITEM_ID)}; + my $fee_ack = $fields->{(FID_FEE_ACK)}; if ($no_block eq 'Y') { @@ -525,7 +526,7 @@ sub handle_checkout { } else { # Does the transaction date really matter for items that are # checkout out while the terminal is online? I'm guessing 'no' - $status = $ils->checkout($patron_id, $item_id, $sc_renewal_policy); + $status = $ils->checkout($patron_id, $item_id, $sc_renewal_policy, $fee_ack); } $item = $status->item; @@ -567,17 +568,10 @@ sub handle_checkout { $resp .= maybe_add(FID_MEDIA_TYPE, $item->sip_media_type); $resp .= maybe_add(FID_ITEM_PROPS, $item->sip_item_properties); - # Financials - if ($status->fee_amount) { - $resp .= add_field(FID_FEE_AMT, $status->fee_amount); - $resp .= maybe_add(FID_CURRENCY, $status->sip_currency); - $resp .= maybe_add(FID_FEE_TYPE, $status->sip_fee_type); - $resp .= maybe_add(FID_TRANSACTION_ID, - $status->transaction_id); } } - } else { + else { # Checkout failed # Checkout Response: not ok, no renewal, don't know mag. media, # no desensitize @@ -607,6 +601,16 @@ sub handle_checkout { } } } + if ($protocol_version >= 2) { + # Financials : return irrespective of ok status + if ($status->fee_amount) { + $resp .= add_field(FID_FEE_AMT, $status->fee_amount); + $resp .= maybe_add(FID_CURRENCY, $status->sip_currency); + $resp .= maybe_add(FID_FEE_TYPE, $status->sip_fee_type); + $resp .= maybe_add(FID_TRANSACTION_ID, + $status->transaction_id); + } + } $self->write_msg($resp,undef,$server->{account}->{terminator},$server->{account}->{encoding}); return(CHECKOUT); -- 2.39.5