Attachment #127885 for bug #29483

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

Lines 2811-2816 sub CanBookBeRenewed { Link Here

(-)a/C4/Circulation.pm (-8 / +25 lines)
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	}




};

subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
    plan tests => 13;
    my $biblio = $builder->build_sample_biblio();
    my $item_1 = $builder->build_sample_item(
        {

    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
    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' );

    Koha::CirculationRules->set_rules(
        {
            categorycode => $patron_category_2->{categorycode},
            itemtype     => $item_1->effective_itemtype,
            branchcode   => undef,
            rules        => {
                reservesallowed => 25,
            }
        }
    );

    # Setting item not checked out to be not for loan but holdable
    $item_2->notforloan(-1)->store;

    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
    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' );

    my $mock_circ = Test::MockModule->new("C4::Circulation");
    $mock_circ->mock( CanItemBeReserved => sub {
        warn "Checked";
        return { status => 'no' }
    } );

    $item_2->notforloan(0)->store;
    $item_3->delete(); 
    # Two items total, one item available, one issued, two holds on record

    warnings_are{
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
    } [], "CanItemBeReserved not called when there are more possible holds than available items";
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );

    $item_3 = $builder->build_sample_item(
        {
            biblionumber     => $biblio->biblionumber,
            library          => $library2->{branchcode},
            itype            => $item_1->effective_itemtype,
        }
    );

    Koha::CirculationRules->set_rules(
        {
            categorycode => undef,
            itemtype     => $item_1->effective_itemtype,
            branchcode   => undef,
            rules        => {
                reservesallowed => 0,
            }
        }
    );

    warnings_are{
       ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
    } ["Checked","Checked"], "CanItemBeReserved only called once per available item if it returns a negative result for all items for a borrower";
    is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' );

};

{
- 

Return to bug 29483

Lines 1574-1580 subtest "Bug 13841 - Do not create new 0 amount fines" => sub { Link Here

(-)a/t/db_dependent/Circulation.t (-2 / +51 lines)
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	-