Attachment #102101 for bug #23172

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





        my $priority = 10000000;
        foreach my $res (@reserves) {

            #Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves
            my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber );
            next unless ($checkreserve eq 'OK');

            }
        }
    }

    # If we get this far, then no exact match was found.
    # We return the most important (i.e. next) reservation.
    if ($highest) {
        $highest->{'itemnumber'} = $item;
        return ( "Reserved", $highest, \@reserves );
    }

    return ( '' );
}


Lines 265-270 Koha::CirculationRules->set_rules( Link Here

(-)a/t/db_dependent/Circulation.t (+20 lines)
265	auto_renew => 0,	265	auto_renew => 0,
266	fine => .10,	266	fine => .10,
267	chargeperiod => 1,	267	chargeperiod => 1,
		268	holds_per_day => 12,
		269	holds_per_record => 1,
268	}	270	}
269	}	271	}
270	);	272	);
Lines 1458-1463 subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Link Here
1458	branchcode => undef,	1460	branchcode => undef,
1459	rules => {	1461	rules => {
1460	onshelfholds => 0,	1462	onshelfholds => 0,
		1463	holds_per_day => 1
1461	}	1464	}
1462	}	1465	}
1463	);	1466	);
Lines 2940-2945 subtest 'Set waiting flag' => sub { Link Here
2940	my $library_2 = $builder->build( { source => 'Branch' } );	2943	my $library_2 = $builder->build( { source => 'Branch' } );
2941	my $patron_2 = $builder->build( { source => 'Borrower', value => { branchcode => $library_2->{branchcode}, categorycode => $patron_category->{categorycode} } } );	2944	my $patron_2 = $builder->build( { source => 'Borrower', value => { branchcode => $library_2->{branchcode}, categorycode => $patron_category->{categorycode} } } );
2942		2945
		2946	my $biblio = $builder->build( { source => 'Biblio' } );
		2947	my $biblioitem = $builder->build( { source => 'Biblioitem', value => { biblionumber => $biblio->{biblionumber} } } );
		2948
		2949	Koha::CirculationRules->set_rules(
		2950	{
		2951	categorycode => undef,
		2952	itemtype => undef,
		2953	branchcode => undef,
		2954	rules => {
		2955	reservesallowed => 1,
		2956	holds_per_day => 1,
		2957	holds_per_record => 1,
		2958	}
		2959	}
		2960	);
		2961
2943	my $item = $builder->build_sample_item(	2962	my $item = $builder->build_sample_item(
2944	{	2963	{
2945	library => $library_1->{branchcode},	2964	library => $library_1->{branchcode},
Lines 2964-2969 subtest 'Set waiting flag' => sub { Link Here
2964	my $hold = Koha::Holds->find( $reserve_id );	2983	my $hold = Koha::Holds->find( $reserve_id );
2965	is( $hold->found, 'T', 'Hold is in transit' );	2984	is( $hold->found, 'T', 'Hold is in transit' );
2966		2985
		2986
2967	my ( $status ) = CheckReserves($item->{itemnumber});	2987	my ( $status ) = CheckReserves($item->{itemnumber});
2968	is( $status, 'Reserved', 'Hold is not waiting yet');	2988	is( $status, 'Reserved', 'Hold is not waiting yet');
2969		2989




        rules        => {
            reservesallowed  => 25,
            holds_per_record => 99,
            holds_per_day => 99,
        }
    }
);

        branchcode   => undef,
        itemtype     => 'CANNOT',
        rules        => {
            reservesallowed  => 1,
            holds_per_record => 99,
            holds_per_day => 99,
        }
    }
);

        priority       => 1,
    }
);

is(
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
    "cannot request item if policy that matches on item-level item type forbids it"





use Modern::Perl;

use Test::More tests => 65;
use Test::MockModule;
use Test::Warn;


        rules        => {
            reservesallowed => 25,
            holds_per_record => 1,
            holds_per_day => 1,
        }
    }
);

