Attachment 139635 Details for Bug 30847 – Bug 30847: Consolidate code to check if patron can place holds and exit if they cannot

[patch] Bug 30847: Consolidate code to check if patron can place holds and exit if they cannot

Bug-30847-Consolidate-code-to-check-if-patron-can-.patch (text/plain), 5.40 KB, created by Victor Grousset/tuxayo on 2022-08-23 00:18:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Grousset/tuxayo

Created: 2022-08-23 00:18:25 UTC

Size: 5.40 KB

patch

obsolete

>From aa8909146a94abb602151f23df74e293e7b69590 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Wed, 25 May 2022 14:58:36 +0000
>Subject: [PATCH] Bug 30847: Consolidate code to check if patron can place
> holds and exit if they cannot
>
>This will save us fetching more info if we are denying the holds
>
>Signed-off-by: Owen Leonard <oleonard@myacpl.org>
>Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>---
> opac/opac-reserve.pl | 136 +++++++++++++++++++++----------------------
> 1 file changed, 67 insertions(+), 69 deletions(-)
>
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index b69ac99ddf..619a881c9f 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -72,18 +72,6 @@ unless ( $can_place_hold_if_available_at_pickup ) {
>     }
> }
> 
>-# check if this user can place a reserve, -1 means use sys pref, 0 means dont block, 1 means block
>-if ( $category->effective_BlockExpiredPatronOpacActions ) {
>-
>-    if ( $patron->is_expired ) {
>-
>-        # cannot reserve, their card has expired and the rules set mean this is not allowed
>-        $template->param( message => 1, expired_patron => 1 );
>-        output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 };
>-        exit;
>-    }
>-}
>-
> # Pass through any reserve charge
> my $reservefee = $category->reservefee;
> if ( $reservefee > 0){
>@@ -123,6 +111,69 @@ if (($#biblionumbers < 0) && (! $query->param('place_reserve'))) {
>     exit;
> }
> 
>+#
>+#
>+# Here we check that the borrower can actually make reserves Stage 1.
>+#
>+#
>+my $noreserves     = 0;
>+if ( $category->effective_BlockExpiredPatronOpacActions ) {
>+    if ( $patron->is_expired ) {
>+        # cannot reserve, their card has expired and the rules set mean this is not allowed
>+        $noreserves = 1;
>+        $template->param( message => 1, expired_patron => 1 );
>+    }
>+}
>+
>+my $maxoutstanding = C4::Context->preference("maxoutstanding");
>+my $amountoutstanding = $patron->account->balance;
>+if ( $amountoutstanding && ($amountoutstanding > $maxoutstanding) ) {
>+    my $amount = sprintf "%.02f", $amountoutstanding;
>+    $template->param( message => 1 );
>+    $noreserves = 1;
>+    $template->param( too_much_oweing => $amount );
>+}
>+
>+if ( $patron->gonenoaddress && ($patron->gonenoaddress == 1) ) {
>+    $noreserves = 1;
>+    $template->param(
>+        message => 1,
>+        GNA     => 1
>+    );
>+}
>+
>+if ( $patron->lost && ($patron->lost == 1) ) {
>+    $noreserves = 1;
>+    $template->param(
>+        message => 1,
>+        lost    => 1
>+    );
>+}
>+
>+if ( $patron->is_debarred ) {
>+    $noreserves = 1;
>+    $template->param(
>+        message          => 1,
>+        debarred         => 1,
>+        debarred_comment => $patron->debarredcomment,
>+        debarred_date    => $patron->debarred,
>+    );
>+}
>+
>+my $holds = $patron->holds;
>+my $reserves_count = $holds->count;
>+$template->param( RESERVES => $holds->unblessed );
>+if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) {
>+    $template->param( message => 1 );
>+    $noreserves = 1;
>+    $template->param( too_many_reserves => $holds->count );
>+}
>+
>+if( $noreserves ){
>+    output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 };
>+    exit;
>+}
>+
> 
> # pass the pickup branch along....
> my $branch = $query->param('branch') || $patron->branchcode || C4::Context->userenv->{branch} || '' ;
>@@ -309,66 +360,13 @@ foreach my $biblioNumber (@biblionumbers) {
>     $biblioData->{rank} = $reservecount + 1;
> }
> 
>-#
>-#
>-# Here we check that the borrower can actually make reserves Stage 1.
>-#
>-#
>-my $noreserves     = 0;
>-my $maxoutstanding = C4::Context->preference("maxoutstanding");
>-my $amountoutstanding = $patron->account->balance;
>-if ( $amountoutstanding && ($amountoutstanding > $maxoutstanding) ) {
>-    my $amount = sprintf "%.02f", $amountoutstanding;
>-    $template->param( message => 1 );
>-    $noreserves = 1;
>-    $template->param( too_much_oweing => $amount );
>-}
> 
>-if ( $patron->gonenoaddress && ($patron->gonenoaddress == 1) ) {
>-    $noreserves = 1;
>-    $template->param(
>-        message => 1,
>-        GNA     => 1
>-    );
>+my $requested_reserves_count = scalar( @biblionumbers );
>+if ( $maxreserves && ( $reserves_count + $requested_reserves_count > $maxreserves ) ) {
>+    $template->param( new_reserves_allowed => $maxreserves - $reserves_count );
> }
> 
>-if ( $patron->lost && ($patron->lost == 1) ) {
>-    $noreserves = 1;
>-    $template->param(
>-        message => 1,
>-        lost    => 1
>-    );
>-}
>-
>-if ( $patron->is_debarred ) {
>-    $noreserves = 1;
>-    $template->param(
>-        message          => 1,
>-        debarred         => 1,
>-        debarred_comment => $patron->debarredcomment,
>-        debarred_date    => $patron->debarred,
>-    );
>-}
>-
>-my $holds = $patron->holds;
>-my $reserves_count = $holds->count;
>-$template->param( RESERVES => $holds->unblessed );
>-if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) {
>-    $template->param( message => 1 );
>-    $noreserves = 1;
>-    $template->param( too_many_reserves => $holds->count );
>-}
>-
>-unless ( $noreserves ) {
>-    my $requested_reserves_count = scalar( @biblionumbers );
>-    if ( $maxreserves && ( $reserves_count + $requested_reserves_count > $maxreserves ) ) {
>-        $template->param( new_reserves_allowed => $maxreserves - $reserves_count );
>-    }
>-}
>-
>-unless ($noreserves) {
>-    $template->param( select_item_types => 1 );
>-}
>+$template->param( select_item_types => 1 );
> 
> 
> #
>-- 
>2.37.2

Actions: View | Diff | Splinter Review

Attachments on bug 30847: 135340 | 135341 | 135342 | 135343 | 135344 | 135345 | 135346 | 135347 | 135348 | 137668 | 137669 | 137670 | 137671 | 137672 | 137673 | 137674 | 137675 | 138654 | 138655 | 138934 | 138935 | 138936 | 138937 | 138938 | 138939 | 138940 | 138941 | 139628 | 139629 | 139630 | 139631 | 139632 | 139633 | 139634 | 139635 | 139915 | 139916 | 139917 | 139918 | 139919 | 139920 | 139921 | 139922 | 139923