Bugzilla – Attachment 118933 Details for
Bug 23172
Holds queue should check patron category hold policies when mapping items to pending reserves
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23172: (follow-up) Fixing test
Bug-23172-follow-up-Fixing-test.patch (text/plain), 29.25 KB, created by
Alex Buckley
on 2021-03-29 05:25:23 UTC
(
hide
)
Description:
Bug 23172: (follow-up) Fixing test
Filename:
MIME Type:
Creator:
Alex Buckley
Created:
2021-03-29 05:25:23 UTC
Size:
29.25 KB
patch
obsolete
>From 667cbde0317545e1c92c95779fa013b3a3513c2c Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Wed, 25 Mar 2020 23:17:42 +0000 >Subject: [PATCH] Bug 23172: (follow-up) Fixing test > >Ensure that the following tests pass: >t/db_dependent/Hold.t >t/db_dependent/Reserves.t > >And that the original test plan works as expected >--- > C4/Reserves.pm | 11 +- > Koha/Hold.pm | 43 +++++--- > t/db_dependent/Circulation.t | 3 - > t/db_dependent/Hold.t | 235 +++++++++++++++++++++++++++---------------- > t/db_dependent/Reserves.t | 186 +++++++++++++++++++++++----------- > 5 files changed, 309 insertions(+), 169 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 5af75d967c0..b73e84b3aa0 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -595,6 +595,10 @@ sub CanReserveBeCanceledFromOpac { > > return 0 unless $reserve->borrowernumber == $borrowernumber; > return $reserve->is_cancelable_from_opac; >+ return 0 if ( $reserve->found && $reserve->found eq 'W' ) or ( $reserve->found && $reserve->found eq 'T' ); >+ >+ return 1; >+ > } > > =head2 GetOtherReserves >@@ -854,7 +858,12 @@ sub CheckReserves { > 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 ); >+ my $checkreserve; >+ if ( $item ){ >+ $checkreserve = Koha::Holds->find( $res->{reserve_id} )->matches_circulation_rules({ itemnumber => $item }); >+ } else { >+ $checkreserve = Koha::Holds->find( $res->{reserve_id} )->matches_circulation_rules({ barcode => $barcode }); >+ } > next unless ($checkreserve eq 'OK'); > if ($res->{'found'} && $res->{'found'} eq 'W') { > return ( "Waiting", $res, \@reserves ); # Found it, it is waiting >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index b23b0445333..37ccd9227cb 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -36,7 +36,6 @@ use Koha::Items; > use Koha::Libraries; > use Koha::Old::Holds; > use Koha::Calendar; >- > use Koha::Exceptions::Hold; > > use base qw(Koha::Object); >@@ -476,41 +475,53 @@ sub is_suspended { > return $self->suspend(); > } > >-=head3 check_if_existing_hold_matches_issuingrules >+=head3 matches_circulation_rules > >-my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules($res->{'borrowernumber'}, $itemnumber ); >+ my $checkreserve = Koha::Hold->matches_circulation_rules({ itemnumber => $itemnumber }); >+ my $checkreserve = Koha::Hold->matches_circulation_rules({ barcode => $barcode }); > > Upon checking in an item of a bibliographic record with a biblio-level hold on it, check if a bib-level hold can be allocated to a patron based on the current values in the issuingrules table > >+Parameter is an attribute of the item that is being checked in >+ > =cut > >-sub check_if_existing_hold_matches_issuingrules { >- my ( $self, $borrowernumber, $itemnumber ) = @_; >+sub matches_circulation_rules { >+ my ( $self, $params ) = @_; > > #Get patron and item objects >- my $patron = Koha::Patrons->find( $borrowernumber ); >- my $item = Koha::Items->find( $itemnumber ); >- $item = $item->unblessed(); >+ my $patron = $self->borrower; >+ my $item = Koha::Items->find( $params->{itemnumber} ); >+ if ( !defined $item ){ >+ $item = Koha::Items->find({ barcode => $params->{barcode} }); >+ } > > #Get branchcode > my $branchcode; > my $controlbranch = C4::Context->preference('ReservesControlBranch'); >- >- if ( $controlbranch eq "ItemHomeLibrary" ) { >- $branchcode = $item->{homebranch}; >- } >- elsif ( $controlbranch eq "PatronLibrary" ) { >+ if ( $controlbranch eq "ItemHomeLibrary" && defined $item ) { >+ $branchcode = $item->homebranch; >+ } else { > $branchcode = $patron->branchcode; > } > >- my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule({ >+ my $issuingrule = Koha::CirculationRules->get_effective_rules({ > branchcode => $branchcode, > categorycode => $patron->categorycode, >- itemtype => $item->{itype}, >+ itemtype => $item->effective_itemtype, >+ rules => [ >+ 'reservesallowed', >+ 'holds_per_record', >+ 'holds_per_day', >+ ], > }); > > #Check if the patron category/item type combination is valid >- if ( ($issuingrule->reservesallowed == 0 || $issuingrule->holds_per_record == 0 || $issuingrule->holds_per_day == 0 )) { >+ if ( >+ ( defined $issuingrule->{reservesallowed} && $issuingrule->{reservesallowed} == 0 ) || >+ ( defined $issuingrule->{holds_per_record} && $issuingrule->{holds_per_record} == 0 ) || >+ ( defined $issuingrule->{holds_per_day} && $issuingrule->{holds_per_day} == 0 ) >+ ) { > return 'InvalidHold'; > } else { > return 'OK'; >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 2ec5b83c3b2..6f2a84fa09a 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -3385,9 +3385,6 @@ subtest 'Set waiting flag' => sub { > my ( $status ) = CheckReserves($item->itemnumber); > is( $status, 'Transferred', 'Hold is not waiting yet'); > >- my ( $status ) = CheckReserves($item->{itemnumber}); >- is( $status, 'Reserved', 'Hold is not waiting yet'); >- > set_userenv( $library_2 ); > $do_transfer = 0; > AddReturn( $item->barcode, $library_2->{branchcode} ); >diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t >index 74e39c00703..e4e599b06f1 100755 >--- a/t/db_dependent/Hold.t >+++ b/t/db_dependent/Hold.t >@@ -28,7 +28,8 @@ use Koha::Holds; > use Koha::Item; > use Koha::DateUtils; > use t::lib::TestBuilder; >-use Koha::IssuingRules; >+use Koha::CirculationRules; >+use C4::Reserves; > > use Test::More tests => 33; > use Test::Exception; >@@ -288,99 +289,155 @@ subtest 'suspend() tests' => sub { > $schema->storage->txn_rollback; > }; > >-subtest "check_if_existing_hold_matches_issuingrules tests" => sub { >+subtest "matches_circulation_rules tests" => sub { > plan tests => 9; > > $schema->storage->txn_begin(); > > #Create test data >- my $branchcode = $builder->build( { source => 'Branch' } )->{ branchcode }; >- my $categorycode1 = $builder->build({ source => 'Category' })->{categorycode}; >- my $categorycode2 = $builder->build({ source => 'Category' })->{categorycode}; >- my $patron1 = $builder->build({source => 'Borrower', value => { branchcode => $branchcode }}); >- >- my $item1 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }}); >- my $item2 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }}); >- >- my $itemtype1 = $item1->{'itype'}; >- my $itemtype2 = $item2->{'itype'}; >- >- Koha::IssuingRules->delete; >- >- #Get branchcode >- my $controlbranch = C4::Context->preference('ReservesControlBranch'); >- >- if ( $controlbranch eq "ItemHomeLibrary" ) { >- $branchcode = $item->{homebranch}; >- } >- elsif ( $controlbranch eq "PatronLibrary" ) { >- $branchcode = $patron1->{branchcode}; >- } >- >- #Test all cases when the check_return >- #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed, holds_per_record and holds_per_day are all 0 >- my $rule1 = Koha::IssuingRule->new({ >- branchcode => $branchcode, >- categorycode => $patron1->{'categorycode'}, >- itemtype => $item1->{'itype'}, >- reservesallowed => 0, >- holds_per_record => 0, >- holds_per_day => 0, >- })->store; >- my $checkreserve1 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); >- is( $checkreserve1, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed, holds_per_record and holds_per_day is 0 for this patron category/itemtype combination' ); >- >- #Confirm that when any of the 4 reserves related issuingrules fields (reservesallowed, holds_per_record, holds_per_day) are set to 0 then check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' and so a newly returned item is not allocated to a specific bib-level reserve >- >- #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed is 0 for the patron category/itemtype combination >- $rule1->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store; >- my $checkreserve2 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); >- is( $checkreserve2, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed is 0 for this patron category/itemtype combination' ); >- >- #Now change to reservesallowed = 1, holds_per_record = 0, holds_per_day = 1 >- $rule1->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store; >- my $checkreserve3 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); >- is( $checkreserve3, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to 0 for this patron category/itemtype combination' ); >- >- #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 0 >- $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store; >- my $checkreserve4 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); >- is( $checkreserve4, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to0 for this patron category/itemtype combination' ); >- >- #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 1 and confirm 'OK' is returned >- $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 1 })->store; >- my $checkreserve5 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); >- is( $checkreserve5, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' ); >- >- #Create a default rule (patron categories: ALL, itemtypes: ALL) >- my $rule3 = Koha::IssuingRule->new({ >- branchcode => $branchcode, >- categorycode => '*', >- itemtype => '*', >- reservesallowed => 1, >- holds_per_record => 1, >- holds_per_day => 1, >- })->store; >- >- #Check that when no specific patron category/item type issuingrule combo exists we revert to using the default ALL categories/ALL >- #itemtypes issuingrule. When generic rule is reservesallowed, holds_per_record and holds_per_day = 1 then 'OK' is returned >- my $checkreserve6 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); >- is( $checkreserve6, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' ); >- >- #When default rule is reservesallowed = 0, holds_per_record = 1, holds_per_day = 1 then 'InvalidHold is returned >- $rule3->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store; >- my $checkreserve7 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); >- is( $checkreserve7, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as reservesallowed is 0' ); >- >- #When default rule is reservesallowed = 1, holds_per_record = 0, holds_per_day = 1 then 'InvalidHold' is returned >- $rule3->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store; >- my $checkreserve8 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); >- is( $checkreserve8, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_record is 0' ); >- >- #When default rule is reservesallowed = 1, holds_per_record = 1, holds_per_day = 0 then 'InvalidHold' is returned >- $rule3->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store; >- my $checkreserve9 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); >- is( $checkreserve9, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_day is 0' ); >+ my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; >+ my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; >+ my $patron = $builder->build({ source => 'Borrower', value => { categorycode => $categorycode, branchcode => $branchcode }}); >+ my $biblionumber = $builder->build({ source => 'Biblio' })->{biblionumber}; >+ my $itemtype1 = $builder->build({ source => 'Itemtype' })->{itemtype}; >+ my $itemtype2 = $builder->build({ source => 'Itemtype' })->{itemtype}; >+ my $item1 = $builder->build({ source => 'Item', value => { homebranch => $branchcode, biblionumber => $biblionumber, itype => $itemtype1 }}); >+ my $item2 = $builder->build({ source => 'Item', value => { homebranch => $branchcode, biblionumber => $biblionumber, itype => $itemtype2 }}); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype1, >+ rules => { >+ reservesallowed => 0, >+ holds_per_record => 0, >+ holds_per_day => 0, >+ } >+ }); >+ >+ my $reserveid1 = AddReserve({ >+ branchcode => $branchcode, >+ borrowernumber => $patron->{borrowernumber}, >+ biblionumber => $biblionumber, >+ }); >+ my $reserve1 = Koha::Holds->find($reserveid1); >+ >+ my $checkreserve1 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); >+ is( $checkreserve1, 'InvalidHold', 'Circulation rules disallow item being allocated to the hold' ); >+ >+ #Confirm that when any of the 3 reserves-related circulaion rules fields (reservesallowed, holds_per_record, holds_per_day) are set to 0 then matches_circulation_rules returns 'InvalidHold' and so a newly returned item is not allocated to a specific bib-level reserve >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype1, >+ rules => { >+ reservesallowed => 0, >+ holds_per_record => 1, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve2 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); >+ is( $checkreserve2, 'InvalidHold', 'Reserves allowed is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype1, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 0, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve3 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); >+ is( $checkreserve3, 'InvalidHold', 'Holds per record is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype1, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 0, >+ } >+ }); >+ my $checkreserve4 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); >+ is( $checkreserve4, 'InvalidHold', 'Holds per day is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype1, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve5 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); >+ is( $checkreserve5, 'OK', 'Hold is allowed, item allocated to hold' ); >+ >+ #Check that when no specific patron category/item type circulation rule combo exists we revert to using the default ALL categories/ALL >+ my $reserveid2 = AddReserve({ >+ branchcode => $branchcode, >+ borrowernumber => $patron->{borrowernumber}, >+ biblionumber => $biblionumber, >+ }); >+ my $reserve2 = Koha::Holds->find($reserveid2); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ reservesallowed => 0, >+ holds_per_record => 1, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve6 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); >+ is( $checkreserve6, 'InvalidHold', 'Reserves allowed is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 0, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve7 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); >+ is( $checkreserve7, 'InvalidHold', 'Holds per record is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 0, >+ } >+ }); >+ my $checkreserve8 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); >+ is( $checkreserve8, 'InvalidHold', 'Holds per day is 0' ); >+ >+ Koha::CirculationRules->set_rules({ >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 1, >+ } >+ }); >+ my $checkreserve9 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); >+ is( $checkreserve9, 'OK', 'Hold is allowed, item allocated to hold' ); > > $schema->storage->txn_rollback; > }; >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index a342f59cf42..3ca36fdfab6 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 66; >+use Test::More tests => 71; > use Test::MockModule; > use Test::Warn; > >@@ -529,13 +529,14 @@ AddReserve( > priority => 1, > } > ); >+ > my (undef, $canres, undef) = CheckReserves($item->itemnumber); > > is( CanReserveBeCanceledFromOpac(), undef, > 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' > ); > is( >- CanReserveBeCanceledFromOpac( $canres->{resserve_id} ), >+ CanReserveBeCanceledFromOpac( $canres->{reserve_id} ), > undef, > 'CanReserveBeCanceledFromOpac should return undef if called without the reserve_id' > ); >@@ -724,67 +725,101 @@ MoveReserve( $item->itemnumber, $borrowernumber ); > 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; >+#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 circulation_rules'); > > #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 >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ reservesallowed => 25, >+ holds_per_record => 1, >+ holds_per_day => 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_1 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 0 >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => $branch_1, >+ categorycode => $category_1, >+ itemtype => $itemtype2, >+ rules => { >+ reservesallowed => 0, >+ holds_per_record => 0, >+ holds_per_day => 0, >+ } >+ } > ); > > #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 >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => $branch_1, >+ categorycode => $category_2, >+ itemtype => $itemtype2, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 1, >+ } >+ } >+); >+ >+#When patron category = $category_1 and itemtype = $itemtype2 AND different branch, then reservesallowed, holds_per_day and holds_per_record are 0 >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => $branch_2, >+ categorycode => $category_1, >+ itemtype => $itemtype2, >+ rules => { >+ reservesallowed => 0, >+ holds_per_record => 0, >+ holds_per_day => 0, >+ } >+ } > ); > >+#Create a new biblio record >+my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; >+ >+# Create an item from new itemtype >+my $item2 = $builder->build_sample_item({ >+ homebranch => $branch_1, >+ holdingbranch => $branch_1, >+ itype => $itemtype2, >+ barcode => 'bug23172_CPL', >+ biblionumber => $bibnum3 >+}); >+ >+#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; >+my $patronnumcat3 = Koha::Patron->new({branchcode => $branch_2, categorycode => $category_1})->store->borrowernumber; >+ > #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 ); >+my $reserveid1 = AddReserve({ >+ branchcode => $branch_1, >+ borrowernumber => $patronnumcat1, >+ biblionumber => $bibnum3 >+}); >+ >+#Create Bib-level hold for borrower who has a patron category of $category_2 >+my $reserveid2 = AddReserve({ >+ branchcode => $branch_1, >+ borrowernumber => $patronnumcat2, >+ biblionumber => $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); >@@ -793,27 +828,58 @@ 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 >+#Test with ReservesControlBranch system preference set to ItemHomeLibrary >+t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); >+ >+# Reserve 1 should be skipped because the circ rule for that patron's category does not allow reserves > (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)'); >+is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'First priority hold is skipped because circulation rules disallow holds for this patron category, so item is allocated to next hold.'); > >-#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 >+#Remove existing reserves >+$dbh->do("DELETE FROM reserves"); >+ >+#Bib-level hold for borrower who has a patron category of $category_1 >+my $reserveid3 = AddReserve({ >+ branchcode => $branch_1, >+ borrowernumber => $patronnumcat3, >+ biblionumber => $bibnum3 >+}); >+ >+#Create Bib-level hold for borrower who has a patron category of $category_2 >+my $reserveid4 = AddReserve({ >+ branchcode => $branch_1, >+ borrowernumber => $patronnumcat2, >+ biblionumber => $bibnum3 >+}); >+ >+#Test with ReservesControlBranch system preference set to PatronLibrary >+t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); >+ >+# Reserve 1 should be skipped because the circ rule for that patron's library does not allow reserves >+(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,'First priority hold is skipped because circulation rules disallow holds for this branch, so item is allocated to next hold.'); >+ >+# Update the circulation rule that prevented patronnumcat3 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0 >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => $branch_2, >+ categorycode => $category_1, >+ itemtype => $itemtype2, >+ rules => { >+ reservesallowed => 1, >+ holds_per_record => 1, >+ holds_per_day => 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)'); >+is( $messages->{ResFound}->{borrowernumber}, $patronnumcat3,'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"); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23172
:
90868
|
91133
|
91301
|
92250
|
92251
|
92252
|
102099
|
102100
|
102101
|
102169
|
118930
|
118931
|
118932
|
118933
|
128817
|
128857