# the one placed by the CPL patron, as the other two patron's hold
# requests cannot be filled by that item per policy.
(undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2);

is( $messages->{ResFound}->{borrowernumber},
    $requesters{$branch_1},
    'restrictive library\'s items only fill requests by own patrons (bug 10272)');

is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');
$dbh->do('DELETE FROM reserves', undef, ($bibnum));






#Test bug 23172. Check when returning an item that any patron category/item type combinations that forbid reservation are skipped over
#when allocating the item to a bib-level hold
$dbh->do('DELETE FROM issuingrules');

#Create two new patrons
my $patronnumcat1 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_1})->store->borrowernumber;
my $patronnumcat2 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_2})->store->borrowernumber;

#Create a new biblio record
my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber;

#Create a second item type
my $itemtype2 = $builder->build({ source => 'Itemtype', value => { notforloan => undef } } )->{itemtype};

# Create an item from new itemtype
my ( $item_bibnum2, $item_bibitemnum2, $itemnumber2 ) = AddItem(
    {   homebranch    => $branch_1,
        holdingbranch => $branch_1,
        itype         => $itemtype2,
        barcode       => 'bug23172_CPL'
    },
    $bibnum3
);

#Make a general All patron category/all item type rule
$dbh->do(
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
      VALUES (?, ?, ?, ?, ?, ?)},
    {},
    '*', '*', '*', 25, 1, 1
);

#When patron category = $category_1 and itemtype = $itemtype1 then reservesallowed, holds_per_day and holds_per_record are 0
$dbh->do(
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
     VALUES (?, ?, ?, ?, ?, ?)},
    {},
    $category_1, $branch_1, $itemtype2, 0, 1, 1
);

#When patron category = $category_2 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 1
$dbh->do(
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
     VALUES (?, ?, ?, ?, ?, ?)},
    {},
    $category_2, $branch_1, $itemtype2, 1, 1, 1
);

#Remove existing reserves
$dbh->do("DELETE FROM reserves");

#Create Bib-level hold for borrower who has a patron category of $category_2
my $reserveid2 = AddReserve($branch_1,  $patronnumcat2, $bibnum3);

#Bib-level hold for borrower who has a patron category of $category_1
my $reserveid1 = AddReserve($branch_1,  $patronnumcat1, $bibnum3 );

#Check bib-level hold priority and confirm hold made by patronnumcat1 (who fits into issuingrule with reserveallowed = 0) has first priority
my $reserve1 = Koha::Holds->find($reserveid1);
is( $reserve1->priority, 1, 'patronnumcat1 reserve is first priority');

my $reserve2 = Koha::Holds->find($reserveid2);
is( $reserve2->priority, 2, 'patronnumcat2 reserve is second priority');

#Return the item
(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);

#First priority hold is skipped as issuingrule is checked and the first hold does not respect issuingrules (as one of the three fields: reservesallowed, holds_per_day is 0) therefore not allocating item to this bib-level hold
is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');

#Remove and re-add the issuing rule that prevented patronnumcat1 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0
$dbh->do(q{DELETE FROM issuingrules WHERE categorycode = ? AND branchcode = ? AND itemtype = ?},
        {},
        $category_1, $branch_1, $itemtype2
);
$dbh->do(
   q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
    VALUES (?, ?, ?, ?, ?, ?)},
   {},
   $category_1, $branch_1, $itemtype2, 1, 1, 1
);

