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

(-)a/C4/Circulation.pm (-6 / +2 lines)
Lines 26-32 use Encode; Link Here
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 3050-3066 sub CanBookBeRenewed { Link Here
3050
            foreach my $possible_hold (@possible_holds) {
3050
            foreach my $possible_hold (@possible_holds) {
3051
                my $fillable = 0;
3051
                my $fillable = 0;
3052
                my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
3052
                my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber);
3053
                my $items_any_available = ItemsAnyAvailableAndNotRestricted({
3054
                    biblionumber => $item->biblionumber,
3055
                    patron => $patron_with_reserve
3056
                });
3057
3053
3058
                # FIXME: We are not checking whether the item we are renewing can fill the hold
3054
                # FIXME: We are not checking whether the item we are renewing can fill the hold
3059
3055
3060
                foreach my $other_item (@other_items) {
3056
                foreach my $other_item (@other_items) {
3061
                  next if defined $matched_items{$other_item->itemnumber};
3057
                  next if defined $matched_items{$other_item->itemnumber};
3062
                  next if IsItemOnHoldAndFound( $other_item->itemnumber );
3058
                  next if IsItemOnHoldAndFound( $other_item->itemnumber );
3063
                  next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available);
3059
                  next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef);
3064
                  next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
3060
                  next unless CanItemBeReserved($patron_with_reserve,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
3065
                  # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
3061
                  # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
3066
                  # CanItemBeReserved checks 'rules' and 'policies' which means
3062
                  # CanItemBeReserved checks 'rules' and 'policies' which means
(-)a/C4/Reserves.pm (-8 / +11 lines)
Lines 1336-1344 sub IsAvailableForItemLevelRequest { Link Here
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
    }
(-)a/opac/opac-reserve.pl (-6 / +2 lines)
Lines 25-31 use CGI qw ( -utf8 ); Link Here
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 } );
(-)a/reserve/request.pl (-8 / +2 lines)
Lines 33-39 use List::MoreUtils qw( uniq ); Link Here
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
- 

Return to bug 34178