Bugzilla – Attachment 178174 Details for
Bug 32092
Improve circulation rules cache utilization
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32092: Improve circulation rules cache utilization
Bug-32092-Improve-circulation-rules-cache-utilizat.patch (text/plain), 40.61 KB, created by
David Gustafsson
on 2025-02-18 12:49:15 UTC
(
hide
)
Description:
Bug 32092: Improve circulation rules cache utilization
Filename:
MIME Type:
Creator:
David Gustafsson
Created:
2025-02-18 12:49:15 UTC
Size:
40.61 KB
patch
obsolete
>From c3e08aa2ab5bdaffc5e69dfe43a777331b5b03e4 Mon Sep 17 00:00:00 2001 >From: David Gustafsson <david.gustafsson@ub.gu.se> >Date: Thu, 3 Nov 2022 14:26:34 +0100 >Subject: [PATCH] Bug 32092: Improve circulation rules cache utilization > >To test: >1) Go to the page for placing a hold for a biblio with a large > number of items >2) Depending on the number of items this page can be very slow > to load >3) Ensure the following tests pass: > t/db_dependent/Circulation.t > t/db_dependent/Reserves.t > t/db_dependent/Biblio.t > t/db_dependent/Koha/Charges/Fees.t > t/db_dependent/Items.t > t/db_dependent/Koha/Patron.t > t/db_dependent/api/v1/checkouts.t > t/db_dependent/Koha/Template/Plugin/CirculationRules.t > t/db_dependent/Template/Plugin/CirculationRules.t > t/db_dependent/Koha/CirculationRules.t > t/db_dependent/Circulation/Branch.t > t/db_dependent/Circulation/TooMany.t >3) Apply the patch >4) The page loading time should now be significantly faster >5) Ensure all the tests in 3) still pass > >Sponsored-by: Gothenburg University Library >Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Circulation.pm | 102 ++++++++++------------- > C4/Reserves.pm | 97 ++++++++++----------- > Koha/Biblio.pm | 24 ++---- > Koha/Charges/Fees.pm | 7 +- > Koha/CirculationRules.pm | 39 ++++----- > Koha/Item.pm | 12 ++- > Koha/Patron.pm | 10 +-- > Koha/REST/V1/Checkouts.pm | 6 +- > Koha/Recall.pm | 10 +-- > Koha/Recalls.pm | 11 +-- > Koha/Template/Plugin/CirculationRules.pm | 4 +- > misc/cronjobs/recalls/expire_recalls.pl | 6 +- > opac/opac-recall.pl | 4 +- > t/db_dependent/Circulation/Branch.t | 4 +- > t/db_dependent/Koha/CirculationRules.t | 5 ++ > 15 files changed, 148 insertions(+), 193 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 80cead8a96e..fdbbd8efa2e 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -422,15 +422,16 @@ sub TooMany { > my $branch = _GetCircControlBranch($item, $patron); > my $type = $item->effective_itemtype; > >- my ($type_object, $parent_type, $parent_maxissueqty_rule); >- $type_object = Koha::ItemTypes->find( $type ); >- $parent_type = $type_object->parent_type if $type_object; >+ my $type_object = Koha::ItemTypes->find( $type ); >+ my $parent_type = $type_object->parent_type if $type_object; > my $child_types = Koha::ItemTypes->search({ parent_type => $type }); > # Find any children if we are a parent_type; > > # given branch, patron category, and item type, determine > # applicable issuing rule > >+ # If the parent rule is for default type we discount it >+ my $parent_maxissueqty_rule; > $parent_maxissueqty_rule = Koha::CirculationRules->get_effective_rule( > { > categorycode => $cat_borrower, >@@ -463,7 +464,7 @@ 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 "") { > > my $checkouts; > if ( $maxissueqty_rule->branchcode ) { >@@ -474,8 +475,7 @@ sub TooMany { > $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron > } else { > my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch'; >- $checkouts = $patron->checkouts->search( >- { "item.$branch_type" => $maxissueqty_rule->branchcode } ); >+ $checkouts = $patron->checkouts->search( { "item.$branch_type" => $maxissueqty_rule->branchcode } ); > } > } else { > $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron >@@ -483,10 +483,9 @@ sub TooMany { > $checkouts = $checkouts->search(undef, { prefetch => 'item' }); > > my $sum_checkouts; >- my $rule_itemtype = $maxissueqty_rule->itemtype; > > my @types; >- unless ( $rule_itemtype || $parent_maxissueqty_rule ) { >+ unless ( $maxissueqty_rule->itemtype || $parent_maxissueqty_rule ) { > # matching rule has the default item type, so count only > # those existing loans that don't fall under a more > # specific rule >@@ -499,7 +498,7 @@ sub TooMany { > } > )->get_column('itemtype'); > } else { >- if ( $parent_maxissueqty_rule ) { >+ if ( defined $parent_maxissueqty_rule ) { > # if we have a parent item type then we count loans of the > # specific item type or its siblings or parent > my $children = Koha::ItemTypes->search({ parent_type => $parent_type }); >@@ -518,7 +517,7 @@ sub TooMany { > while ( my $c = $checkouts->next ) { > my $itemtype = $c->item->effective_itemtype; > >- unless ( $rule_itemtype || $parent_maxissueqty_rule ) { >+ unless ( $maxissueqty_rule->itemtype || $parent_maxissueqty_rule ) { > next if grep {$_ eq $itemtype} @types; > } else { > next unless grep {$_ eq $itemtype} @types; >@@ -544,15 +543,18 @@ sub TooMany { > onsite_circulation_rule => $maxonsiteissueqty_rule, > }; > # If parent rules exists >- if ( defined($parent_maxissueqty_rule) and defined($parent_maxissueqty_rule->rule_value) ){ >- $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty_rule ? $parent_maxissueqty_rule->rule_value : undef; >+ if ( defined $parent_maxissueqty_rule ) { >+ $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty_rule->rule_value; > my $qty_over = _check_max_qty($checkout_rules); > return $qty_over if defined $qty_over; > > # If the parent rule is less than or equal to the child, we only need check the parent >- if( $maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value && defined($maxissueqty_rule->itemtype) ) { >+ if( >+ $maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value >+ && defined($maxissueqty_rule->itemtype) >+ ) { > $checkout_rules->{checkout_count} = $checkout_count_type; >- $checkout_rules->{max_checkouts_allowed} = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef; >+ $checkout_rules->{max_checkouts_allowed} = $maxissueqty_rule->rule_value; > my $qty_over = _check_max_qty($checkout_rules); > return $qty_over if defined $qty_over; > } >@@ -564,7 +566,10 @@ 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 '') { >+ if ( >+ defined($branch_borrower_circ_rule->{patron_maxissueqty}) >+ and $branch_borrower_circ_rule->{patron_maxissueqty} ne '' >+ ) { > my $checkouts; > if ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) { > $checkouts = $patron->checkouts->search( >@@ -597,7 +602,7 @@ sub TooMany { > return $qty_over if defined $qty_over; > } > >- if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) { >+ unless ( defined $maxissueqty_rule || defined $branch_borrower_circ_rule->{patron_maxissueqty} ) { > return { reason => 'NO_RULE_DEFINED', max_allowed => 0 }; > } > >@@ -617,7 +622,7 @@ sub _check_max_qty { > my $onsite_circulation_rule = $params->{onsite_circulation_rule}; > > if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) { >- if ( $max_onsite_checkouts_allowed eq '' ) { return; } >+ return if $max_onsite_checkouts_allowed eq ''; > if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) { > return { > reason => 'TOO_MANY_ONSITE_CHECKOUTS', >@@ -628,7 +633,7 @@ sub _check_max_qty { > } > } > if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) { >- if ( $max_checkouts_allowed eq '' ) { return; } >+ return if $max_checkouts_allowed eq ''; > my $delta = $switch_onsite_checkout ? 1 : 0; > if ( $checkout_count >= $max_checkouts_allowed + $delta ) { > return { >@@ -640,10 +645,10 @@ sub _check_max_qty { > } > } > elsif ( not $onsite_checkout ) { >- if ( $max_checkouts_allowed eq '' ) { return; } >+ return if $max_checkouts_allowed eq ''; > if ( >- $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed ) >- { >+ $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed >+ ) { > return { > reason => 'TOO_MANY_CHECKOUTS', > count => $checkout_count - $onsite_checkout_count, >@@ -1470,22 +1475,16 @@ sub checkHighHolds { > > my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $patron ); > >- my $rule = Koha::CirculationRules->get_effective_rule_value( >+ # overrides decreaseLoanHighHoldsDuration syspref >+ my $duration = Koha::CirculationRules->get_effective_rule_value( > { > categorycode => $patron->categorycode, > itemtype => $item->effective_itemtype, > branchcode => $branchcode, > rule_name => 'decreaseloanholds', > } >- ); >+ ) || C4::Context->preference('decreaseLoanHighHoldsDuration'); > >- my $duration; >- if ( defined($rule) && $rule ne '' ){ >- # overrides decreaseLoanHighHoldsDuration syspref >- $duration = $rule; >- } else { >- $duration = C4::Context->preference('decreaseLoanHighHoldsDuration'); >- } > my $reduced_datedue = $calendar->addDuration( $issuedate, $duration ); > $reduced_datedue->set_hour($orig_due->hour); > $reduced_datedue->set_minute($orig_due->minute); >@@ -1974,27 +1973,14 @@ wildcards. > sub GetBranchBorrowerCircRule { > my ( $branchcode, $categorycode ) = @_; > >- # Initialize default values >- my $rules = { >- patron_maxissueqty => undef, >- patron_maxonsiteissueqty => undef, >- }; >- >- # Search for rules! >- foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) { >- my $rule = Koha::CirculationRules->get_effective_rule( >- { >- categorycode => $categorycode, >- itemtype => undef, >- branchcode => $branchcode, >- rule_name => $rule_name, >- } >- ); >- >- $rules->{$rule_name} = $rule->rule_value if defined $rule; >- } >- >- return $rules; >+ return Koha::CirculationRules->get_effective_rules( >+ { >+ categorycode => $categorycode, >+ itemtype => undef, >+ branchcode => $branchcode, >+ rules => ['patron_maxissueqty', 'patron_maxonsiteissueqty'] >+ } >+ ); > } > > =head2 GetBranchItemRule >@@ -2731,11 +2717,11 @@ sub _calculate_new_debar_dt { > ] > } > ); >- my $finedays = $issuing_rule ? $issuing_rule->{finedays} : undef; >- my $unit = $issuing_rule ? $issuing_rule->{lengthunit} : undef; >+ return unless $issuing_rule->{finedays}; >+ my $finedays = $issuing_rule->{finedays}; >+ my $unit = $issuing_rule->{lengthunit}; > my $chargeable_units = C4::Overdues::get_chargeable_units($unit, $dt_due, $return_date, $branchcode); > >- return unless $finedays; > > # finedays is in days, so hourly loans must multiply by 24 > # thus 1 hour late equals 1 day suspension * finedays rate >@@ -3293,14 +3279,14 @@ sub AddRenewal { > # a maximum value has been set in the circ rules > my $unseen_renewals = $issue->unseen_renewals; > if (C4::Context->preference('UnseenRenewals')) { >- my $rule = Koha::CirculationRules->get_effective_rule( >+ my $unseen_renewals_allowed = Koha::CirculationRules->get_effective_rule_value( > { categorycode => $patron->categorycode, > itemtype => $item_object->effective_itemtype, > branchcode => $circ_library->branchcode, > rule_name => 'unseen_renewals_allowed' > } > ); >- if (!$seen && $rule && looks_like_number($rule->rule_value)) { >+ if (!$seen && $unseen_renewals_allowed && looks_like_number($unseen_renewals_allowed)) { > $unseen_renewals++; > } else { > # If the renewal is seen, unseen should revert to 0 >@@ -3665,14 +3651,14 @@ sub GetIssuingCharges { > # FIXME This should follow CircControl > my $branch = C4::Context::mybranch(); > $patron //= Koha::Patrons->find( $borrowernumber ); >- my $discount = Koha::CirculationRules->get_effective_rule({ >+ my $discount = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $patron->categorycode, > branchcode => $branch, > itemtype => $item_type, > rule_name => 'rentaldiscount' > }); > if ($discount) { >- $charge = ( $charge * ( 100 - $discount->rule_value ) ) / 100; >+ $charge = ( $charge * ( 100 - $discount ) ) / 100; > } > $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 667020adef9..024ea0bf6e9 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -361,7 +361,7 @@ See CanItemBeReserved() for possible return values. > > =cut > >-sub CanBookBeReserved{ >+sub CanBookBeReserved { > my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_; > > # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) >@@ -374,16 +374,16 @@ sub CanBookBeReserved{ > > # biblio-level, item type-contrained > my $patron = Koha::Patrons->find($borrowernumber); >- my $reservesallowed = Koha::CirculationRules->get_effective_rule( >+ my $reservesallowed = Koha::CirculationRules->get_effective_rule_value( > { > itemtype => $params->{itemtype}, > categorycode => $patron->categorycode, > branchcode => $pickup_branchcode, > rule_name => 'reservesallowed', > } >- )->rule_value; >+ ); > >- $reservesallowed = ( $reservesallowed eq '' ) ? undef : $reservesallowed; >+ $reservesallowed = $reservesallowed eq '' ? undef : $reservesallowed; > > my $count = $patron->holds->search( > { >@@ -467,7 +467,7 @@ sub CanItemBeReserved { > } > > my $dbh = C4::Context->dbh; >- my $ruleitemtype; # itemtype of the matching issuing rule >+ my $effective_itemtype = $item->effective_itemtype; > my $allowedreserves = 0; # Total number of holds allowed across all records, default to none > > # We check item branch if IndependentBranches is ON >@@ -503,8 +503,12 @@ sub CanItemBeReserved { > } > > # check if a recall exists on this item from this borrower >- return _cache { status => 'recall' } >- if $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count; >+ if ( >+ C4::Context->preference('UseRecalls') && >+ $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count >+ ) { >+ return _cache { status => 'recall' }; >+ } > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > >@@ -520,32 +524,24 @@ sub CanItemBeReserved { > $reserves_control_branch = $patron->branchcode; > } > >- # we retrieve rights >- if ( >- my $reservesallowed = Koha::CirculationRules->get_effective_rule({ >- itemtype => $item->effective_itemtype, >- categorycode => $patron->categorycode, >- branchcode => $reserves_control_branch, >- rule_name => 'reservesallowed', >- }) >- ) { >- $ruleitemtype = $reservesallowed->itemtype; >- $allowedreserves = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited >- } >- else { >- $ruleitemtype = undef; >- } >+ #undefined is 0, blank is unlimited >+ $allowedreserves = Koha::CirculationRules->get_effective_rule_value({ >+ itemtype => $effective_itemtype, >+ categorycode => $patron->categorycode, >+ branchcode => $reserves_control_branch, >+ rule_name => 'reservesallowed' >+ }) // 0; > > my $rights = Koha::CirculationRules->get_effective_rules({ > categorycode => $patron->categorycode, >- itemtype => $item->effective_itemtype, >+ itemtype => $effective_itemtype, > branchcode => $reserves_control_branch, >- rules => ['holds_per_record','holds_per_day'] >+ rules => ['holds_per_record', 'holds_per_day'] > }); > my $holds_per_record = $rights->{holds_per_record} // 1; > my $holds_per_day = $rights->{holds_per_day}; > >- if ( defined $holds_per_record && $holds_per_record ne '' ){ >+ if ( defined $holds_per_record && $holds_per_record ne '' ){ > if ( $holds_per_record == 0 ) { > return _cache { status => "noReservesAllowed" }; > } >@@ -569,8 +565,8 @@ sub CanItemBeReserved { > } > > # we check if it's ok or not >- if ( defined $allowedreserves && $allowedreserves ne '' ){ >- if( $allowedreserves == 0 ){ >+ if ( $allowedreserves ne '' ) { >+ if( $allowedreserves == 0 ) { > return _cache { status => 'noReservesAllowed' }; > } > if ( !$params->{ignore_hold_counts} ) { >@@ -587,31 +583,23 @@ sub CanItemBeReserved { > > # If using item-level itypes, fall back to the record > # level itemtype if the hold has no associated item >- if ( defined $ruleitemtype ) { >- if ( C4::Context->preference('item-level_itypes') ) { >- $querycount .= q{ >- AND ( COALESCE( items.itype, biblioitems.itemtype ) = ? >- OR reserves.itemtype = ? ) >- }; >- } >- else { >- $querycount .= q{ >- AND ( biblioitems.itemtype = ? >- OR reserves.itemtype = ? ) >- }; >- } >- } >- >- my $sthcount = $dbh->prepare($querycount); >- >- if ( defined $ruleitemtype ) { >- $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype, $ruleitemtype ); >+ if ( C4::Context->preference('item-level_itypes') ) { >+ $querycount .= q{ >+ AND ( COALESCE( items.itype, biblioitems.itemtype ) = ? >+ OR reserves.itemtype = ? ) >+ }; > } > else { >- $sthcount->execute( $patron->borrowernumber, $reserves_control_branch ); >+ $querycount .= q{ >+ AND ( biblioitems.itemtype = ? >+ OR reserves.itemtype = ? ) >+ }; > } > >- my $reservecount = "0"; >+ my $sthcount = $dbh->prepare($querycount); >+ $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $effective_itemtype, $effective_itemtype ); >+ >+ my $reservecount = 0; > if ( my $rowcount = $sthcount->fetchrow_hashref() ) { > $reservecount = $rowcount->{count}; > } >@@ -621,25 +609,25 @@ sub CanItemBeReserved { > } > > # Now we need to check hold limits by patron category >- my $rule = Koha::CirculationRules->get_effective_rule( >+ my $max_holds = Koha::CirculationRules->get_effective_rule_value( > { > categorycode => $patron->categorycode, > branchcode => $reserves_control_branch, > rule_name => 'max_holds', > } > ); >- if (!$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) { >+ if (!$params->{ignore_hold_counts} && defined $max_holds && $max_holds ne '' ) { > my $total_holds_count = Koha::Holds->search( > { > borrowernumber => $patron->borrowernumber > } > )->count(); > >- return _cache { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value; >+ return _cache { status => 'tooManyReserves', limit => $max_holds } if $total_holds_count >= $max_holds; > } > > my $branchitemrule = >- C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); >+ C4::Circulation::GetBranchItemRule( $reserves_control_branch, $effective_itemtype ); > > if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) { > return _cache { status => 'notReservable' }; >@@ -2231,13 +2219,12 @@ sub GetMaxPatronHoldsForRecord { > > $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" ); > >- my $rule = Koha::CirculationRules->get_effective_rule({ >+ my $holds_per_record = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $categorycode, > itemtype => $itemtype, > branchcode => $branchcode, > rule_name => 'holds_per_record' >- }); >- my $holds_per_record = $rule ? $rule->rule_value : 0; >+ }) // 0; > $max = $holds_per_record if $holds_per_record > $max; > } > >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index 927d7b898e0..0953b367fc7 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -560,16 +560,13 @@ $borrower must be a Koha::Patron object > sub article_request_type { > my ( $self, $borrower ) = @_; > >- return q{} unless $borrower; >+ return unless $borrower; > >- my $rule = $self->article_request_type_for_items( $borrower ); >- return $rule if $rule; >+ my $type = $self->article_request_type_for_items( $borrower ); >+ return $type if $type; > > # If the record has no items that are requestable, go by the record itemtype >- $rule = $self->article_request_type_for_bib($borrower); >- return $rule if $rule; >- >- return q{}; >+ return $self->article_request_type_for_bib($borrower); > } > > =head3 article_request_type_for_bib >@@ -583,21 +580,18 @@ Returns the article request type 'yes', 'no', 'item_only', 'bib_only', for the g > sub article_request_type_for_bib { > my ( $self, $borrower ) = @_; > >- return q{} unless $borrower; >+ return unless $borrower; > > my $borrowertype = $borrower->categorycode; > my $itemtype = $self->itemtype(); > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ return Koha::CirculationRules->get_effective_rule_value( > { > rule_name => 'article_requests', > categorycode => $borrowertype, > itemtype => $itemtype, > } > ); >- >- return q{} unless $rule; >- return $rule->rule_value || q{} > } > > =head3 article_request_type_for_items >@@ -2122,9 +2116,9 @@ sub can_be_recalled { > 'on_shelf_recalls', > ], > }); >- push( @recalls_allowed, $rule->{recalls_allowed} ) if $rule; >- push( @recalls_per_record, $rule->{recalls_per_record} ) if $rule; >- push( @on_shelf_recalls, $rule->{on_shelf_recalls} ) if $rule; >+ push( @recalls_allowed, $rule->{recalls_allowed} ) if $rule->{recalls_allowed}; >+ push( @recalls_per_record, $rule->{recalls_per_record} ) if $rule->{recalls_per_record}; >+ push( @on_shelf_recalls, $rule->{on_shelf_recalls} ) if $rule->{on_shelf_recalls}; > } > my $recalls_allowed = (sort {$b <=> $a} @recalls_allowed)[0]; # take highest > my $recalls_per_record = (sort {$b <=> $a} @recalls_per_record)[0]; # take highest >diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm >index a9bbf0fe601..3815f6fa747 100644 >--- a/Koha/Charges/Fees.pm >+++ b/Koha/Charges/Fees.pm >@@ -89,8 +89,8 @@ sub new { > sub accumulate_rentalcharge { > my ($self) = @_; > >- my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); >- my $lengthunit_rule = Koha::CirculationRules->get_effective_rule( >+ my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); >+ my $units = Koha::CirculationRules->get_effective_rule_value( > { > categorycode => $self->patron->categorycode, > itemtype => $itemtype->id, >@@ -98,9 +98,8 @@ sub accumulate_rentalcharge { > rule_name => 'lengthunit', > } > ); >- return 0 unless $lengthunit_rule; >+ return 0 unless $units; > >- my $units = $lengthunit_rule->rule_value; > my $rentalcharge_increment = > ( $units eq 'days' ) > ? $itemtype->rentalcharge_daily >diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm >index d558c1e7810..a02a020bb31 100644 >--- a/Koha/CirculationRules.pm >+++ b/Koha/CirculationRules.pm >@@ -319,13 +319,18 @@ sub get_effective_rule_value { > my $cache_key = sprintf "CircRules:%s:%s:%s:%s", $rule_name // q{}, > $categorycode // q{}, $branchcode // q{}, $itemtype // q{}; > >- my $cached = $memory_cache->get_from_cache($cache_key); >- return $cached if $cached; >- >- my $rule = $self->get_effective_rule($params); >- >- my $value= $rule ? $rule->rule_value : undef; >- $memory_cache->set_in_cache( $cache_key, $value ); >+ my $value = $memory_cache->get_from_cache($cache_key); >+ unless (defined $value) { >+ my $rule = $self->get_effective_rule($params); >+ if ($rule) { >+ $value = $rule->rule_value; >+ } >+ else { >+ $value = 'undef'; >+ } >+ $memory_cache->set_in_cache( $cache_key, $value ); >+ } >+ return if $value eq 'undef'; > return $value; > } > >@@ -563,7 +568,7 @@ sub get_opacitemholds_policy { > > return unless $item or $patron; > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ return Koha::CirculationRules->get_effective_rule_value( > { > categorycode => $patron->categorycode, > itemtype => $item->effective_itemtype, >@@ -571,8 +576,6 @@ sub get_opacitemholds_policy { > rule_name => 'opacitemholds', > } > ); >- >- return $rule ? $rule->rule_value : undef; > } > > =head3 get_onshelfholds_policy >@@ -586,15 +589,14 @@ sub get_onshelfholds_policy { > my $item = $params->{item}; > my $itemtype = $item->effective_itemtype; > my $patron = $params->{patron}; >- my $rule = Koha::CirculationRules->get_effective_rule( >+ return Koha::CirculationRules->get_effective_rule_value( > { > categorycode => ( $patron ? $patron->categorycode : undef ), > itemtype => $itemtype, > branchcode => $item->holdingbranch, > rule_name => 'onshelfholds', > } >- ); >- return $rule ? $rule->rule_value : 0; >+ ) // 0; > } > > =head3 get_lostreturn_policy >@@ -649,7 +651,7 @@ sub get_lostreturn_policy { > my $rules = Koha::CirculationRules->get_effective_rules( > { > branchcode => $branch, >- rules => ['lostreturn','processingreturn'] >+ rules => ['lostreturn', 'processingreturn'] > } > ); > >@@ -736,7 +738,7 @@ sub get_effective_daysmode { > my $itemtype = $params->{itemtype}; > my $branchcode = $params->{branchcode}; > >- my $daysmode_rule = $class->get_effective_rule( >+ my $daysmode = $class->get_effective_rule_value( > { > categorycode => $categorycode, > itemtype => $itemtype, >@@ -744,12 +746,7 @@ sub get_effective_daysmode { > rule_name => 'daysmode', > } > ); >- >- return ( defined($daysmode_rule) >- and $daysmode_rule->rule_value ne '' ) >- ? $daysmode_rule->rule_value >- : C4::Context->preference('useDaysMode'); >- >+ return $daysmode || C4::Context->preference('useDaysMode'); > } > > >diff --git a/Koha/Item.pm b/Koha/Item.pm >index 07fc6ad5c8c..489fa463c43 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -1122,7 +1122,7 @@ sub article_request_type { > : undef; > my $borrowertype = $borrower->categorycode; > my $itemtype = $self->effective_itemtype(); >- my $rule = Koha::CirculationRules->get_effective_rule( >+ return Koha::CirculationRules->get_effective_rule_value( > { > rule_name => 'article_requests', > categorycode => $borrowertype, >@@ -1130,9 +1130,6 @@ sub article_request_type { > branchcode => $branchcode > } > ); >- >- return q{} unless $rule; >- return $rule->rule_value || q{} > } > > =head3 current_holds >@@ -2359,12 +2356,13 @@ sub can_be_recalled { > }); > > # check recalls allowed has been set and is not zero >- return 0 if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); >+ return 0 if !$rule->{recalls_allowed}; > > if ( $patron ) { > # check borrower has not reached open recalls allowed limit > return 0 if ( $patron->recalls->filter_by_current->count >= $rule->{recalls_allowed} ); > >+ $rule->{recalls_per_record} //= 0; > # check borrower has not reach open recalls allowed per record limit > return 0 if ( $patron->recalls->filter_by_current->search({ biblio_id => $self->biblionumber })->count >= $rule->{recalls_per_record} ); > >@@ -2433,12 +2431,12 @@ sub can_be_waiting_recall { > branchcode => $branchcode, > categorycode => $most_relevant_recall ? $most_relevant_recall->patron->categorycode : undef, > itemtype => $self->effective_itemtype, >- rules => [ 'recalls_allowed', ], >+ rules => [ 'recalls_allowed' ], > } > ); > > # check recalls allowed has been set and is not zero >- return 0 if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); >+ return 0 if !$rule->{recalls_allowed}; > > # can recall > return 1; >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 20ac75196d8..68042f06cc4 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1298,7 +1298,7 @@ sub can_request_article { > > $library_id //= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ my $limit = Koha::CirculationRules->get_effective_rule_value( > { > branchcode => $library_id, > categorycode => $self->categorycode, >@@ -1306,8 +1306,6 @@ sub can_request_article { > } > ); > >- my $limit = ($rule) ? $rule->rule_value : undef; >- > return 1 unless defined $limit; > > my $count = Koha::ArticleRequests->search( >@@ -1339,7 +1337,7 @@ sub article_request_fee { > > $library_id //= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ my $value = Koha::CirculationRules->get_effective_rule_value( > { > branchcode => $library_id, > categorycode => $self->categorycode, >@@ -1347,9 +1345,7 @@ sub article_request_fee { > } > ); > >- my $fee = ($rule) ? $rule->rule_value + 0 : 0; >- >- return $fee; >+ return $value ? $value + 0 : 0; > } > > =head3 add_article_request_fee_if_needed >diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm >index a2c76c6efa8..6c67aec5c6a 100644 >--- a/Koha/REST/V1/Checkouts.pm >+++ b/Koha/REST/V1/Checkouts.pm >@@ -415,19 +415,19 @@ sub allows_renewal { > my $renewable = Mojo::JSON->false; > $renewable = Mojo::JSON->true if $can_renew; > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ my $renewalsallowed = Koha::CirculationRules->get_effective_rule_value( > { > categorycode => $checkout->patron->categorycode, > itemtype => $checkout->item->effective_itemtype, > branchcode => $checkout->branchcode, >- rule_name => 'renewalsallowed', >+ rule_name => 'renewalsallowed' > } > ); > return $c->render( > status => 200, > openapi => { > allows_renewal => $renewable, >- max_renewals => $rule->rule_value, >+ max_renewals => $renewalsallowed, > current_renewals => $checkout->renewals_count, > unseen_renewals => $checkout->unseen_renewals, > error => $error >diff --git a/Koha/Recall.pm b/Koha/Recall.pm >index a060a5bf3a0..5cfe949e781 100644 >--- a/Koha/Recall.pm >+++ b/Koha/Recall.pm >@@ -115,13 +115,13 @@ sub checkout { > my @items = Koha::Items->search({ biblionumber => $self->biblio_id })->as_list; > my @itemnumbers; > foreach (@items) { >- my $recalls_allowed = Koha::CirculationRules->get_effective_rule({ >+ my $recalls_allowed = Koha::CirculationRules->get_effective_rule_value({ > branchcode => C4::Context->userenv->{'branch'}, > categorycode => $self->patron->categorycode, > itemtype => $_->effective_itemtype, > rule_name => 'recalls_allowed', > }); >- if ( defined $recalls_allowed and $recalls_allowed->rule_value > 0 ) { >+ if ( defined $recalls_allowed and $recalls_allowed > 0 ) { > push ( @itemnumbers, $_->itemnumber ); > } > } >@@ -261,14 +261,12 @@ sub calc_expirationdate { > $branchcode = C4::Circulation::_GetCircControlBranch( $item, $self->patron ); > } > >- my $rule = Koha::CirculationRules->get_effective_rule({ >+ my $shelf_time = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $self->patron->categorycode, > branchcode => $branchcode, > itemtype => $item ? $item->effective_itemtype : undef, > rule_name => 'recall_shelf_time' >- }); >- >- my $shelf_time = defined $rule ? $rule->rule_value : C4::Context->preference('RecallsMaxPickUpDelay'); >+ }) // C4::Context->preference('RecallsMaxPickUpDelay'); > > my $expirationdate = dt_from_string->add( days => $shelf_time ); > return $expirationdate; >diff --git a/Koha/Recalls.pm b/Koha/Recalls.pm >index 332f1deb79e..f56cecbab37 100644 >--- a/Koha/Recalls.pm >+++ b/Koha/Recalls.pm >@@ -135,18 +135,15 @@ sub add_recall { > > # get checkout and adjust due date based on circulation rules > my $checkout = $recall->checkout; >- my $recall_due_date_interval = Koha::CirculationRules->get_effective_rule({ >+ my $due_interval = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $checkout->patron->categorycode, > itemtype => $checkout->item->effective_itemtype, > branchcode => $branchcode, > rule_name => 'recall_due_date_interval', >- }); >- my $due_interval = 5; >- $due_interval = $recall_due_date_interval->rule_value >- if defined $recall_due_date_interval && $recall_due_date_interval->rule_value; >- my $timestamp = dt_from_string( $recall->timestamp ); >+ }) // 5; >+ my $timestamp = dt_from_string( $recall->timestamp ); > my $checkout_due_date = dt_from_string( $checkout->date_due ); >- my $recall_due_date = $timestamp->set( >+ my $recall_due_date = $timestamp->set( > { > hour => $checkout_due_date->hour, minute => $checkout_due_date->minute, > second => $checkout_due_date->second >diff --git a/Koha/Template/Plugin/CirculationRules.pm b/Koha/Template/Plugin/CirculationRules.pm >index 5e00e456380..100a46f1a96 100644 >--- a/Koha/Template/Plugin/CirculationRules.pm >+++ b/Koha/Template/Plugin/CirculationRules.pm >@@ -46,7 +46,7 @@ sub Get { > $categorycode = undef if $categorycode eq q{} or $categorycode eq q{*}; > $itemtype = undef if $itemtype eq q{} or $itemtype eq q{*}; > >- my $rule = Koha::CirculationRules->get_effective_rule( >+ return Koha::CirculationRules->get_effective_rule_value( > { > branchcode => $branchcode, > categorycode => $categorycode, >@@ -54,8 +54,6 @@ sub Get { > rule_name => $rule_name, > } > ); >- >- return $rule->rule_value if $rule; > } > > =head3 Search >diff --git a/misc/cronjobs/recalls/expire_recalls.pl b/misc/cronjobs/recalls/expire_recalls.pl >index 71d9c2bcaef..a03c86864f6 100755 >--- a/misc/cronjobs/recalls/expire_recalls.pl >+++ b/misc/cronjobs/recalls/expire_recalls.pl >@@ -50,7 +50,7 @@ while( my $recall = $recalls->next ) { > } > } > if ( $recall->waiting ) { >- my $recall_shelf_time = Koha::CirculationRules->get_effective_rule({ >+ my $recall_shelf_time = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $recall->patron->categorycode, > itemtype => $recall->item->effective_itemtype, > branchcode => $recall->pickup_library_id, >@@ -58,8 +58,8 @@ while( my $recall = $recalls->next ) { > }); > my $waitingdate = dt_from_string( $recall->waiting_date )->truncate( to => 'day' ); > my $days_waiting = $today->subtract_datetime( $waitingdate ); >- if ( defined $recall_shelf_time and $recall_shelf_time->rule_value >= 0 ) { >- if ( $days_waiting->days > $recall_shelf_time->rule_value ) { >+ if ( defined $recall_shelf_time and $recall_shelf_time >= 0 ) { >+ if ( $days_waiting->days > $recall_shelf_time ) { > # recall has been awaiting pickup for longer than the circ rules allow > $recall->set_expired({ interface => 'COMMANDLINE' }); > } >diff --git a/opac/opac-recall.pl b/opac/opac-recall.pl >index eed014cbf5f..11174f3cbc7 100755 >--- a/opac/opac-recall.pl >+++ b/opac/opac-recall.pl >@@ -48,13 +48,13 @@ if ( C4::Context->preference('UseRecalls') ) { > # check if already recalled > my $recalled = $biblio->recalls->filter_by_current->search({ patron_id => $borrowernumber })->count; > if ( defined $recalled and $recalled > 0 ) { >- my $recalls_per_record = Koha::CirculationRules->get_effective_rule({ >+ my $recalls_per_record = Koha::CirculationRules->get_effective_rule_value({ > categorycode => $patron->categorycode, > branchcode => undef, > itemtype => undef, > rule_name => 'recalls_per_record' > }); >- if ( defined $recalls_per_record and $recalls_per_record->rule_value and $recalled >= $recalls_per_record->rule_value ){ >+ if ( defined $recalls_per_record and $recalls_per_record and $recalled >= $recalls_per_record ) { > $error = 'duplicate'; > } > } >diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t >index 80a8cc61427..2cc259baf24 100755 >--- a/t/db_dependent/Circulation/Branch.t >+++ b/t/db_dependent/Circulation/Branch.t >@@ -143,8 +143,8 @@ my $borrower_id1 = Koha::Patron->new({ > > is_deeply( > GetBranchBorrowerCircRule(), >- { patron_maxissueqty => undef, patron_maxonsiteissueqty => undef }, >-"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined" >+ undef, >+ "Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined" > ); > > Koha::CirculationRules->set_rules( >diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t >index 90eb827aff0..a272ea92037 100755 >--- a/t/db_dependent/Koha/CirculationRules.t >+++ b/t/db_dependent/Koha/CirculationRules.t >@@ -628,9 +628,13 @@ subtest 'get_onshelfholds_policy() tests' => sub { > } > ); > >+ my $memory_cache = Koha::Cache::Memory::Lite->get_instance; >+ $memory_cache->flush(); >+ > is( $circ_rules->get_onshelfholds_policy({ item => $item }), 1, 'If rule_value is set on a matching rule, return it' ); > # Delete the rule (i.e. get_effective_rule returns undef) > $circ_rules->delete; >+ $memory_cache->flush(); > is( $circ_rules->get_onshelfholds_policy({ item => $item }), 0, 'If no matching rule, fallback to 0' ); > > $schema->storage->txn_rollback; >@@ -901,6 +905,7 @@ subtest 'get_lostreturn_policy() tests' => sub { > ) > ->next > ->delete; >+ $memory_cache->flush(); > is_deeply( Koha::CirculationRules->get_lostreturn_policy( $params ), > { lostreturn => 'refund', processingreturn => 'refund' },'No rule for branch, no default rule, fallback default (refund)'); > >-- >2.48.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32092
:
143120
|
146956
|
147149
|
147150
|
147151
|
149782
|
149783
|
151195
|
151196
|
151197
|
151709
|
151710
|
151711
|
153168
|
153169
|
153170
|
157534
|
157535
|
157536
|
157537
|
157551
| 178174