#Return the item again and notice this time it is allocated to the bib-level hold made by patronnumcat1
(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);
is( $messages->{ResFound}->{borrowernumber}, $patronnumcat1,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');

$cache->clear_from_cache("MarcStructure-0-$frameworkcode");
$cache->clear_from_cache("MarcStructure-1-$frameworkcode");
$cache->clear_from_cache("default_value_for_mod_marc-");
- 

Return to bug 23172

Lines 810-816 sub CheckReserves { Link Here

(-)a/C4/Reserves.pm (-3 lines)
810		810
811	my $priority = 10000000;	811	my $priority = 10000000;
812	foreach my $res (@reserves) {	812	foreach my $res (@reserves) {
813
814	#Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves	813	#Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves
815	my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber );	814	my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber );
816	next unless ($checkreserve eq 'OK');	815	next unless ($checkreserve eq 'OK');
Lines 866-879 sub CheckReserves { Link Here
866	}	865	}
867	}	866	}
868	}	867	}
869
870	# If we get this far, then no exact match was found.	868	# If we get this far, then no exact match was found.
871	# We return the most important (i.e. next) reservation.	869	# We return the most important (i.e. next) reservation.
872	if ($highest) {	870	if ($highest) {
873	$highest->{'itemnumber'} = $item;	871	$highest->{'itemnumber'} = $item;
874	return ( "Reserved", $highest, \@reserves );	872	return ( "Reserved", $highest, \@reserves );
875	}	873	}
876
877	return ( '' );	874	return ( '' );
878	}	875	}
879		876

Lines 240-245 Koha::CirculationRules->set_rules( Link Here

(-)a/t/db_dependent/Holds.t (-1 / +4 lines)
240	rules => {	240	rules => {
241	reservesallowed => 25,	241	reservesallowed => 25,
242	holds_per_record => 99,	242	holds_per_record => 99,
		243	holds_per_day => 99,
243	}	244	}
244	}	245	}
245	);	246	);
Lines 249-256 Koha::CirculationRules->set_rules( Link Here
249	branchcode => undef,	250	branchcode => undef,
250	itemtype => 'CANNOT',	251	itemtype => 'CANNOT',
251	rules => {	252	rules => {
252	reservesallowed => 0,	253	reservesallowed => 1,
253	holds_per_record => 99,	254	holds_per_record => 99,
		255	holds_per_day => 99,
254	}	256	}
255	}	257	}
256	);	258	);
Lines 355-360 AddReserve( Link Here
355	priority => 1,	357	priority => 1,
356	}	358	}
357	);	359	);
		360
358	is(	361	is(
359	CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',	362	CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
360	"cannot request item if policy that matches on item-level item type forbids it"	363	"cannot request item if policy that matches on item-level item type forbids it"

Lines 17-23 Link Here

(-)a/t/db_dependent/Reserves.t (-2 / +94 lines)
17		17
18	use Modern::Perl;	18	use Modern::Perl;
19		19
20	use Test::More tests => 62;	20	use Test::More tests => 65;
21	use Test::MockModule;	21	use Test::MockModule;
22	use Test::Warn;	22	use Test::Warn;
23		23
Lines 190-195 Koha::CirculationRules->set_rules( Link Here
190	rules => {	190	rules => {
191	reservesallowed => 25,	191	reservesallowed => 25,
192	holds_per_record => 1,	192	holds_per_record => 1,
		193	holds_per_day => 1,
193	}	194	}
194	}	195	}
195	);	196	);
Lines 314-319 my $messages; Link Here
314	# the one placed by the CPL patron, as the other two patron's hold	315	# the one placed by the CPL patron, as the other two patron's hold
315	# requests cannot be filled by that item per policy.	316	# requests cannot be filled by that item per policy.
316	(undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2);	317	(undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2);
		318
