From 45b710f2db91b4d89b2a17271bcf69cf663dd047 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Sun, 28 Oct 2018 22:10:43 +0000 Subject: [PATCH] Bug 15565: (follow-up) Changes to maximum hold allowed logic Implemented changes to the logic to address Katrin Fischer's points in comment #175, also changed the modal box wording to use 'hold' not 'reserve' Also updated t/db_dependent/Reserves/MultiplePerRecord.t test to reflect moving function. Added the display of the JS modal box informing OPAC borrower that they have exceeded hold limit after they click 'A specific item' radiobutton on opac-reserve if holds per day, holds per record or maxreserves is 1 Test plan: 1. Apply patches 2. Restart memcached and plack 3. Run t/db_dependent/Reserves/MultiplePerRecord.t 4. Set your maxreserves syspref to 3, and a circ rules holds per record to 2 and holds per day to 1 5. In the OPAC visit the 'Place hold' interface of item. Select the 'A specific item' radiobutton and observe the first item checkbox is checked and other unchecked checkboxes are disabled. 6. Repeat step 4 this time setting holds per day to 5 7. Now reloading the OPAC 'Place hold' page notice after clicking 'A specific item' no modal displays 8. Select a second item checkbox and observe a modal saying you have reached the maximum number of holds for the record is loaded - notice the wording 'hold' not 'reserve' in use in the modal box 9. Click 'OK' on modal and observe all unchecked item checkboxes are automatically disabled to prevent additional holds being placed 10. Select 'Confirm hold' 11. On your holds summary page confirm both holds are placed 12. In the staff client set the 'AllowHoldPolicyOverride' syspref to "Don't allow" 13. Visit a biblio reservation interface in the staff client 14. Select 2 item checkboxes and observe the modal box explaining you have reached maximum holds for the record is displayed - notice the wording of 'hold' not 'reserve' is used in the modal box. 15. Select 'OK' on the modal and notice all un-checked item checkboxes are automatically disabled 16. Repeat steps 12-15 this time with the value of 'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once you have clicked on the second item checkbox although the modal still loads the item checkboxes do not disable - you are able to override and continue selecting checkboxes each time with modal warning loading. 17. In both the OPAC and staff client try placing a record level hold, selecting a value from the 'Holds to place (count)' dropdown. Notice that the maximum value of this is the lowest value out of the 'maximum number of holds per record', 'maximum number of holds per day' and the 'maxreserves' system preference so in this case it will be 2 18. Save and notice 2 holds are placed 19. Observe the allow_holds() function is in the Koha/Biblio.pm file and the GetAllowedHoldsForPatronToday() function has been removed from C4/Reserves.pm Sponsored-By: Brimbank Library, Australia --- C4/Reserves.pm | 1 + Koha/Biblio.pm | 32 ++++++++++++++ .../prog/en/modules/reserve/request.tt | 25 +++++++++-- koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss | 1 + .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 50 +++++++++++++++++++--- opac/opac-reserve.pl | 21 ++++++++- reserve/request.pl | 25 ++++++++++- t/db_dependent/Reserves/MultiplePerRecord.t | 19 +++++++- 8 files changed, 162 insertions(+), 12 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 39adfeb..df8166b 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -4,6 +4,7 @@ package C4::Reserves; # 2006 SAN Ouest Provence # 2007-2010 BibLibre Paul POULAIN # 2011 Catalyst IT +# 2018 Catalyst IT # # This file is part of Koha. # diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 56f9185..6f944e8 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -467,6 +467,38 @@ sub has_items_waiting_or_intransit { return 0; } +=head3 allowed_holds + +my $holds_allowed_on_record_today = $biblio->allowed_holds( $patron_obj ); + +Calculates and returns the number of item-level holds a borrower is allowed to place on the record on a single day. + +=cut + +sub allowed_holds { + + my ( $self, $patron ) = @_; + + my $controlbranch = C4::Context->preference('ReservesControlBranch'); + + my $categorycode = $patron->categorycode; + my $branchcode; + $branchcode = $patron->branchcode if ( $controlbranch eq "PatronLibrary" ); + + my $holds_allowed = 0; + foreach my $item ( $self->items()->as_list() ) { + my $itemtype = $item->effective_itemtype(); + $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" ); + + my $rule = C4::Reserves::GetHoldRule( $categorycode, $itemtype, $branchcode ); + my $holds_per_day = $rule ? $rule->{holds_per_day} : 0; + if ( $holds_per_day ) { + $holds_allowed = $holds_per_day if $holds_per_day > $holds_allowed; + } + } + return $holds_allowed; +} + =head3 type =cut diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index a3b1676..6acafe8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -249,7 +249,7 @@ [% IF remaining_holds_for_record > 1 %]
  • - +
    • [% ELSE %] @@ -677,8 +677,12 @@ }; var MSG_NO_ITEMS_AVAILABLE = _("A hold cannot be requested on any of these items."); columns_settings_borrowers_table = [% ColumnsSettings.GetColumns( 'circ', 'circulation', 'table_borrowers', 'json' ) | $raw %] - var MSG_EXCEEDED_HOLDS_PER_RECORD = _("Patron has exceeded the number of holds for this record."); + var MSG_EXCEEDED_HOLDS_PER_RECORD = _("Patron reached the maximum number of holds for this record."); + var MSG_EXCEEDED_HOLDS_PER_DAY = _("Patron reached the maximum number of holds allowed today."); + var MSG_EXCEEDED_MAXRESERVES_SYSPREF = _("Patron reached the maximum number of holds allowed at once"); var remainingHolds = "[% remaining_holds_for_record | html %]"; + var remainingHoldsToday = "[% remainig_holds_allowed_today | html %]"; + var maxreserves = "[% maxreserves | html %]"; $(document).ready(function() { function ToggleHoldsToPlace() { @@ -824,8 +828,23 @@ } else { $("#requestany").prop("checked",true); } - if (remainingHolds - numchecked < 0) { + if (remainingHolds - numchecked <= 0) { alert(MSG_EXCEEDED_HOLDS_PER_RECORD); + [% IF !(Koha.Preference('AllowHoldPolicyOverride')) %] + $('input[name=checkitem]:not(:checked)').attr('disabled', 'disabled'); + [% END %] + } + if (remainingHoldsToday - numchecked <= 0) { + alert(MSG_EXCEEDED_HOLDS_PER_DAY); + [% IF !(Koha.Preference('AllowHoldPolicyOverride')) %] + $('input[name=checkitem]:not(:checked)').attr('disabled', 'disabled'); + [% END %] + } + if (maxreserves - numchecked <= 0) { + alert(MSG_EXCEEDED_MAXRESERVES_SYSPREF); + [% IF !(Koha.Preference('AllowHoldPolicyOverride')) %] + $('input[name=checkitem]:not(:checked)').attr('disabled', 'disabled'); + [% END %] } }); var prev_rank_request; diff --git a/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss b/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss index b0f99c9..a3c50f0 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss +++ b/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss @@ -3042,6 +3042,7 @@ button.closebtn { .hold-options { clear: both; + width: auto; } .toggle-hold-options { diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index 56d5d27..92a97fa 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -156,6 +156,7 @@ [% FOREACH bibitemloo IN bibitemloop %]
    +

    [% IF ( bibitemloo.holdable ) %] @@ -302,8 +303,10 @@ [% END # / IF OpacHoldNotes %] + [% IF bibitemloo.itemholdable %]

    + [% END # / IF bibitemloo.itemholdable %] [% IF bibitemloo.remaining_holds_for_record > 1 %] @@ -325,7 +330,7 @@