From fcf438af36f900b3045f5af5a2665a068e43cddf 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 --- C4/SIP/ILS.pm | 5 ++++- 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, 42 insertions(+), 10 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index a8db7ad..099aa9c 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 = C4::SIP::ILS::Transaction::Checkout->new(); # BEGIN TRANSACTION $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 333152c..4e74e93 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 d277c80..5c95b12 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -90,9 +90,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 { $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); $self->ok(0); diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm index a96811e..d3ef014 100644 --- a/C4/SIP/ILS/Transaction/Renew.pm +++ b/C4/SIP/ILS/Transaction/Renew.pm @@ -32,6 +32,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 a113712..cf3a737 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); -- 1.7.10.4