Bugzilla – Attachment 36155 Details for
Bug 12820
SIP2 does not send rental charge fee!
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12820: Handle rental fees in Sip issue and renew
Bug-12820-Handle-rental-fees-in-Sip-issue-and-rene.patch (text/plain), 6.75 KB, created by
Brendan Gallagher
on 2015-02-25 05:35:07 UTC
(
hide
)
Description:
Bug 12820: Handle rental fees in Sip issue and renew
Filename:
MIME Type:
Creator:
Brendan Gallagher
Created:
2015-02-25 05:35:07 UTC
Size:
6.75 KB
patch
obsolete
>From fcf438af36f900b3045f5af5a2665a068e43cddf Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >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 <brendan@bywatersolutions.com> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12820
:
31136
|
31153
|
33901
|
33902
|
33911
|
33994
|
35878
|
36155
|
37038
|
37039