From bc172982f8e296a6c923601ca553c116d8fe0d16 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 23 Sep 2022 15:06:05 +0000 Subject: [PATCH] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea Signed-off-by: Sam Lau Signed-off-by: Emily Lamancusa Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- .../prog/en/modules/reserve/request.tt | 619 ++++++++++-------- reserve/placerequest.pl | 3 +- 2 files changed, 331 insertions(+), 291 deletions(-) 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 0b04f509ea..268ba8c3be 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -542,72 +542,41 @@
-
- Hold details + [% UNLESS ( multi_hold ) %]
[% INCLUDE 'csrf-token.inc' %] - - - [% FOREACH biblionumber IN biblionumbers %] - - [% END %] - [% IF ( multi_hold ) %] - - - [% FOREACH biblioloo IN biblioloop %] - [% UNLESS biblioloo.none_avail %] - - - - [% END %] +
+ Hold details + + + [% FOREACH biblionumber IN biblionumbers %] + [% END %] - [% ELSE %] - - - - [% END # /IF multi_hold %] + + + -
    -
  1. - Patron: - [% IF ( patron.borrowernumber ) %] - [% INCLUDE 'patron-title.inc' patron => patron no_title => 1 hide_patron_infos_if_needed => 1 %] - [% ELSE %] - Not defined yet - [% END %] -
    2. +
      +
    1. + Patron: + [% IF ( patron.borrowernumber ) %] + [% INCLUDE 'patron-title.inc' patron => patron no_title => 1 hide_patron_infos_if_needed => 1 %] + [% ELSE %] + Not defined yet + [% END %] +
      2. - [% UNLESS ( multi_hold ) %]
    2. Estimated priority: [% fixedRank | html %]
      3. - [% END %] -
    3. - - -
      4. -
    4. - - [% UNLESS ( multi_hold ) %] - - - [% FOREACH pickup_location IN multi_pickup_locations %] - - [% END %] - [% END %] - -
      5. +
    5. + + +
      6. - [% UNLESS ( multi_hold ) %] [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
    6. @@ -619,7 +588,6 @@
      7. [% END %] - [% END # /UNLESS multi_hold %] [% IF ( Koha.Preference('AllowHoldDateInFuture') ) %]
    7. @@ -633,51 +601,184 @@
      8. - [% UNLESS ( multi_hold ) %] +
    8. + + + A non priority hold doesn't prevent a current checkout from renewing +
    9. - - [% IF force_hold_level == 'item' || force_hold_level == 'item_group' %] - + + +
      10. +
    +
+
+ + + [% IF force_hold_level == 'item' || force_hold_level == 'item_group' %] + [% ELSIF force_hold_level == 'record' %] - + [% ELSE %] - + [% END %] - - + + +
    - [% IF remaining_holds_for_record > 1 %]
  1. - - + +
    2. - [% ELSE %] - - [% END %] - [% END # /UNLESS multi_hold %] -
  2. - - - A non priority hold doesn't prevent a current checkout from renewing -
    3. -
+ [% IF Koha.Preference('AllowHoldItemTypeSelection') %] +
  • + + +
    • + [% END %] + [% IF remaining_holds_for_record > 1 %] +
  • + + +
    • + [% ELSE %] + + [% END %] + + - [% UNLESS ( multi_hold ) %]
    [% IF ( patron.borrowernumber ) %] [% IF ( override_required ) %] - + [% ELSIF ( none_available ) %] - + [% ELSE %] - + [% END %] [% END %]
    +
    + +
    + + [% biblio = biblioloop.0 %] + + [% IF Koha.Preference('EnableItemGroupHolds') && biblio.object.item_groups.count %] +
    + + + [% IF force_hold_level == 'item_group' %] + + (Required) + [% ELSIF force_hold_level == 'item' || force_hold_level == 'record' %] + + [% ELSE %] + + [% END %] + + + [% IF force_hold_level == 'record' # Patron has placed a record level hold previously for this record %] + + + Hold must be record level + + [% ELSIF force_hold_level == 'item' # Patron has placed an item level hold previously for this record %] + + + Hold must be item level + + [% ELSE %] +
      +
    • + + +
      • +
    • + + + + + + + + + + [% FOREACH g IN biblio.object.item_groups.search({}, { order_by => ['display_order'] }) %] + [% IF g.items.count %] + + + + + + [% ELSE %] + + + + + + [% END %] + [% END %] + +
      HoldItem groupHoldable items
      + + + + + [% FOREACH i IN g.items %] + + [% END %] +
      + + + + +
      No holdable items in this item group.
      +
      +
      • +
    + [% END %] + +
    + [% END %] + - [% biblio_info = biblioloop.0 %] + +
    + + + [% IF force_hold_level == 'item' %] + + (Required) + [% ELSIF force_hold_level == 'record' || force_hold_level == 'item_group' %] + + [% ELSE %] + + [% END %] +
      [% UNLESS Koha.Preference('item-level_itypes') %] @@ -695,77 +796,6 @@ [% END %]
    - - [% IF Koha.Preference('EnableItemGroupHolds') && biblio_info.object.item_groups.count %] -

    - Hold next available item from an item group - [% IF force_hold_level == 'item_group' %] - (Required) - [% END %] -

    - - [% IF force_hold_level == 'record' # Patron has placed a record level hold previously for this record %] - - - Hold must be record level - - [% ELSIF force_hold_level == 'item' # Patron has placed an item level hold previously for this record %] - - - Hold must be item level - - [% ELSE %] - - - - - - - - - - [% FOREACH g IN biblio_info.object.item_groups.search({}, { order_by => ['display_order'] }) %] - [% IF g.items.count %] - - - - - - [% ELSE %] - - - - - - [% END %] - [% END %] - -
    HoldItem groupHoldable items
    - - - - - [% FOREACH i IN g.items %] - - [% END %] -
    - - - - -
    No holdable items in this item group.
    -
    - [% END %] - [% END %] - - -

    - Place a hold on a specific item - [% IF force_hold_level == 'item' %] - (Required) - [% END %] -

    - @@ -808,9 +838,9 @@ Hold must be item group level [% ELSIF ( itemloo.available ) %] - + [% ELSIF ( itemloo.override ) %] - + [% ELSE %] @@ -855,7 +885,7 @@
    [% IF (itemloo.pickup_locations_count > 0) || Koha.Preference('AllowHoldPolicyOverride') %] - - - - - - [% UNLESS Koha.Preference('item-level_itypes') %] - - [% END %] - - - - [% FOREACH biblioloo IN biblioloop %] - [% IF ( biblioloo.warn ) %] - - [% ELSE %] - - [% END %] - - - + + [% END # /FOREACH biblioloo %] +
     Pickup locationTitleItem typePriorityInformation
    - [% UNLESS ( biblioloo.warn ) %] - - [% END %] - - [% UNLESS ( biblioloo.none_avail || biblioloo.noitems ) %] - +
    + Hold details + + + + + [% FOREACH biblioloo IN biblioloop %] + + [% UNLESS biblioloo.none_avail %] + + + [% END %] -
    		-
      -
    • - [% biblioloo.title | html %] - [% IF biblioloo.author %] by [% biblioloo.author | html %][% END %] -
      • - [% IF ( biblioloo.publicationyear ) %] + [% END %] + + + + + + + [% UNLESS Koha.Preference('item-level_itypes') %] + + [% END %] + + + + [% FOREACH biblioloo IN biblioloop %] + [% IF ( biblioloo.warn ) %] + + [% ELSE %] + + [% END %] + + + - [% UNLESS Koha.Preference('item-level_itypes') %] - - [% END %] - - [% END %] - [% IF ( biblioloo.alreadyres ) %] -
        - [% ELSE %] - [% IF ( biblioloo.none_avail || biblioloo.noitems ) %] +
      + - - [% END # /FOREACH biblioloo %] -
       Pickup locationTitleItem typePriorityInformation
      + [% UNLESS ( biblioloo.warn ) %] + + [% END %] + + [% UNLESS ( biblioloo.none_avail || biblioloo.noitems ) %] + + [% END %] + +
      • - Publication year: [% biblioloo.publicationyear | html %] + [% biblioloo.title | html %] + [% IF biblioloo.author %] by [% biblioloo.author | html %][% END %]
        • + [% IF ( biblioloo.publicationyear ) %] +
      • + Publication year: [% biblioloo.publicationyear | html %] +
        • + [% END %] +
      + [% IF ( biblioloo.warn ) %] + [% END %] - - [% IF ( biblioloo.warn ) %] - - [% END %] -       		- [% biblioloo.itemtype.translated_description | html %] [% biblioloo.rank | html %] - [% IF ( biblioloo.checked_previously ) %] - Patron has previously checked out this title
      + [% UNLESS Koha.Preference('item-level_itypes') %] +       		+ [% biblioloo.itemtype.translated_description | html %] + [% biblioloo.rank | html %] + [% IF ( biblioloo.checked_previously ) %] + Patron has previously checked out this title
      + [% END %] + [% IF ( biblioloo.alreadyres ) %]
        + [% ELSE %] + [% IF ( biblioloo.none_avail || biblioloo.noitems ) %] +
          + [% END %] [% END %] - [% END %] - - [% IF ( biblioloo.alreadyres ) %] -
        • - [% INCLUDE 'patron-title.inc' patron => patron no_title => 1 no_cardnumber => 1 hide_patron_infos_if_needed => 1 %] already has a hold on this item -
          • - [% END %] - [% IF ( biblioloo.none_avail || biblioloo.noitems ) %] -
        • No items are available to be placed on hold
          • - [% END %] - [% IF ( biblioloo.alreadyres ) %] -
        - [% ELSE %] + [% IF ( biblioloo.alreadyres ) %] +
      • + [% INCLUDE 'patron-title.inc' patron => patron no_title => 1 no_cardnumber => 1 hide_patron_infos_if_needed => 1 %] already has a hold on this item +
        • + [% END %] [% IF ( biblioloo.none_avail || biblioloo.noitems ) %] +
      • No items are available to be placed on hold
        • + [% END %] + + [% IF ( biblioloo.alreadyres ) %]
      + [% ELSE %] + [% IF ( biblioloo.none_avail || biblioloo.noitems ) %] + + [% END %] [% END %] - [% END %] -
      +
    + [% END # /UNLESS multi_hold %] @@ -1079,27 +1124,27 @@ [% IF ( patron AND patron.borrowernumber ) %] [% IF ( multi_hold ) %] [% IF ( override_required ) %] - + [% ELSIF ( no_bibs_available ) %] - + [% ELSIF ( none_available ) %] - + [% ELSE %] - + [% END %] [% ELSE %] [% IF ( override_required ) %] - + [% ELSIF ( none_available ) %] - + [% ELSE %] - + [% END %] [% END %] [% END # /IF patron %] - - + + [% END %] [% UNLESS ( patron ) %] @@ -1440,21 +1485,14 @@ } - function ToggleHoldsToPlace() { - if ( $("#requestany").prop('checked') ) { - $("#holds_to_place_count").prop('disabled', false); - } else { - $("#holds_to_place_count").prop('disabled', true); - } - } ToggleHoldsToPlace(); - $("#requestany").on('change', function(){ + $("#requestany,.requestspecific[name='request'],.requestgrp").on('change', function(){ ToggleHoldsToPlace(); }); [% IF Koha.Preference('UseBranchTransferLimits') %] - $("#pickup").on('change', function(){ - var pickup = $("#pickup").val(); + $("#pickup,#pickup-next-avail,#pickup-item-group").on('change', function(){ + var pickup = $(this).val(); var url = "?pickup=" + pickup; url += "&borrowernumber=" + borrowernumber; url += "&biblionumber=" + biblionumbers[0]; @@ -1467,12 +1505,6 @@ "dom": '<"top pager"ilf>t', })); - //Override fieldset styling for dataTables search box - $("div.top.pager").css("margin-left","1em"); - $(".dataTables_filter label").css({ - "width":"auto", - "margin-right":"0em" - }); $("#club-request-form").on("submit", function() { let $t = $(this); @@ -1565,7 +1597,7 @@ } }); - $("#pickup").each( function () { + $("#pickup,#pickup-item-group,#pickup-next-avail").each( function () { $(this).pickup_locations_dropdown(); }); @@ -1574,19 +1606,40 @@ }); }); + function ToggleHoldsToPlace() { + if ( $("#requestany").prop('checked') ) { + $("#holds_to_place_count, #pickup-next-avail, #itemtype, #hold_any_btn").prop('disabled', false); + $(".requestspecific,.requestgrp").prop('checked', false); + $(".requestspecific","#requestspecific").prop('disabled',true); + $("#hold_item_btn, #hold_grp_btn, #pickup-item-group").prop("disabled", true ); + $("#hold_any_btn").prop("disabled", false ); + } else if( $(".requestspecific").prop('checked') ) { + $(".requestspecific","#requestspecific").prop('disabled',false); + $("#holds_to_place_count, #pickup-item-group, #pickup-next-avail #itemtype, #hold_any_btn").prop('disabled', true); + $("#hold_item_btn").prop("disabled", false ); + $("#hold_any_btn,#hold_grp_btn").prop("disabled", true ); + $("#requestany,.requestgrp").prop('checked', false); + } else { + $("#holds_to_place_count, #pickup-next-avail, #itemtype, #hold_any_btn").prop('disabled', true); + $("#hold_grp_btn, #pickup-item-group").prop("disabled", false ); + $(".requestspecific","#requestspecific").prop('disabled',true); + $("#hold_any_btn,#hold_item_btn").prop("disabled", true ); + $("#requestany,.requestspecific").prop('checked', false); + } + } + function check( e, table ) { var msg = ""; - if ( ! $("#requestany").is(":checked") ) { + if ( $(".requestspecific").is(":checked") ) { // requestany not selected, go through the item-specific cases - if ( $('input[name="checkitem"]:checked').length > 0 ) { + var selected_items = $('#requestspecific input[name="checkitem"]:checked'); + if ( selected_items.length > 0 ) { // got item-specific hold requests in the form! // verify they have a pickup location selected - if (table.find('input[name="checkitem"]:checked') - .closest('tr') - .find(".pickup_locations").val() === null) { + if ( selected_items.closest('tr').find(".pickup_locations").val() == '' ) { msg = _("- Please select a pickup location for the item" + "\n") } @@ -1594,8 +1647,18 @@ else { msg = (_("- Please select an item to place a hold") + "\n"); } + } else if ( $("#requestgrp").is(":checked") ) { + var selected_group = $('#requestgroup input[type="radio"]:checked'); + if( selected_group.length > 0 ){ + if( $("#pickup-item-group").length < 1 || $("#pickup-item-grp").val() == "" ){ + msg = _("- Please select a pickup location for this hold" + "\n"); + } + } else { + msg = (_("- Please select an item group to place a hold") + "\n"); + } } else { - if( $("#pickup").length < 1 || $("#pickup").val() == "" || $('#pickup').val() === null ){ + // Requesting next available + if( $("#pickup-next-avail").length < 1 || $("#pickup-next-avail").val() == "" || $('#pickup-next-avail').val() === null ){ msg = _("- Please select a pickup location for this hold" + "\n"); } } @@ -1667,30 +1730,6 @@ $("button.warning").click(function() { return confirm( _("None of these items can normally be put on hold for this patron.") + "\n\n" + _("Place hold?") ); }); - $("#requestany").click(function() { - if(this.checked){ - $("input[name=checkitem]").each(function() { - $(this).prop("checked", false); - }); - } - }); - $("input[name=checkitem]").click(function() { - let onechecked = 0; - $("input[name=checkitem]").each(function() { - if(this.checked){ - onechecked++; - } - }); - if(onechecked > 0){ - $("#requestany").prop("checked", false); - $("#holds_to_place_count").prop('disabled', true); - - $("#holds_to_place_count").val(onechecked); - } else { - $("#requestany").prop("checked",true); - $("#holds_to_place_count").prop('disabled', false); - } - }); var prev_rank_request; $("select[name=rank-request]").on("focus", function() { prev_rank_request = $(this).val(); diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 44dd3f24dd..85c0d3f1eb 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -50,6 +50,7 @@ my $expirationdate = $input->param('expiration_date'); my $itemtype = $input->param('itemtype') || undef; my $non_priority = $input->param('non_priority'); my $op = $input->param('op') || q{}; +my $multi_holds = $input->param('multi_holds'); my $patron = Koha::Patrons->find( $borrowernumber ); @@ -110,7 +111,7 @@ if ( $op eq 'cud-placerequest' && $patron ) { } } - } elsif (@biblionumbers > 1) { + } elsif (@biblionumbers > 1 || $multi_holds) { my $bibinfo = $bibinfos{$biblionumber}; if ( $can_override || CanBookBeReserved($patron->borrowernumber, $biblionumber)->{status} eq 'OK' ) { AddReserve( -- 2.39.5