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

(-)a/C4/Circulation.pm (-8 / +25 lines)
Lines 2811-2816 sub CanBookBeRenewed { Link Here
2811
        return ( 0, $auto_renew  ) if $auto_renew =~ 'auto_too_much_oweing';
2811
        return ( 0, $auto_renew  ) if $auto_renew =~ 'auto_too_much_oweing';
2812
    }
2812
    }
2813
2813
2814
    # Note: possible_reserves will contain all title level holds on this bib and item level
2815
    # holds on the checked out item
2814
    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
2816
    my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber);
2815
2817
2816
    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
2818
    # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber.
Lines 2835-2868 sub CanBookBeRenewed { Link Here
2835
        else {
2837
        else {
2836
2838
2837
            # Get all other items that could possibly fill reserves
2839
            # Get all other items that could possibly fill reserves
2840
            # FIXME We could join reserves (or more tables) here to eliminate some checks later
2838
            my $items = Koha::Items->search({
2841
            my $items = Koha::Items->search({
2839
                biblionumber => $resrec->{biblionumber},
2842
                biblionumber => $resrec->{biblionumber},
2840
                onloan       => undef,
2843
                onloan       => undef,
2841
                notforloan   => 0,
2844
                notforloan   => 0,
2842
                -not         => { itemnumber => $itemnumber }
2845
                -not         => { itemnumber => $itemnumber }
2843
            });
2846
            });
2847
            my $item_count = $items->count();
2844
2848
2845
            # Get all other reserves that could have been filled by this item
2849
            # Get all other reserves that could have been filled by this item
2846
            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
2850
            my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves;
2851
            # Note: fetching the patrons in this manner means that a patron with 2 holds will
2852
            # not block renewal if one reserve can be satisfied i.e. each patron is checked once
2847
            my $patrons = Koha::Patrons->search({
2853
            my $patrons = Koha::Patrons->search({
2848
                borrowernumber => { -in => \@borrowernumbers }
2854
                borrowernumber => { -in => \@borrowernumbers }
2849
            });
2855
            });
2856
            my $patron_count = $patrons->count();
2850
2857
2851
            # If the count of the union of the lists of reservable items for each borrower
2858
            return ( 0, "on_reserve" ) if ($patron_count > $item_count);
2852
            # is equal or greater than the number of borrowers, we know that all reserves
2859
            # We cannot possibly fill all reserves if we don't have enough items
2853
            # can be filled with available items. We can get the union of the sets simply
2860
2854
            # by pushing all the elements onto an array and removing the duplicates.
2861
            # If we can fill each hold that has been found with the available items on the record
2855
            my @reservable;
2862
            # then the patron can renew. If we cannot, they cannot renew.
2863
            # FIXME This code does not check whether the item we are renewing can fill
2864
            # any of the existing reserves.
2865
            my $reservable = 0;
2856
            my %matched_items;
2866
            my %matched_items;
2867
            my $seen = 0;
2857
            PATRON: while ( my $patron = $patrons->next ) {
2868
            PATRON: while ( my $patron = $patrons->next ) {
2869
                # If there is a reserve that cannot be filled we are done
2870
                return ( 0, "on_reserve" ) if ( $seen > $reservable );
2858
                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
2871
                my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
2859
                while ( my $other_item = $items->next ) {
2872
                while ( my $other_item = $items->next ) {
2860
                    next if $matched_items{$other_item->itemnumber} == 1;
2873
                    next if defined $matched_items{$other_item->itemnumber};
2861
                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
2874
                    next if IsItemOnHoldAndFound( $other_item->itemnumber );
2862
                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
2875
                    next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available);
2863
                    next unless CanItemBeReserved($patron->borrowernumber,$other_item->itemnumber,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
2876
                    next unless CanItemBeReserved($patron->borrowernumber,$other_item->itemnumber,undef,{ignore_hold_counts=>1})->{status} eq 'OK';
2864
                    push @reservable, $other_item->itemnumber;
2877
                    # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy'
2865
                    if (@reservable >= @borrowernumbers) {
2878
                    # CanItemBeReserved checks 'rules' and 'policies' which means
2879
                    # items will fill holds at checkin that are rejected here
2880
                    $reservable++;
2881
                    if ($reservable >= $patron_count) {
2866
                        $resfound = 0;
2882
                        $resfound = 0;
2867
                        last PATRON;
2883
                        last PATRON;
2868
                    }
2884
                    }
Lines 2870-2875 sub CanBookBeRenewed { Link Here
2870
                    last;
2886
                    last;
2871
                }
2887
                }
2872
                $items->reset;
2888
                $items->reset;
2889
                $seen++;
2873
            }
2890
            }
2874
        }
2891
        }
2875
    }
2892
    }
(-)a/t/db_dependent/Circulation.t (-2 / +51 lines)
Lines 1574-1580 subtest "Bug 13841 - Do not create new 0 amount fines" => sub { Link Here
1574
};
1574
};
1575
1575
1576
subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
1576
subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
1577
    plan tests => 9;
1577
    plan tests => 13;
1578
    my $biblio = $builder->build_sample_biblio();
1578
    my $biblio = $builder->build_sample_biblio();
1579
    my $item_1 = $builder->build_sample_item(
1579
    my $item_1 = $builder->build_sample_item(
1580
        {
1580
        {
Lines 1731-1742 subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Link Here
1731
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1731
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1732
    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' );
1732
    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' );
1733
1733
1734
    Koha::CirculationRules->set_rules(
1735
        {
1736
            categorycode => $patron_category_2->{categorycode},
1737
            itemtype     => $item_1->effective_itemtype,
1738
            branchcode   => undef,
1739
            rules        => {
1740
                reservesallowed => 25,
1741
            }
1742
        }
1743
    );
1744
1734
    # Setting item not checked out to be not for loan but holdable
1745
    # Setting item not checked out to be not for loan but holdable
1735
    $item_2->notforloan(-1)->store;
1746
    $item_2->notforloan(-1)->store;
1736
1747
1737
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1748
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1738
    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' );
1749
    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' );
1739
1750
1751
    my $mock_circ = Test::MockModule->new("C4::Circulation");
1752
    $mock_circ->mock( CanItemBeReserved => sub {
1753
        warn "Checked";
1754
        return { status => 'no' }
1755
    } );
1756
1757
    $item_2->notforloan(0)->store;
1758
    $item_3->delete(); 
1759
    # Two items total, one item available, one issued, two holds on record
1760
1761
    warnings_are{
1762
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1763
    } [], "CanItemBeReserved not called when there are more possible holds than available items";
1764
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );
1765
1766
    $item_3 = $builder->build_sample_item(
1767
        {
1768
            biblionumber     => $biblio->biblionumber,
1769
            library          => $library2->{branchcode},
1770
            itype            => $item_1->effective_itemtype,
1771
        }
1772
    );
1773
1774
    Koha::CirculationRules->set_rules(
1775
        {
1776
            categorycode => undef,
1777
            itemtype     => $item_1->effective_itemtype,
1778
            branchcode   => undef,
1779
            rules        => {
1780
                reservesallowed => 0,
1781
            }
1782
        }
1783
    );
1784
1785
    warnings_are{
1786
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1787
    } ["Checked","Checked"], "CanItemBeReserved only called once per available item if it returns a negative result for all items for a borrower";
1788
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );
1789
1740
};
1790
};
1741
1791
1742
{
1792
{
1743
- 

Return to bug 29483