From aadc4ac388993369a03684a5f19f816454a8fe0f Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 24 Nov 2025 11:36:05 +0100 Subject: [PATCH] Bug 41280: Fix missing branchcode for patron qty rule Content-Type: text/plain; charset=utf-8 Since we now want to check if the rule is global or not, we need more than the rule value returned by GetBranchBorrowerCircRule. This highlights another issue in this code (for a new report): the code assumes that there is a regular issue rule on the same level (global or branch) for regular issues when counting onsite issues. Theoretically, this may not be the case. Added FIXMEs. --- C4/Circulation.pm | 58 ++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 6917a04734..863378f78d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -482,17 +482,17 @@ sub TooMany { # if a rule is found and has a loan limit set, count # how many loans the patron already has that meet that # rule - if ( defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne "" ) { + if ( defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne "" ) { # FIXME Dont forget onsite checkouts my $checkouts; my $prefetch = { prefetch => 'item' }; - if ( !$maxissueqty_rule->branchcode ) { # global level: look at all checkouts + if ( !$maxissueqty_rule->branchcode ) { # global level: look at all checkouts $checkouts = $patron->checkouts( undef, $prefetch ); } elsif ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) { $checkouts = $patron->checkouts->search( { 'me.branchcode' => $branch }, $prefetch ); } elsif ( C4::Context->preference('CircControl') eq 'PatronLibrary' ) { $checkouts = $patron->checkouts->search( { "item.$branch_type" => $branch }, $prefetch ); - } else { # ItemHomeLibrary + } else { # ItemHomeLibrary $checkouts = $patron->checkouts->search( { "item.$branch_type" => $branch }, $prefetch ); } @@ -543,7 +543,7 @@ sub TooMany { } $sum_checkouts->{total}++; - $sum_checkouts->{onsite_checkouts}++ if $c->onsite_checkout; + $sum_checkouts->{onsite_checkouts}++ if $c->onsite_checkout; #FIXME Scope issue? $sum_checkouts->{itemtype}->{$itemtype}++; } @@ -586,25 +586,25 @@ sub TooMany { # Now count total loans against the limit for the branch my $branch_borrower_circ_rule = GetBranchBorrowerCircRule( $branch, $cat_borrower ); - if ( defined( $branch_borrower_circ_rule->{patron_maxissueqty} ) - and $branch_borrower_circ_rule->{patron_maxissueqty} ne '' ) - { + my $regular_rule = $branch_borrower_circ_rule->{patron_maxissueqty}; + my $onsite_rule = $branch_borrower_circ_rule->{patron_maxonsiteissueqty}; + if ( $regular_rule && $regular_rule->rule_value ne '' ) { # FIXME onsite_rule only or other level? my $checkouts; my $prefetch = { prefetch => 'item' }; - if ( !$branch_borrower_circ_rule->{branchcode} ) { # global level: look at all checkouts + if ( !$regular_rule->branchcode ) { # global level: look at all checkouts $checkouts = $patron->checkouts; } elsif ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) { $checkouts = $patron->checkouts->search( { 'me.branchcode' => $branch } ); } elsif ( C4::Context->preference('CircControl') eq 'PatronLibrary' ) { $checkouts = $patron->checkouts->search( { "item.$branch_type" => $branch }, $prefetch ); - } else { # ItemHomeLibrary + } else { # ItemHomeLibrary $checkouts = $patron->checkouts->search( { "item.$branch_type" => $branch }, $prefetch ); } my $checkout_count = $checkouts->count; my $onsite_checkout_count = $checkouts->search( { onsite_checkout => 1 } )->count; - my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty}; - my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty} || undef; + my $max_checkouts_allowed = $regular_rule->rule_value; + my $max_onsite_checkouts_allowed = $onsite_rule ? $onsite_rule->rule_value : undef; my $qty_over = _check_max_qty( { @@ -614,7 +614,8 @@ sub TooMany { max_checkouts_allowed => $max_checkouts_allowed, max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, switch_onsite_checkout => $switch_onsite_checkout, - circulation_rule => $branch_borrower_circ_rule, + circulation_rule => $regular_rule, + onsite_circulation_rule => $onsite_rule, } ); return $qty_over if defined $qty_over; @@ -2061,30 +2062,16 @@ sub GetHardDueDate { my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode); -Retrieves circulation rule attributes that apply to the given -branch and patron category, regardless of item type. -The return value is a hashref containing the following key: - -patron_maxissueqty - maximum number of loans that a -patron of the given category can have at the given -branch. If the value is undef, no limit. +Returns hash with two circulation rules (for patron_maxissueqty and +patron_maxonsiteissueqty) that apply to the given branch and +patron category, regardless of item type. -patron_maxonsiteissueqty - maximum of on-site checkouts that a -patron of the given category can have at the given -branch. If the value is undef, no limit. - -This will check for different branch/category combinations in the following order: +Checks different branch/category combinations in the following order: branch and category branch only category only default branch and category -If no rule has been found in the database, it will default to -the built in rule: - -patron_maxissueqty - undef -patron_maxonsiteissueqty - undef - C<$branchcode> and C<$categorycode> should contain the literal branch code and patron category code, respectively - no wildcards. @@ -2094,15 +2081,10 @@ wildcards. sub GetBranchBorrowerCircRule { my ( $branchcode, $categorycode ) = @_; - # Initialize default values - my $rules = { - patron_maxissueqty => undef, - patron_maxonsiteissueqty => undef, - }; - # Search for rules! + my $rules; foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) { - my $rule = Koha::CirculationRules->get_effective_rule( + $rules->{$rule_name} = Koha::CirculationRules->get_effective_rule( { categorycode => $categorycode, itemtype => undef, @@ -2110,8 +2092,6 @@ sub GetBranchBorrowerCircRule { rule_name => $rule_name, } ); - - $rules->{$rule_name} = $rule->rule_value if defined $rule; } return $rules; -- 2.39.5