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

(-)a/C4/Circulation.pm (-8 / +25 lines)
Lines 2916-2921 sub CanBookBeRenewed { Link Here
2916
        }
2916
        }
2917
    }
2917
    }
2918
2918
2919
    # Note: possible_reserves will contain all title level holds on this bib and item level
2920
    # holds on the checked out item
2919
    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
2921
    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
2920
2922
2921
    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
2923
    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
Lines 2940-2973 sub CanBookBeRenewed { Link Here
2940
        else {
2942
        else {
2941
2943
2942
            # Get all other items that could possibly fill reserves
2944
            # Get all other items that could possibly fill reserves
2945
            # FIXME We could join reserves (or more tables) here to eliminate some checks later
2943
            my $items = Koha::Items->search({
2946
            my $items = Koha::Items->search({
2944
                biblionumber => $resrec->{biblionumber},
2947
                biblionumber => $resrec->{biblionumber},
2945
                onloan       => undef,
2948
                onloan       => undef,
2946
                notforloan   => 0,
2949
                notforloan   => 0,
2947
                -not         => { itemnumber => $itemnumber }
2950
                -not         => { itemnumber => $itemnumber }
2948
            });
2951
            });
2952
            my $item_count = $items->count();
2949
2953
2950
            # Get all other reserves that could have been filled by this item
2954
            # Get all other reserves that could have been filled by this item
2951
            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
2955
            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
2956
            # Note: fetching the patrons in this manner means that a patron with 2 holds will
2957
            # not block renewal if one reserve can be satisfied i.e. each patron is checked once
2952
            my $patrons = Koha::Patrons->search({
2958
            my $patrons = Koha::Patrons->search({
2953
                borrowernumber => { -in => \@borrowernumbers }
2959
                borrowernumber => { -in => \@borrowernumbers }
2954
            });
2960
            });
2961
            my $patron_count = $patrons->count();
2955
2962
2956
            # If the count of the union of the lists of reservable items for each borrower
2963
            return ( 0, "on_reserve" ) if ($patron_count > $item_count);
2957
            # is equal or greater than the number of borrowers, we know that all reserves
2964
            # We cannot possibly fill all reserves if we don't have enough items
2958
            # can be filled with available items. We can get the union of the sets simply
2965
2959
            # by pushing all the elements onto an array and removing the duplicates.
2966
            # If we can fill each hold that has been found with the available items on the record
2960
            my @reservable;
2967
            # then the patron can renew. If we cannot, they cannot renew.
2968
            # FIXME This code does not check whether the item we are renewing can fill
2969
            # any of the existing reserves.
2970
            my $reservable = 0;
2961
            my %matched_items;
2971
            my %matched_items;
2972
            my $seen = 0;
2962
            PATRON: while ( my $patron = $patrons->next ) {
2973
            PATRON: while ( my $patron = $patrons->next ) {
2974
                # If there is a reserve that cannot be filled we are done
2975
                return ( 0, "on_reserve" ) if ( $seen > $reservable );
2963
                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
2976
                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
2964
                while ( my $other_item = $items->next ) {
2977
                while ( my $other_item = $items->next ) {
2965
                    next if $matched_items{$other_item->itemnumber} == 1;
2978
                    next if defined $matched_items{$other_item->itemnumber};
2966
                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
2979
                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
2967
                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
2980
                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
2968
                    next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
2981
                    next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
2969
                    push @reservable, $other_item->itemnumber;
2982
                    # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
2970
                    if (@reservable >= @borrowernumbers) {
2983
                    # CanItemBeReserved checks 'rules' and 'policies' which means
2984
                    # items will fill holds at checkin that are rejected here
2985
                    $reservable++;
2986
                    if ($reservable >= $patron_count) {
2971
                        $resfound = 0;
2987
                        $resfound = 0;
2972
                        last PATRON;
2988
                        last PATRON;
2973
                    }
2989
                    }
Lines 2975-2980 sub CanBookBeRenewed { Link Here
2975
                    last;
2991
                    last;
2976
                }
2992
                }
2977
                $items->reset;
2993
                $items->reset;
2994
                $seen++;
2978
            }
2995
            }
2979
        }
2996
        }
2980
    }
2997
    }
(-)a/t/db_dependent/Circulation.t (-2 / +51 lines)
Lines 1623-1629 subtest "Bug 13841 - Do not create new 0 amount fines" => sub { Link Here
1623
};
1623
};
1624
1624
1625
subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
1625
subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
1626
    plan tests => 9;
1626
    plan tests => 13;
1627
    my $biblio = $builder->build_sample_biblio();
1627
    my $biblio = $builder->build_sample_biblio();
1628
    my $item_1 = $builder->build_sample_item(
1628
    my $item_1 = $builder->build_sample_item(
1629
        {
1629
        {
Lines 1780-1791 subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Link Here
1780
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1780
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1781
    is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record, but only one of those holds can be filled when AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' );
1781
    is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record, but only one of those holds can be filled when AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' );
1782
1782
1783
    Koha::CirculationRules->set_rules(
1784
        {
1785
            categorycode => $patron_category_2->{categorycode},
1786
            itemtype     => $item_1->effective_itemtype,
1787
            branchcode   => undef,
1788
            rules        => {
1789
                reservesallowed => 25,
1790
            }
1791
        }
1792
    );
1793
1783
    # Setting item not checked out to be not for loan but holdable
1794
    # Setting item not checked out to be not for loan but holdable
1784
    $item_2->notforloan(-1)->store;
1795
    $item_2->notforloan(-1)->store;
1785
1796
1786
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1797
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1787
    is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' );
1798
    is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' );
1788
1799
1800
    my $mock_circ = Test::MockModule->new("C4::Circulation");
1801
    $mock_circ->mock( CanItemBeReserved => sub {
1802
        warn "Checked";
1803
        return { status => 'no' }
1804
    } );
1805
1806
    $item_2->notforloan(0)->store;
1807
    $item_3->delete();
1808
    # Two items total, one item available, one issued, two holds on record
1809
1810
    warnings_are{
1811
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1812
    } [], "CanItemBeReserved not called when there are more possible holds than available items";
1813
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );
1814
1815
    $item_3 = $builder->build_sample_item(
1816
        {
1817
            biblionumber     => $biblio->biblionumber,
1818
            library          => $library2->{branchcode},
1819
            itype            => $item_1->effective_itemtype,
1820
        }
1821
    );
1822
1823
    Koha::CirculationRules->set_rules(
1824
        {
1825
            categorycode => undef,
1826
            itemtype     => $item_1->effective_itemtype,
1827
            branchcode   => undef,
1828
            rules        => {
1829
                reservesallowed => 0,
1830
            }
1831
        }
1832
    );
1833
1834
    warnings_are{
1835
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1836
    } ["Checked","Checked"], "CanItemBeReserved only called once per available item if it returns a negative result for all items for a borrower";
1837
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );
1838
1789
};
1839
};
1790
1840
1791
{
1841
{
1792
- 

Return to bug 29483