Attachment 161633 Details for Bug 34886 – Bug 34886: Adjust holdability checks on opac details page

[patch] Bug 34886: Adjust holdability checks on opac details page

Bug-34886-Adjust-holdability-checks-on-opac-detail.patch (text/plain), 4.88 KB, created by Nick Clemens (kidclamp) on 2024-01-30 12:53:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2024-01-30 12:53:16 UTC

Size: 4.88 KB

patch

obsolete

>From 0ff42fa3d7238aa84d4523ad2a335ac9f83bde4b Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Thu, 28 Sep 2023 12:27:21 +0000
>Subject: [PATCH] Bug 34886: Adjust holdability checks on opac details page
>
>This patch tries to simplify some of the logic here to match that on the search results. When we don't have a patron, we fallback to determining if an item can be held buy determining whether there are any items that don't have holds disallowed at the all libraries level. We also remove items with non-holdable statuses like withdrawn etc (and check some system preferences)
>
>If we don't have a patron, then we are done, however, if we do, then we need to check each item against the policies related to that patron.
>
>This patch also removes two checks at the end:
>CountItemsIssued($biblionumber)
>$biblio->has_items_waiting_or_intransit
>
>These seem to be from bug 4319 - however, those rules are checked by IsAvailableForItemLevelRequest and are only relevant when we have a patron. These checks essentially assumed 'onshelfholds' policy of 'If any unavailable' For consistency sake I think we should follow the same logic as the results page.
>
>To test:
>1 - Find a record with two items, of different types, set a 'Default checkout, hold and return policy' of 'No holds allowed'
>2 - Search opac, not logged in, and verify neither the results page or details page shows the place hold button
>3 - Delete that rule, make both items withdrawn
>4 - Search opac, not logged in, and verify neither the results page or details page shows the place hold button
>5 - Mark one item as not withdrawn
>6 - Search opac, not logged in, and verify both the results page or details page shows the place hold button
>7 - Log in to opac
>8 - Search opac, logged in, and verify both the results page or details page shows the place hold button
>9 - Place an 'On shelf holds policy' rule for that patron category of 'If any unavailable'
>10 - Search opac, logged in, and verify the results and details page shows the place hold button
>11 - Set the other item to not withdrawn
>12 - Search opac, logged in, and verify the results page shows the place hold button, but details does not
>13 - Try various other scenarios - details page should be more correct, results page is always an approximation
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> opac/opac-detail.pl | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index b9c644e38d7..e1d3f5cefc7 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -662,7 +662,13 @@ if ( C4::Context->preference('OPACAcquisitionDetails' ) ) {
>     };
> }
> 
>-my $can_item_be_reserved = 0;
>+# Count the number of items that allow holds
>+my $holdable_items = $biblio->items->filter_by_for_hold->count;
>+# If we have a patron and there are no holdable items - we set to no
>+# If we have a patron and there are holdable items we set to no - and we need to check each item specifically
>+# If we don't have a patron, then holdable items determines holdability
>+my $can_holds_be_placed = $patron && $holdable_items ? 0 : $holdable_items;
>+
> my ( $itemloop_has_images, $otheritemloop_has_images );
> if ( not $viewallitems and $items->count > $max_items_to_display ) {
>     $template->param(
>@@ -686,7 +692,11 @@ else {
>         $item_info->{holding_library_info} = $opac_info_holding->content if $opac_info_holding;
>         $item_info->{home_library_info} = $opac_info_home->content if $opac_info_home;
> 
>-        $can_item_be_reserved = $can_item_be_reserved || $patron && IsAvailableForItemLevelRequest($item, $patron, undef);
>+        # We only need to check if we haven't determined holds can be place, and there are items that can
>+        # be held, and we have a patron to determine holdability
>+        if( !$can_holds_be_placed && $holdable_items && $patron ){
>+            $can_holds_be_placed = IsAvailableForItemLevelRequest($item, $patron, undef);
>+        }
> 
>         # get collection code description, too
>         my $ccode = $item->ccode;
>@@ -762,9 +772,7 @@ else {
>     }
> }
> 
>-if( $can_item_be_reserved || CountItemsIssued($biblionumber) || $biblio->has_items_waiting_or_intransit ) {
>-    $template->param( ReservableItems => 1 );
>-}
>+$template->param( ReservableItems => $can_holds_be_placed );
> 
> $template->param(
>     itemloop_has_images      => $itemloop_has_images,
>@@ -792,10 +800,8 @@ if( C4::Context->preference('ArticleRequests') ) {
>     $template->param( artreqpossible => $artreqpossible );
> }
> 
>-my $norequests = ! $biblio->items->filter_by_for_hold->count;
>     $template->param(
>                      MARCNOTES               => $marcnotesarray,
>-                     norequests              => $norequests,
>                      itemdata_ccode          => $itemfields{ccode},
>                      itemdata_materials      => $itemfields{materials},
>                      itemdata_enumchron      => $itemfields{enumchron},
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 34886: 156107 | 156114 | 156150 | 156151 | 156152 | 156153 | 156277 | 156278 | 156279 | 156293 | 156294 | 156295 | 156326 | 156327 | 156980 | 156981 | 159678 | 159679 | 161633 | 161634 | 161981 | 161982 | 163674 | 164106 | 164107 | 164108 | 164109 | 164110 | 164159 | 164160 | 164161