From deb634cee66f665fdf68bf2cf09b2c13a67d9ecd 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 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly --- .../prog/en/modules/reserve/request.tt | 587 ++++++++++-------- reserve/placerequest.pl | 3 +- 2 files changed, 315 insertions(+), 275 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 fb095ed66c..ac28066f46 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -509,70 +509,39 @@
-
- Hold details + [% UNLESS ( multi_hold ) %]
+
+ Hold details - - [% FOREACH biblionumber IN biblionumbers %] - - [% END %] - [% IF ( multi_hold ) %] - - - [% FOREACH biblioloo IN biblioloop %] - [% UNLESS biblioloo.none_avail %] - - - - [% END %] + + [% FOREACH biblionumber IN biblionumbers %] + [% END %] - [% ELSE %] - - - - [% END # /IF multi_hold %] + + + -
    -
  1. - Patron: - [% IF ( patron.borrowernumber ) %] - [% INCLUDE 'patron-title.inc' patron => patron no_title => 1 no_cardnumber => 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 no_cardnumber => 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. @@ -584,7 +553,6 @@
      7. [% END %] - [% END # /UNLESS multi_hold %] [% IF ( Koha.Preference('AllowHoldDateInFuture') ) %]
    7. @@ -598,51 +566,166 @@
      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 %] +
    + + [% 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 %] + + + + + [% END %] + [% END %] + +
      HoldItem group
      + + + +
      +
      • +
    + [% END %] + +
    + [% END %] + + + +
    + + + [% IF force_hold_level == 'item' %] + + (Required) + [% ELSIF force_hold_level == 'record' || force_hold_level == 'item_group' %] + + [% ELSE %] + + [% END %] +
      [% UNLESS Koha.Preference('item-level_itypes') %] @@ -660,59 +743,6 @@ [% END %]
    - - [% IF Koha.Preference('EnableItemGroupHolds') && biblio.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.object.item_groups.search({}, { order_by => ['display_order'] }) %] - [% IF g.items %] - - - - - [% END %] - [% END %] - -
    HoldItem group
    - - - -
    - [% END %] - [% END %] - - -

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

    - @@ -755,9 +785,9 @@ Hold must be item group level [% ELSIF ( itemloo.available ) %] - + [% ELSIF ( itemloo.override ) %] - + [% ELSE %] @@ -802,7 +832,7 @@
    [% IF (itemloo.pickup_locations_count > 0) %] -
    [% IF hiddencount %] - -

    - Show all items ([% hiddencount | html %] hidden) -

    - +

    + Show all items ([% hiddencount | html %] hidden) +

    [% END # /IF hiddencount %] [% ELSE # /UNLESS multi_hold %] - - - - - - - [% 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 %] @@ -1027,27 +1070,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 ) %] @@ -1365,21 +1408,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]; @@ -1392,12 +1428,6 @@ "sDom": '<"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); @@ -1490,7 +1520,7 @@ } }); - $("#pickup").each( function () { + $("#pickup,#pickup-item-group,#pickup-next-avail").each( function () { $(this).pickup_locations_dropdown(); }); @@ -1499,19 +1529,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[type="radio"]:checked').length > 0 ) { + var selected_item = $('#requestspecific input[type="radio"]:checked'); + if ( selected_item.length > 0 ) { // got item-specific hold requests in the form! // verify they have a pickup location selected - if (table.find('input[type="radio"]:checked') - .closest('tr') - .find(".pickup_locations").val() === null) { + if ( selected_item.closest('tr').find(".pickup_locations").val() == '' ) { msg = _("- Please select a pickup location for the item" + "\n") } @@ -1519,8 +1570,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() == "" ){ + // Requesting next available + if( $("#pickup-next-avail").length < 1 || $("#pickup-next-avail").val() == "" ){ msg = _("- Please select a pickup location for this hold" + "\n"); } } @@ -1592,28 +1653,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() { - onechecked = 0; - $("input[name=checkitem]").each(function() { - if(this.checked){ - onechecked = 1; - } - }); - if(onechecked == 1){ - $("#requestany").prop("checked", false); - $("#holds_to_place_count").prop('disabled', true); - } 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 a253fb5547..3495833a2e 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -49,6 +49,7 @@ my $item_group_id = $input->param('item_group_id'); my $expirationdate = $input->param('expiration_date'); my $itemtype = $input->param('itemtype') || undef; my $non_priority = $input->param('non_priority'); +my $multi_holds = $input->param('multi_holds'); my $patron = Koha::Patrons->find( $borrowernumber ); @@ -103,7 +104,7 @@ if ( $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.30.2