Attachment #133391 for bug #29483

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

Lines 2916-2921 sub CanBookBeRenewed { Link Here

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




};

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

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