Attachment #153628 for bug #34178

View | Details | Raw Unified | Return to bug 34178
Collapse All | Expand All





use C4::Context;
use C4::Stats qw( UpdateStats );
use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest );
use C4::Biblio qw( UpdateTotalIssues );
use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf );
use C4::Accounts;

            foreach my $possible_hold (@possible_holds) {
                my $fillable = 0;
                my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
                my $items_any_available = ItemsAnyAvailableAndNotRestricted({
                    biblionumber => $item->biblionumber,
                    patron => $patron_with_reserve
                });

                # FIXME: We are not checking whether the item we are renewing can fill the hold

                foreach my $other_item (@other_items) {
                  next if defined $matched_items{$other_item->itemnumber};
                  next if IsItemOnHoldAndFound( $other_item->itemnumber );
                  next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef);
                  next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
                  # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
                  # CanItemBeReserved checks 'rules' and 'policies' which means




    my $item                = shift;
    my $patron              = shift;
    my $pickup_branchcode   = shift;
    # items_any_available is precalculated status passed from request.pl when set of items
    # looped outside of IsAvailableForItemLevelRequest to avoid nested loops:
    my $items_any_available = shift;

    my $dbh = C4::Context->dbh;
    # must check the notforloan setting of the itemtype

        return 1;
    } elsif ( $on_shelf_holds == 2 ) {

        # These calculations work at the biblio level, and can be expensive
        # we use the in-memory cache to avoid calling once per item when looping items on a biblio
        return  $items_any_available ? 0 : 1
            if defined $items_any_available;

        my $memory_cache = Koha::Cache::Memory::Lite->get_instance();
        my $cache_key = sprintf "ItemsAnyAvailableAndNotRestricted:%s:%s", $patron->id, $item->biblionumber;

        my $any_available = $memory_cache->get_from_cache( $cache_key );
        return $any_available ? 0 : 1 if defined( $any_available );

        $any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
        $memory_cache->set_in_cache( $cache_key, $any_available );
        return $any_available ? 0 : 1;

    } else { # on_shelf_holds == 0 "If any unavailable" (the description is rather cryptic and could still be improved)
        return $item->onloan || IsItemOnHoldAndFound( $item->itemnumber );
    }




use C4::Auth qw( get_template_and_user );
use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc );
use C4::Circulation qw( GetBranchItemRule );
use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch IsAvailableForItemLevelRequest GetReserveFee );
use C4::Biblio qw( GetBiblioData GetFrameworkCode );
use C4::Output qw( output_html_with_http_headers );
use C4::Context;

    # to pass this value further inside down to IsAvailableForItemLevelRequest to
    # it's complicated logic to analyse.
    # (before this loop was inside that sub loop so it was O(n^2) )
    my $items_any_available;
    $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioNum, patron => $patron }) if $patron;
    foreach my $item (@{$biblioData->{items}}) {

        my $item_info = $item->unblessed;


        my $branch = GetReservesControlBranch( $item_info, $patron_unblessed );

        # items_any_available defined outside of the current loop,
        # so we avoiding loop inside IsAvailableForItemLevelRequest:
        my $policy_holdallowed =
            CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK' &&
            IsAvailableForItemLevelRequest($item, $patron, undef);

        if ($policy_holdallowed) {
            my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } );




use Date::Calc qw( Date_to_Days );
use C4::Output qw( output_html_with_http_headers );
use C4::Auth qw( get_template_and_user );
use C4::Reserves qw( RevertWaitingStatus AlterPriority ToggleLowestPriority ToggleSuspend CanBookBeReserved GetMaxPatronHoldsForRecord CanItemBeReserved IsAvailableForItemLevelRequest );
use C4::Items qw( get_hostitemnumbers_of );
use C4::Koha qw( getitemtypeimagelocation );
use C4::Serials qw( CountSubscriptionFromBiblionumber );

            # to pass this value further inside down to IsAvailableForItemLevelRequest to
            # it's complicated logic to analyse.
            # (before this loop was inside that sub loop so it was O(n^2) )
            my $items_any_available;
            $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio->biblionumber, patron => $patron })
                if $patron;

            for my $item_object ( @items ) {
                my $do_check;

                               !$item->{cantreserve}
                            && !$exceeded_maxreserves
                            && $can_item_be_reserved eq 'OK'
                            && IsAvailableForItemLevelRequest($item_object, $patron, undef)
                            # so we avoiding loop inside IsAvailableForItemLevelRequest:
                            && IsAvailableForItemLevelRequest($item_object, $patron, undef, $items_any_available)
                        ) || C4::Context->preference('AllowHoldPolicyOverride')
                             # If AllowHoldPolicyOverride is set, it overrides EVERY restriction
                             # not just branch item rules
- 

Return to bug 34178