317	is( $messages->{ResFound}->{borrowernumber},	319	is( $messages->{ResFound}->{borrowernumber},
318	$requesters{$branch_1},	320	$requesters{$branch_1},
319	'restrictive library\'s items only fill requests by own patrons (bug 10272)');	321	'restrictive library\'s items only fill requests by own patrons (bug 10272)');
Lines 725-730 MoveReserve( $item->itemnumber, $borrowernumber ); Link Here
725	is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');	727	is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');
726	$dbh->do('DELETE FROM reserves', undef, ($bibnum));	728	$dbh->do('DELETE FROM reserves', undef, ($bibnum));
727		729
		730
		731
		732
		733
		734
		735	#Test bug 23172. Check when returning an item that any patron category/item type combinations that forbid reservation are skipped over
		736	#when allocating the item to a bib-level hold
		737	$dbh->do('DELETE FROM issuingrules');
		738
		739	#Create two new patrons
		740	my $patronnumcat1 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_1})->store->borrowernumber;
		741	my $patronnumcat2 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_2})->store->borrowernumber;
		742
		743	#Create a new biblio record
		744	my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber;
		745
		746	#Create a second item type
		747	my $itemtype2 = $builder->build({ source => 'Itemtype', value => { notforloan => undef } } )->{itemtype};
		748
		749	# Create an item from new itemtype
		750	my ( $item_bibnum2, $item_bibitemnum2, $itemnumber2 ) = AddItem(
		751	{ homebranch => $branch_1,
		752	holdingbranch => $branch_1,
		753	itype => $itemtype2,
		754	barcode => 'bug23172_CPL'
		755	},
		756	$bibnum3
		757	);
		758
		759	#Make a general All patron category/all item type rule
		760	$dbh->do(
		761	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
		762	VALUES (?, ?, ?, ?, ?, ?)},
		763	{},
		764	'', '', '*', 25, 1, 1
		765	);
		766
		767	#When patron category = $category_1 and itemtype = $itemtype1 then reservesallowed, holds_per_day and holds_per_record are 0
		768	$dbh->do(
		769	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
		770	VALUES (?, ?, ?, ?, ?, ?)},
		771	{},
		772	$category_1, $branch_1, $itemtype2, 0, 1, 1
		773	);
		774
		775	#When patron category = $category_2 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 1
		776	$dbh->do(
		777	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
		778	VALUES (?, ?, ?, ?, ?, ?)},
		779	{},
		780	$category_2, $branch_1, $itemtype2, 1, 1, 1
		781	);
		782
		783	#Remove existing reserves
		784	$dbh->do("DELETE FROM reserves");
		785
		786	#Create Bib-level hold for borrower who has a patron category of $category_2
		787	my $reserveid2 = AddReserve($branch_1, $patronnumcat2, $bibnum3);
		788
		789	#Bib-level hold for borrower who has a patron category of $category_1
		790	my $reserveid1 = AddReserve($branch_1, $patronnumcat1, $bibnum3 );
		791
		792	#Check bib-level hold priority and confirm hold made by patronnumcat1 (who fits into issuingrule with reserveallowed = 0) has first priority
		793	my $reserve1 = Koha::Holds->find($reserveid1);
794	is( $reserve1->priority, 1, 'patronnumcat1 reserve is first priority');
795
796	my $reserve2 = Koha::Holds->find($reserveid2);
797	is( $reserve2->priority, 2, 'patronnumcat2 reserve is second priority');
798
799	#Return the item
800	(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);
801
802	#First priority hold is skipped as issuingrule is checked and the first hold does not respect issuingrules (as one of the three fields: reservesallowed, holds_per_day is 0) therefore not allocating item to this bib-level hold
803	is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');
804
805	#Remove and re-add the issuing rule that prevented patronnumcat1 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0
806	$dbh->do(q{DELETE FROM issuingrules WHERE categorycode = ? AND branchcode = ? AND itemtype = ?},
807	{},
808	$category_1, $branch_1, $itemtype2
809	);
810	$dbh->do(
811	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
812	VALUES (?, ?, ?, ?, ?, ?)},
813	{},
814	$category_1, $branch_1, $itemtype2, 1, 1, 1
815	);
816
817	#Return the item again and notice this time it is allocated to the bib-level hold made by patronnumcat1
818	(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);
819	is( $messages->{ResFound}->{borrowernumber}, $patronnumcat1,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');
820
728	$cache->clear_from_cache("MarcStructure-0-$frameworkcode");	821	$cache->clear_from_cache("MarcStructure-0-$frameworkcode");
729	$cache->clear_from_cache("MarcStructure-1-$frameworkcode");	822	$cache->clear_from_cache("MarcStructure-1-$frameworkcode");
730	$cache->clear_from_cache("default_value_for_mod_marc-");	823	$cache->clear_from_cache("default_value_for_mod_marc-");
731	-