Bugzilla – Attachment 120931 Details for
Bug 28338
Validate item holdability and pickup location separately
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28338: Make item-level holds use locally defined pickup branches
Bug-28338-Make-item-level-holds-use-locally-define.patch (text/plain), 11.68 KB, created by
Andrew Fuerste-Henry
on 2021-05-13 14:15:00 UTC
(
hide
)
Description:
Bug 28338: Make item-level holds use locally defined pickup branches
Filename:
MIME Type:
Creator:
Andrew Fuerste-Henry
Created:
2021-05-13 14:15:00 UTC
Size:
11.68 KB
patch
obsolete
>From 070d8cedb330238e11990de82c29c7d88206d525 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <tomascohen@theke.io> > >Signed-off-by: Owen Leonard <oleonard@myacpl.org> > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >--- > .../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 27218671ff..7b6fbf77b5 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -624,6 +624,8 @@ > Only pickup locations within the same hold group are allowed > [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %] > No reserves are allowed on this item >+ [% ELSIF itemloo.not_holdable == 'no_valid_pickup_location' %] >+ No valid pickup location > [% ELSE %] > [% itemloo.not_holdable | html %] > [% END %] >@@ -721,10 +723,10 @@ > [% END %] > </td> > <td> >- [% IF itemloo.any_pickup_location %] >- Any library >- [% ELSE %] >- [% itemloo.pickup_locations | html %] >+ [% IF (itemloo.pickup_locations_count > 0) %] >+ <select name="item_pickup" class="pickup_locations" >+ data-item-id="[% itemloo.itemnumber | html %]" >+ data-patron-id="[% patron.borrowernumber | html %]"></select> > [% END %] > </td> > </tr> >@@ -1187,7 +1189,7 @@ > > [% UNLESS ( multi_hold ) %] > $("#hold-request-form").on("submit", function(){ >- return check(); >+ return check($(this)); > }); > [% ELSE %] > $("#hold-request-form").on("submit", function(){ >@@ -1288,35 +1290,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<document.form.checkitem.length;i++){ >- if (document.form.checkitem[i].checked == true) { >- count_reserv++ ; >+ if ( $("#requestany").attr("checked") !== "checked" ) { >+ // requestany not selected, go through the item-specific cases >+ if ( $('input[type="radio"]:checked').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) { >+ >+ 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 ea81592ef0..2e715ef804 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'); >@@ -99,12 +100,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 9c83b617cf..b570818b75 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -563,8 +563,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 } ); >@@ -578,17 +577,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.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=120931">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28338
:
120896
|
120905
|
120908
|
120920
|
120921
|
120931
|
120932
|
120938
|
120939