Lines 1336-1344 sub IsAvailableForItemLevelRequest { Link Here

(-)a/C4/Reserves.pm (-8 / +11 lines)
1336	my $item = shift;	1336	my $item = shift;
1337	my $patron = shift;	1337	my $patron = shift;
1338	my $pickup_branchcode = shift;	1338	my $pickup_branchcode = shift;
1339	# items_any_available is precalculated status passed from request.pl when set of items
1340	# looped outside of IsAvailableForItemLevelRequest to avoid nested loops:
1341	my $items_any_available = shift;
1342		1339
1343	my $dbh = C4::Context->dbh;	1340	my $dbh = C4::Context->dbh;
1344	# must check the notforloan setting of the itemtype	1341	# must check the notforloan setting of the itemtype
Lines 1376-1388 sub IsAvailableForItemLevelRequest { Link Here
1376	return 1;	1373	return 1;
1377	} elsif ( $on_shelf_holds == 2 ) {	1374	} elsif ( $on_shelf_holds == 2 ) {
1378		1375
1379	# if we have this param predefined from outer caller sub, we just need	1376	# These calculations work at the biblio level, and can be expensive
1380	# to return it, so we saving from having loop inside other loop:	1377	# we use the in-memory cache to avoid calling once per item when looping items on a biblio
1381	return $items_any_available ? 0 : 1
1382	if defined $items_any_available;
1383		1378
1384	my $any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });	1379	my $memory_cache = Koha::Cache::Memory::Lite->get_instance();
		1380	my $cache_key = sprintf "ItemsAnyAvailableAndNotRestricted:%s:%s", $patron->id, $item->biblionumber;
		1381
		1382	my $any_available = $memory_cache->get_from_cache( $cache_key );
		1383	return $any_available ? 0 : 1 if defined( $any_available );
		1384
		1385	$any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
		1386	$memory_cache->set_in_cache( $cache_key, $any_available );
1385	return $any_available ? 0 : 1;	1387	return $any_available ? 0 : 1;
		1388
1386	} else { # on_shelf_holds == 0 "If any unavailable" (the description is rather cryptic and could still be improved)	1389	} else { # on_shelf_holds == 0 "If any unavailable" (the description is rather cryptic and could still be improved)
1387	return $item->onloan \|\| IsItemOnHoldAndFound( $item->itemnumber );	1390	return $item->onloan \|\| IsItemOnHoldAndFound( $item->itemnumber );
1388	}	1391	}

Lines 33-39 use List::MoreUtils qw( uniq ); Link Here

