From 7fbb07ba59373f9b815c67c3737d1cbafa397736 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 12 May 2021 15:19:29 -0300 Subject: [PATCH] Bug 28338: Make item-level holds use locally defined pickup branches Besides the commit subject, this patch does much more: - It makes request.pl stop passing a pickup location to CanItemBeReserved - It makes the page use the API to render a dropdown for each item, with their valid pickup locations - Items with no valid pickup locations have a nice message about why they are disabled for selection To test: 1. Apply this patch 2. Choose a biblio for placing a hold 3. Choose a patron => SUCCESS: You are presented with a new layout, that includes a dropdown for choosing each item's pickup location. If an item is not holdable, it still isn't. 4. Try having an item whose home branch is not marked as a pickup location => SUCCESS: Notice you cannot choose that item 5. CHoose an item, but do not choose a branch, and click 'Place hold' => SUCCESS: It shows an alert about the need to choose a pickup location 6. Choose one of the (only possible) pickup locations for the specific item 7. Place the item level hold => SUCCESS: All goes as expected! 8. Sign off :D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Owen Leonard Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- .../prog/en/modules/reserve/request.tt | 84 +++++++++++++------ reserve/placerequest.pl | 5 +- reserve/request.pl | 19 ++--- 3 files changed, 70 insertions(+), 38 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 00ceeee8c2..abdbc8f293 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -631,6 +631,8 @@ No reserves are allowed on this item [% ELSIF itemloo.not_holdable == 'libraryNotPickupLocation' %] Library is not a pickup location + [% ELSIF itemloo.not_holdable == 'no_valid_pickup_location' %] + No valid pickup location [% ELSE %] [% itemloo.not_holdable | html %] [% END %] @@ -728,10 +730,10 @@ [% END %] - [% IF itemloo.any_pickup_location %] - Any library - [% ELSE %] - [% itemloo.pickup_locations | html %] + [% IF (itemloo.pickup_locations_count > 0) %] + [% END %] @@ -1206,7 +1208,7 @@ [% UNLESS ( multi_hold ) %] $("#hold-request-form").on("submit", function(){ - return check(); + return check($(this)); }); [% ELSE %] $("#hold-request-form").on("submit", function(){ @@ -1327,35 +1329,67 @@ templateResult: display_pickup_location }); }); + $(".pickup_locations").each( function () { + var this_dropdown = $(this); + var patron_id = $(this).data('patron-id'); + var item_id = $(this).data('item-id'); + + this_dropdown.select2({ + allowClear: true, + ajax: { + url: '/api/v1/items/' + encodeURIComponent(item_id) + '/pickup_locations', + delay: 300, // wait 300 milliseconds before triggering the request + dataType: 'json', + data: function (params) { + var search_term = (params.term === undefined) ? '' : params.term; + var query = { + "q": JSON.stringify({"name":{"-like":search_term+'%'}}), + "_order_by": "name", + "patron_id": patron_id + }; + return query; + }, + processResults: function (data) { + var results = []; + data.forEach( function ( pickup_location ) { + results.push( + { + "id": pickup_location.library_id.escapeHtml(), + "text": pickup_location.name.escapeHtml(), + "needs_override": pickup_location.needs_override + } + ); + }); + return { "results": results }; + } + }, + templateResult: display_pickup_location + }); + }); }); - function check() { + function check( table ) { + var msg = ""; - var count_reserv = 0; - // check if we have checkitem form - if (document.form.checkitem){ - for (i=0;i 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) { + + msg = _("- Please select a pickup location for the item" + "\n") } } - // for only one item, check the checkitem without consider the loop checkitem - if (i==0){ - if (document.form.checkitem.checked == true) { - count_reserv++; - } + else { + msg = (_("- Please select an item to place a hold") + "\n"); } } - if (document.form.requestany.checked == true){ - count_reserv++ ; - } - - if (count_reserv == "0"){ - msg += (_("- Please select an item to place a hold") + "\n"); - } - if (msg == "") { $('#hold-request-form').preventDoubleFormSubmit(); return(true); diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 05b5c7f3c9..35e24b43f1 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -45,6 +45,7 @@ my $biblionumber = $input->param('biblionumber'); my $borrowernumber = $input->param('borrowernumber'); my $notes = $input->param('notes'); my $branch = $input->param('pickup'); +my $item_pickup = $input->param('item_pickup'); my $startdate = $input->param('reserve_date') || ''; my @rank = $input->multi_param('rank-request'); my $type = $input->param('type'); @@ -100,12 +101,12 @@ if ( $type eq 'str8' && $borrower ) { $biblionumber = $item->biblionumber; } - my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $branch)->{status}; + my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $item_pickup)->{status}; if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) { AddReserve( { - branchcode => $branch, + branchcode => $item_pickup, borrowernumber => $borrower->{'borrowernumber'}, biblionumber => $biblionumber, priority => $rank[0], diff --git a/reserve/request.pl b/reserve/request.pl index 7637692c99..63321af627 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -570,8 +570,7 @@ foreach my $biblionumber (@biblionumbers) { $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - my $reserves_control_branch = $pickup || C4::Reserves::GetReservesControlBranch( $item, $patron_unblessed ); - my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber, $reserves_control_branch )->{status}; + my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{status}; $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' ); $item->{item_level_holds} = Koha::CirculationRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); @@ -585,17 +584,15 @@ foreach my $biblionumber (@biblionumbers) { && IsAvailableForItemLevelRequest($item_object, $patron, undef, $items_any_available) ) { - $item->{available} = 1; - $num_available++; - - if ( $branchitemrule->{'hold_fulfillment_policy'} eq 'any' ) - { - $item->{any_pickup_location} = 1; + # Send the pickup locations count to the UI, the pickup locations will be pulled using the API + $item->{pickup_locations_count} = $item_object->pickup_locations({ patron => $patron })->count; + if ( $item->{pickup_locations_count} > 0 ) { + $num_available++; + $item->{available} = 1; } else { - my @pickup_locations = $item_object->pickup_locations({ patron => $patron }); - - $item->{pickup_locations} = join( ', ', map { $_->branchname } @pickup_locations ); + $item->{available} = 0; + $item->{not_holdable} = "no_valid_pickup_location"; } push( @available_itemtypes, $item->{itype} ); -- 2.39.5