From 2260a07baa6d045fb78d574025d4784d3d553c15 Mon Sep 17 00:00:00 2001 From: David Gustafsson 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/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 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 Signed-off-by: Kyle M Hall --- C4/Circulation.pm | 79 ++++++++----------- C4/Reserves.pm | 96 +++++++++++------------- Koha/Biblio.pm | 18 ++--- Koha/Charges/Fees.pm | 7 +- Koha/CirculationRules.pm | 37 ++++----- Koha/Item.pm | 5 +- Koha/Patron.pm | 10 +-- Koha/REST/V1/Checkouts.pm | 6 +- Koha/Recall.pm | 10 +-- Koha/Recalls.pm | 5 +- Koha/Template/Plugin/CirculationRules.pm | 4 +- 11 files changed, 112 insertions(+), 165 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index ab03362e06..a48aac98c8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -425,16 +425,17 @@ sub TooMany { my $branch = _GetCircControlBranch($item_object->unblessed,$borrower); my $type = $item_object->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 - $parent_maxissueqty_rule = Koha::CirculationRules->get_effective_rule( + # If the parent rule is for default type we discount it + my $parent_maxissueqty; + $parent_maxissueqty = Koha::CirculationRules->get_effective_rule_value( { categorycode => $cat_borrower, itemtype => $parent_type, @@ -442,10 +443,8 @@ sub TooMany { rule_name => 'maxissueqty', } ) if $parent_type; - # If the parent rule is for default type we discount it - $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && !defined $parent_maxissueqty_rule->itemtype; - my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule( + my $maxissueqty = Koha::CirculationRules->get_effective_rule_value( { categorycode => $cat_borrower, itemtype => $type, @@ -454,7 +453,7 @@ sub TooMany { } ); - my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule( + my $maxonsiteissueqty = Koha::CirculationRules->get_effective_rule_value( { categorycode => $cat_borrower, itemtype => $type, @@ -468,19 +467,18 @@ 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 and $maxissueqty ne "") { my $checkouts; - if ( $maxissueqty_rule->branchcode ) { + if ( $branch ) { if ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) { $checkouts = $patron->checkouts->search( - { 'me.branchcode' => $maxissueqty_rule->branchcode } ); + { 'me.branchcode' => $branch } ); } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') { $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" => $branch } ); } } else { $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron @@ -488,23 +486,22 @@ sub TooMany { $checkouts = $checkouts->search(undef, { prefetch => 'item' }); my $sum_checkouts; - my $rule_itemtype = $maxissueqty_rule->itemtype; my @types; - unless ( $rule_itemtype ) { + unless ( $type ) { # matching rule has the default item type, so count only # those existing loans that don't fall under a more # specific rule @types = Koha::CirculationRules->search( { - branchcode => $maxissueqty_rule->branchcode, - categorycode => [ $maxissueqty_rule->categorycode, $cat_borrower ], + branchcode => $branch, + categorycode => $cat_borrower, itemtype => { '!=' => undef }, rule_name => 'maxissueqty' } )->get_column('itemtype'); } else { - if ( $parent_maxissueqty_rule ) { + if ( $parent_maxissueqty ) { # 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 }); @@ -523,7 +520,7 @@ sub TooMany { while ( my $c = $checkouts->next ) { my $itemtype = $c->item->effective_itemtype; - unless ( $rule_itemtype ) { + unless ( $type ) { next if grep {$_ eq $itemtype} @types; } else { next unless grep {$_ eq $itemtype} @types; @@ -542,20 +539,20 @@ sub TooMany { checkout_count => $checkout_count, onsite_checkout_count => $onsite_checkout_count, onsite_checkout => $onsite_checkout, - max_checkouts_allowed => $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef, - max_onsite_checkouts_allowed => $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef, + max_checkouts_allowed => $maxissueqty,, + max_onsite_checkouts_allowed => $maxonsiteissueqty, switch_onsite_checkout => $switch_onsite_checkout, }; # 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 ){ + $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty; 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 < $parent_maxissueqty && defined($type) ) { $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; my $qty_over = _check_max_qty($checkout_rules); return $qty_over if defined $qty_over; } @@ -599,7 +596,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 || defined $branch_borrower_circ_rule->{patron_maxissueqty} ) { return { reason => 'NO_RULE_DEFINED', max_allowed => 0 }; } @@ -1926,28 +1923,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 diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 01aa726322..bb0f530f52 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -333,7 +333,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) @@ -346,16 +346,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( { @@ -439,7 +439,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 @@ -480,8 +480,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'); @@ -498,31 +502,24 @@ sub CanItemBeReserved { } # we retrieve rights - if ( - my $reservesallowed = Koha::CirculationRules->get_effective_rule({ - itemtype => $item->effective_itemtype, - categorycode => $borrower->{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 => $borrower->{categorycode}, + branchcode => $reserves_control_branch, + rule_name => 'reservesallowed' + }) // 0; my $rights = Koha::CirculationRules->get_effective_rules({ categorycode => $borrower->{'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" }; } @@ -546,8 +543,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} ) { @@ -564,31 +561,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}; } @@ -598,25 +587,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' }; @@ -2347,13 +2336,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 43b53e17b0..a11e53fdf7 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -367,16 +367,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 @@ -390,21 +387,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 diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm index a9bbf0fe60..3815f6fa74 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 f96df496fb..f138a36375 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -300,13 +300,16 @@ 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; + } + $value //= 'undef'; + $memory_cache->set_in_cache( $cache_key, $value ); + } + return if $value eq 'undef'; return $value; } @@ -541,7 +544,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, @@ -549,8 +552,6 @@ sub get_opacitemholds_policy { rule_name => 'opacitemholds', } ); - - return $rule ? $rule->rule_value : undef; } =head3 get_onshelfholds_policy @@ -564,15 +565,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 @@ -627,7 +627,7 @@ sub get_lostreturn_policy { my $rules = Koha::CirculationRules->get_effective_rules( { branchcode => $branch, - rules => ['lostreturn','processingreturn'] + rules => ['lostreturn', 'processingreturn'] } ); @@ -714,7 +714,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, @@ -722,12 +722,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 2ad4c3fdcb..c8df57b585 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -818,7 +818,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, @@ -826,9 +826,6 @@ sub article_request_type { branchcode => $branchcode } ); - - return q{} unless $rule; - return $rule->rule_value || q{} } =head3 current_holds diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 2a8eecf9f6..3cabb307f5 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1049,7 +1049,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, @@ -1057,8 +1057,6 @@ sub can_request_article { } ); - my $limit = ($rule) ? $rule->rule_value : undef; - return 1 unless defined $limit; my $count = Koha::ArticleRequests->search( @@ -1090,15 +1088,13 @@ sub article_request_fee { $library_id //= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; - my $rule = Koha::CirculationRules->get_effective_rule( + my $fee = Koha::CirculationRules->get_effective_rule_value( { branchcode => $library_id, categorycode => $self->categorycode, rule_name => 'article_request_fee' } - ); - - my $fee = ($rule) ? $rule->rule_value + 0 : 0; + ) + 0; return $fee; } diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index ea9ccd5267..4e8bd4aeed 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -354,19 +354,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 ab390962b2..7ae159ecc0 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->unblessed, $self->patron->unblessed ); } - 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 5526f7d529..b615c215da 100644 --- a/Koha/Recalls.pm +++ b/Koha/Recalls.pm @@ -134,13 +134,12 @@ 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 = defined $recall_due_date_interval ? $recall_due_date_interval->rule_value : 5; + }) // 5; my $timestamp = dt_from_string( $recall->timestamp ); my $due_date = $timestamp->add( days => $due_interval ); $checkout->update({ date_due => $due_date }); diff --git a/Koha/Template/Plugin/CirculationRules.pm b/Koha/Template/Plugin/CirculationRules.pm index 5e00e45638..100a46f1a9 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 -- 2.30.2