Attachment #129501 for bug #29483

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

Lines 2803-2808 sub CanBookBeRenewed { Link Here

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




};

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 1556-1562 subtest "Bug 13841 - Do not create new 0 amount fines" => sub { Link Here

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