Bugzilla – Attachment 64265 Details for
Bug 15438
Checking out an on-hold item sends holder's borrowernumber in AF (screen message) field.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15438 - Checking out an on-hold item sends holder's borrowernumber in AF (screen message) field.
Bug-15438---Checking-out-an-on-hold-item-sends-hol.patch (text/plain), 5.64 KB, created by
Kyle M Hall
on 2017-06-13 17:39:28 UTC
(
hide
)
Description:
Bug 15438 - Checking out an on-hold item sends holder's borrowernumber in AF (screen message) field.
Filename:
MIME Type:
Creator:
Kyle M Hall
Created:
2017-06-13 17:39:28 UTC
Size:
5.64 KB
patch
obsolete
>From 521f8940edf158bfd95235e85cbfdec3d481a770 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Wed, 24 Aug 2016 13:52:52 +0100 >Subject: [PATCH] Bug 15438 - Checking out an on-hold item sends holder's > borrowernumber in AF (screen message) field. > >The returns from C4::Circulation::CanBookBeIssued used >to be structured as a hashref of entries like > REASON => { > data => 'foo', > moredata => 'bar', > }; >Some entries still are. But many are now > REASON => 1, > data => 'foo', > moredata => 'bar', > >The sip Checkout routine still assumed the former, as it >reports any causes it was not aware of (to maintain support for >a changing api) The data fields could leak into the screen message >field of the response. e.g. the borrowernumber or surname of the >borrower who has a hold on an issued title. Some real messages were >getting obscured by this > >This patch sanatizes the return from from CanBookBeIssued >by removing keys which are not all uppercase >It also fixes a case where the key's data element was used >for the screen message when we should use the key itself > >Updated the documentation of CanBookBeIssued to flag up >the assumption re case and the fact that 3 elements rather >than two may be returned > >The loop through the returned keys was a bit bogus >so we now explicitly jump out if noerror is unset >--- > C4/Circulation.pm | 8 +++++++- > C4/SIP/ILS/Transaction/Checkout.pm | 42 ++++++++++++++++++++++++++++---------- > 2 files changed, 38 insertions(+), 12 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index a439021..5753c4c 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -564,7 +564,7 @@ sub TooMany { > > =head2 CanBookBeIssued > >- ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $borrower, >+ ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) = CanBookBeIssued( $borrower, > $barcode, $duedate, $inprocess, $ignore_reserves, $params ); > > Check if a book can be issued. >@@ -661,6 +661,12 @@ sticky due date is invalid or due date in the past > > if the borrower borrows to much things > >+=head3 NB >+ >+The assumption by users of the routine is that causes blocking >+the issue are keyed by uppercase labels and other returned >+data is keyed in lower case >+ > =cut > > sub CanBookBeIssued { >diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm >index 14a2b66..a88022b 100644 >--- a/C4/SIP/ILS/Transaction/Checkout.pm >+++ b/C4/SIP/ILS/Transaction/Checkout.pm >@@ -56,19 +56,17 @@ sub do_checkout { > $debug and warn "do_checkout: patron (" . $patron_barcode . ")"; > my $borrower = $self->{patron}->getmemberdetails_object(); > $debug and warn "do_checkout borrower: . " . Dumper $borrower; >- my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( >- $borrower, >- $barcode, >- undef, >- 0, >+ my ($issuingimpossible, $needsconfirmation) = _can_we_issue($borrower, $barcode, > C4::Context->preference("AllowItemsOnHoldCheckout") > ); >- my $noerror=1; >- if (scalar keys %$issuingimpossible) { >- foreach (keys %$issuingimpossible) { >+ >+ my $noerror=1; # If set to zero we block the issue >+ if (keys %{$issuingimpossible}) { >+ foreach (keys %{$issuingimpossible}) { > # do something here so we pass these errors >- $self->screen_msg($_ . ': ' . $issuingimpossible->{$_}); >+ $self->screen_msg("Issue failed : $_"); > $noerror = 0; >+ last; > } > } else { > foreach my $confirmation (keys %{$needsconfirmation}) { >@@ -80,25 +78,30 @@ sub do_checkout { > $self->screen_msg("Item was reserved for you."); > } else { > $self->screen_msg("Item is reserved for another patron upon return."); >- # $noerror = 0; >+ $noerror = 0; > } > } elsif ($confirmation eq 'ISSUED_TO_ANOTHER') { > $self->screen_msg("Item already checked out to another patron. Please return item for check-in."); > $noerror = 0; >+ last; > } elsif ($confirmation eq 'DEBT') { > $self->screen_msg('Outstanding Fines block issue'); > $noerror = 0; >+ last; > } 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; >+ last; > } > } else { >- $self->screen_msg($needsconfirmation->{$confirmation}); >+ # We've been returned a case other than those above >+ $self->screen_msg("Item cannot be issued: $confirmation"); > $noerror = 0; > syslog('LOG_DEBUG', "Blocking checkout Reason:$confirmation"); >+ last; > } > } > } >@@ -145,5 +148,22 @@ sub do_checkout { > return $self; > } > >+sub _can_we_issue { >+ my ( $borrower, $barcode, $pref ) = @_; >+ >+ my ( $issuingimpossible, $needsconfirmation, $alerts ) = >+ CanBookBeIssued( $borrower, $barcode, undef, 0, $pref ); >+ for my $href ( $issuingimpossible, $needsconfirmation ) { >+ >+ # some data is returned using lc keys we only >+ foreach my $key ( keys %{$href} ) { >+ if ( $key =~ m/[^A-Z_]/ ) { >+ delete $href->{$key}; >+ } >+ } >+ } >+ return ( $issuingimpossible, $needsconfirmation ); >+} >+ > 1; > __END__ >-- >2.10.2
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 15438
:
54783
|
54827
|
61738
|
64264
|
64265
|
64266
|
64757
|
66982
|
66983