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

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

Return to bug 29483