From 4f4c5e67efc33a974523139b0bc0e57fae9cfc46 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 5 Oct 2015 07:50:18 -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 Signed-off-by: David Kuhn Signed-off-by: Jonathan Druart Signed-off-by: Tomas Cohen Arazi --- C4/Reserves.pm | 50 ++++++++++++------- .../bootstrap/en/modules/opac-user.tt | 7 +++ opac/opac-reserve.pl | 6 ++- opac/opac-user.pl | 18 +++---- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 80ebf0ad48..4b10ff95c1 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,29 @@ 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 $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 +189,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 b5bc8d2cad..8689c3e4e3 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 ) %]

Thank you!

Your corrections have been submitted to the library, and a staff member will update your record as soon as possible.

[% END %] + [% IF failed_holds %] +
+

Notice:

+

One or more holds were not placed due to existing holds.

+
+ [% END %] + [% IF ( BORROWER_INF.warndeparture ) %]
Please note: Your card will expire on [% BORROWER_INF.warndeparture | $KohaDates %]. Please contact the library for more information. diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 51feeeed7e..6e35c182a1 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 c6935dee05..052680b604 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -385,17 +385,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, undef, { force_no_caching => 1 }; -- 2.39.5