Bugzilla – Attachment 42530 Details for
Bug 5144
Duplicate holds allowed if patron clicks back button after placing hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 5144 - Duplicate holds allowed if patron clicks back button after placing hold
Bug-5144---Duplicate-holds-allowed-if-patron-click.patch (text/plain), 9.74 KB, created by
Kyle M Hall (khall)
on 2015-09-14 15:15:17 UTC
(
hide
)
Description:
Bug 5144 - Duplicate holds allowed if patron clicks back button after placing hold
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2015-09-14 15:15:17 UTC
Size:
9.74 KB
patch
obsolete
>From 274742bfd14115879c79554c31508683b7253cf2 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 7 Jul 2015 11:22:31 -0400 >Subject: [PATCH] Bug 5144 - Duplicate holds allowed if patron clicks back button after placing hold > >Koha is currently not engineered to handle multiple holds per record. >Until such time that is does, we should not allow them to be created. > >Test Plan: >1) Apply this patch >2) Log in to the opac >3) Place a hold >4) Hit the back button on your browser >5) Place the hold again >6) Note the new message >--- > C4/Reserves.pm | 52 +++++++++++++------ > .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 7 +++ > opac/opac-reserve.pl | 6 ++- > opac/opac-user.pl | 18 +++---- > t/db_dependent/Reserves.t | 14 ++++- > 5 files changed, 65 insertions(+), 32 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index dab2b48..47d0fa4 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -40,8 +40,11 @@ use C4::Dates qw( format_date_in_iso ); > use Koha::DateUtils; > use Koha::Calendar; > use Koha::Database; >+use Koha::Hold; >+use Koha::Holds; > > use List::MoreUtils qw( firstidx any ); >+use Carp; > > use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); > >@@ -155,18 +158,31 @@ sub AddReserve { > $bibitems, $priority, $resdate, $expdate, $notes, > $title, $checkitem, $found > ) = @_; >+ >+ if ( Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->count() > 0 ) { >+ carp("AddReserve: borrower $borrowernumber already has a hold for biblionumber $biblionumber"); >+ return; >+ } >+ >+ my $fee = GetReserveFee( $borrowernumber, $biblionumber ); >+ > my $dbh = C4::Context->dbh; >+ > $resdate = format_date_in_iso( $resdate ) if ( $resdate ); > $resdate = C4::Dates->today( 'iso' ) unless ( $resdate ); >+ > if ($expdate) { > $expdate = format_date_in_iso( $expdate ); > } else { > undef $expdate; # make reserves.expirationdate default to null rather than '0000-00-00' > } >- if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) { >- # Make room in reserves for this before those of a later reserve date >- $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority ); >+ >+ if ( C4::Context->preference('AllowHoldDateInFuture') ) { >+ >+ # Make room in reserves for this before those of a later reserve date >+ $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority ); > } >+ > my $waitingdate; > > # If the reserv had the waiting status, we had the value of the resdate >@@ -175,20 +191,22 @@ sub AddReserve { > } > > # updates take place here >- my $query = qq{ >- INSERT INTO reserves >- (borrowernumber,biblionumber,reservedate,branchcode, >- priority,reservenotes,itemnumber,found,waitingdate,expirationdate) >- VALUES >- (?,?,?,?,?, >- ?,?,?,?,?) >- }; >- my $sth = $dbh->prepare($query); >- $sth->execute( >- $borrowernumber, $biblionumber, $resdate, $branch, $priority, >- $notes, $checkitem, $found, $waitingdate, $expdate >- ); >- my $reserve_id = $sth->{mysql_insertid}; >+ my $hold = Koha::Hold->new( >+ { >+ borrowernumber => $borrowernumber, >+ biblionumber => $biblionumber, >+ reservedate => $resdate, >+ branchcode => $branch, >+ priority => $priority, >+ reservenotes => $notes, >+ itemnumber => $checkitem, >+ found => $found, >+ waitingdate => $waitingdate, >+ expirationdate => $expdate >+ } >+ )->store(); >+ my $reserve_id = $hold->id(); >+ > # add a reserve fee if needed > my $fee = GetReserveFee( $borrowernumber, $biblionumber ); > ChargeReserveFee( $borrowernumber, $fee, $title ); >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >index b5bc8d2..8689c3e 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >@@ -48,6 +48,13 @@ > > [% IF ( patronupdate ) %]<div class="alert alert-info"><h3>Thank you!</h3><p>Your corrections have been submitted to the library, and a staff member will update your record as soon as possible.</p></div>[% END %] > >+ [% IF failed_holds %] >+ <div class="alert alert-info"> >+ <h3>Notice:</h3> >+ <p>One or more holds were not placed due to existing holds.</p> >+ </div> >+ [% END %] >+ > [% IF ( BORROWER_INF.warndeparture ) %] > <div class="alert" id="warndeparture"> > <strong>Please note:</strong><span> Your card will expire on <span id="warndeparture_date">[% BORROWER_INF.warndeparture | $KohaDates %]</span>. Please contact the library for more information.</span> >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 4dff9a2..b399bb9 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -221,6 +221,7 @@ if ( $query->param('place_reserve') ) { > &get_out($query, $cookie, $template->output); > } > >+ my $failed_holds = 0; > while (@selectedItems) { > my $biblioNum = shift(@selectedItems); > my $itemNum = shift(@selectedItems); >@@ -283,7 +284,7 @@ if ( $query->param('place_reserve') ) { > > # Here we actually do the reserveration. Stage 3. > if ($canreserve) { >- AddReserve( >+ my $reserve_id = AddReserve( > $branch, $borrowernumber, > $biblioNum, > [$biblioNum], $rank, >@@ -291,11 +292,12 @@ if ( $query->param('place_reserve') ) { > $notes, $biblioData->{title}, > $itemNum, $found > ); >+ $failed_holds++ unless $reserve_id; > ++$reserve_cnt; > } > } > >- print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-holds"); >+ print $query->redirect("/cgi-bin/koha/opac-user.pl?failed_holds=$failed_holds#opac-user-holds"); > exit; > } > >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index d4fbbe3..c1716ac 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -383,17 +383,15 @@ if ( $borr->{'opacnote'} ) { > } > > $template->param( >- bor_messages_loop => GetMessages( $borrowernumber, 'B', 'NONE' ), >- waiting_count => $wcount, >- patronupdate => $patronupdate, >- OpacRenewalAllowed => C4::Context->preference("OpacRenewalAllowed"), >- userview => 1, >-); >- >-$template->param( >- SuspendHoldsOpac => C4::Context->preference('SuspendHoldsOpac'), >+ bor_messages_loop => GetMessages( $borrowernumber, 'B', 'NONE' ), >+ waiting_count => $wcount, >+ patronupdate => $patronupdate, >+ OpacRenewalAllowed => C4::Context->preference("OpacRenewalAllowed"), >+ userview => 1, >+ SuspendHoldsOpac => C4::Context->preference('SuspendHoldsOpac'), > AutoResumeSuspendedHolds => C4::Context->preference('AutoResumeSuspendedHolds'), >- OpacHoldNotes => C4::Context->preference('OpacHoldNotes'), >+ OpacHoldNotes => C4::Context->preference('OpacHoldNotes'), >+ failed_holds => $query->param('failed_holds'), > ); > > output_html_with_http_headers $query, $cookie, $template->output; >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 107cee9..2cb7148 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -153,6 +153,13 @@ $requesters{'CPL'} = AddMember( > categorycode => 'PT', > surname => 'borrower from CPL', > ); >+for my $i ( 2 .. 5 ) { >+ $requesters{"CPL$i"} = AddMember( >+ branchcode => 'CPL', >+ categorycode => 'PT', >+ surname => 'borrower $i from CPL', >+ ); >+} > $requesters{'FPL'} = AddMember( > branchcode => 'FPL', > categorycode => 'PT', >@@ -393,11 +400,12 @@ ModReserveAffect( $itemnumber, $requesters{'CPL'} , 0); #confirm hold > is(defined $results[3]?1:0, 1, 'GetReservesFromItemnumber returns a future wait (confirmed future hold)'); > # End of tests for bug 9788 > >+$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); > # Tests for CalculatePriority (bug 8918) > my $p = C4::Reserves::CalculatePriority($bibnum2); > is($p, 4, 'CalculatePriority should now return priority 4'); > $resdate=undef; >-AddReserve('CPL', $requesters{'CPL'}, $bibnum2, >+AddReserve('CPL', $requesters{'CPL2'}, $bibnum2, > $bibitems, $p, $resdate, $expdate, $notes, > $title, $checkitem, $found); > $p = C4::Reserves::CalculatePriority($bibnum2); >@@ -416,7 +424,7 @@ ModReserveAffect( $itemnumber, $requesters{'CPL'} , 0); > $p = C4::Reserves::CalculatePriority($bibnum); > is($p, 1, 'CalculatePriority should now return priority 1'); > #add another biblio hold, no resdate >-AddReserve('CPL', $requesters{'CPL'}, $bibnum, >+AddReserve('CPL', $requesters{'CPL2'}, $bibnum, > $bibitems, $p, $resdate, $expdate, $notes, > $title, $checkitem, $found); > $p = C4::Reserves::CalculatePriority($bibnum); >@@ -425,7 +433,7 @@ is($p, 2, 'CalculatePriority should now return priority 2'); > C4::Context->set_preference('AllowHoldDateInFuture', 1); > $resdate= dt_from_string(); > $resdate->add_duration(DateTime::Duration->new(days => 1)); >-AddReserve('CPL', $requesters{'CPL'}, $bibnum, >+AddReserve('CPL', $requesters{'CPL3'}, $bibnum, > $bibitems, $p, output_pref($resdate), $expdate, $notes, > $title, $checkitem, $found); > $p = C4::Reserves::CalculatePriority($bibnum); >-- >1.7.2.5
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 5144
:
28657
|
28658
|
28659
|
34983
|
34984
|
34985
|
34986
|
34987
|
34988
|
35090
|
35091
|
35092
|
35093
|
35451
|
40823
|
40824
|
42530
|
42792
|
42848
|
43118
|
43119
|
43120
|
43121
|
43122
|
43125
|
43126
|
43164
|
43165
|
43166
|
43167