(-)a/reserve/request.pl (-8 / +2 lines)
33	use Date::Calc qw( Date_to_Days );	33	use Date::Calc qw( Date_to_Days );
34	use C4::Output qw( output_html_with_http_headers );	34	use C4::Output qw( output_html_with_http_headers );
35	use C4::Auth qw( get_template_and_user );	35	use C4::Auth qw( get_template_and_user );
36	use C4::Reserves qw( RevertWaitingStatus AlterPriority ToggleLowestPriority ToggleSuspend CanBookBeReserved GetMaxPatronHoldsForRecord ItemsAnyAvailableAndNotRestricted CanItemBeReserved IsAvailableForItemLevelRequest );	36	use C4::Reserves qw( RevertWaitingStatus AlterPriority ToggleLowestPriority ToggleSuspend CanBookBeReserved GetMaxPatronHoldsForRecord CanItemBeReserved IsAvailableForItemLevelRequest );
37	use C4::Items qw( get_hostitemnumbers_of );	37	use C4::Items qw( get_hostitemnumbers_of );
38	use C4::Koha qw( getitemtypeimagelocation );	38	use C4::Koha qw( getitemtypeimagelocation );
39	use C4::Serials qw( CountSubscriptionFromBiblionumber );	39	use C4::Serials qw( CountSubscriptionFromBiblionumber );
Lines 393-401 if ( ( $findborrower && $borrowernumber_hold \|\| $findclub && $club_hold ) Link Here
393	# to pass this value further inside down to IsAvailableForItemLevelRequest to	393	# to pass this value further inside down to IsAvailableForItemLevelRequest to
394	# it's complicated logic to analyse.	394	# it's complicated logic to analyse.
395	# (before this loop was inside that sub loop so it was O(n^2) )	395	# (before this loop was inside that sub loop so it was O(n^2) )
396	my $items_any_available;
397	$items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio->biblionumber, patron => $patron })
398	if $patron;
399		396
400	for my $item_object ( @items ) {	397	for my $item_object ( @items ) {
401	my $do_check;	398	my $do_check;
Lines 512-520 if ( ( $findborrower && $borrowernumber_hold \|\| $findclub && $club_hold ) Link Here
512	!$item->{cantreserve}	509	!$item->{cantreserve}
513	&& !$exceeded_maxreserves	510	&& !$exceeded_maxreserves
514	&& $can_item_be_reserved eq 'OK'	511	&& $can_item_be_reserved eq 'OK'
515	# items_any_available defined outside of the current loop,	512	&& IsAvailableForItemLevelRequest($item_object, $patron, undef)
516	# so we avoiding loop inside IsAvailableForItemLevelRequest:
517	&& IsAvailableForItemLevelRequest($item_object, $patron, undef, $items_any_available)
518	) \|\| C4::Context->preference('AllowHoldPolicyOverride')	513	) \|\| C4::Context->preference('AllowHoldPolicyOverride')
519	# If AllowHoldPolicyOverride is set, it overrides EVERY restriction	514	# If AllowHoldPolicyOverride is set, it overrides EVERY restriction
520	# not just branch item rules	515	# not just branch item rules
521	-

Lines 26-32 use Encode; Link Here

(-)a/C4/Circulation.pm (-6 / +2 lines)
26		26
27	use C4::Context;	27	use C4::Context;
28	use C4::Stats qw( UpdateStats );	28	use C4::Stats qw( UpdateStats );
29	use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest ItemsAnyAvailableAndNotRestricted );	29	use C4::Reserves qw( CheckReserves CanItemBeReserved MoveReserve ModReserve ModReserveMinusPriority RevertWaitingStatus IsItemOnHoldAndFound IsAvailableForItemLevelRequest );
30	use C4::Biblio qw( UpdateTotalIssues );	30	use C4::Biblio qw( UpdateTotalIssues );
31	use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf );	31	use C4::Items qw( ModItemTransfer ModDateLastSeen CartToShelf );
32	use C4::Accounts;	32	use C4::Accounts;
Lines 3053-3069 sub CanBookBeRenewed { Link Here
3053	foreach my $possible_hold (@possible_holds) {	3053	foreach my $possible_hold (@possible_holds) {
3054	my $fillable = 0;	3054	my $fillable = 0;
3055	my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);	3055	my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
3056	my $items_any_available = ItemsAnyAvailableAndNotRestricted({
3057	biblionumber => $item->biblionumber,
3058	patron => $patron_with_reserve
3059	});
3060		3056
3061	# FIXME: We are not checking whether the item we are renewing can fill the hold	3057	# FIXME: We are not checking whether the item we are renewing can fill the hold
3062		3058
3063	foreach my $other_item (@other_items) {	3059	foreach my $other_item (@other_items) {
3064	next if defined $matched_items{$other_item->itemnumber};	3060	next if defined $matched_items{$other_item->itemnumber};
3065	next if IsItemOnHoldAndFound( $other_item->itemnumber );	3061	next if IsItemOnHoldAndFound( $other_item->itemnumber );
3066	next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available);	3062	next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef);
3067	next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';	3063	next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
3068	# NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'	3064	# NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
3069	# CanItemBeReserved checks 'rules' and 'policies' which means	3065	# CanItemBeReserved checks 'rules' and 'policies' which means

Lines 25-31 use CGI qw ( -utf8 ); Link Here

(-)a/opac/opac-reserve.pl (-6 / +2 lines)
25	use C4::Auth qw( get_template_and_user );	25	use C4::Auth qw( get_template_and_user );
26	use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc );	26	use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc );
27	use C4::Circulation qw( GetBranchItemRule );	27	use C4::Circulation qw( GetBranchItemRule );
28	use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest GetReserveFee );	28	use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch IsAvailableForItemLevelRequest GetReserveFee );
29	use C4::Biblio qw( GetBiblioData GetFrameworkCode );	29	use C4::Biblio qw( GetBiblioData GetFrameworkCode );
30	use C4::Output qw( output_html_with_http_headers );	30	use C4::Output qw( output_html_with_http_headers );
31	use C4::Context;	31	use C4::Context;
Lines 441-448 foreach my $biblioNum (@biblionumbers) { Link Here
441	# to pass this value further inside down to IsAvailableForItemLevelRequest to	441	# to pass this value further inside down to IsAvailableForItemLevelRequest to
442	# it's complicated logic to analyse.	442	# it's complicated logic to analyse.
443	# (before this loop was inside that sub loop so it was O(n^2) )	443	# (before this loop was inside that sub loop so it was O(n^2) )
444	my $items_any_available;
445	$items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioNum, patron => $patron }) if $patron;
446	foreach my $item (@{$biblioData->{items}}) {	444	foreach my $item (@{$biblioData->{items}}) {
447		445
448	my $item_info = $item->unblessed;	446	my $item_info = $item->unblessed;
Lines 481-491 foreach my $biblioNum (@biblionumbers) { Link Here
481		479
482	my $branch = GetReservesControlBranch( $item_info, $patron_unblessed );	480	my $branch = GetReservesControlBranch( $item_info, $patron_unblessed );
483		481
484	# items_any_available defined outside of the current loop,
485	# so we avoiding loop inside IsAvailableForItemLevelRequest:
486	my $policy_holdallowed =	482	my $policy_holdallowed =
487	CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK' &&	483	CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status} eq 'OK' &&
488	IsAvailableForItemLevelRequest($item, $patron, undef, $items_any_available);	484	IsAvailableForItemLevelRequest($item, $patron, undef);
489		485
490	if ($policy_holdallowed) {	486	if ($policy_holdallowed) {
491	my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } );	487	my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } );