Bugzilla – Attachment 38398 Details for
Bug 13687
ILS-DI services HoldItem and HoldTitle don't consider hold policy by item type
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[PASSED QA] Bug 13687: Move hold policy check into CanItemBeReserved
Bug-13687-Move-hold-policy-check-into-CanItemBeRes.patch (text/plain), 6.64 KB, created by
Julian Maurice
on 2015-04-23 10:17:14 UTC
(
hide
)
Description:
[PASSED QA] Bug 13687: Move hold policy check into CanItemBeReserved
Filename:
MIME Type:
Creator:
Julian Maurice
Created:
2015-04-23 10:17:14 UTC
Size:
6.64 KB
patch
obsolete
>From 3b3f352a8957d89e8aad83074e959c6d1ceb440d Mon Sep 17 00:00:00 2001 >From: Julian Maurice <julian.maurice@biblibre.com> >Date: Tue, 10 Feb 2015 11:34:01 +0100 >Subject: [PATCH] Bug 13687: Move hold policy check into CanItemBeReserved > >This way ILS-DI HoldItem and HoldTitle services also benefit from this >check > >Test plan: > >1/ Define some default holds policies by item type in >/admin/smart-rules.pl >2/ Use ILS-DI HoldItem service and check that those rules are respected >3/ Check that staff and opac hold behaviour is unchanged regarding >these rules. > >Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> >Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> >Passes tests and QA script. No regressions found, >improves the ILS-DI HoldItem response. >--- > C4/Reserves.pm | 18 +++++++++++++++++- > opac/opac-reserve.pl | 10 ---------- > reserve/request.pl | 10 +--------- > t/db_dependent/Holds.t | 42 +++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 59 insertions(+), 21 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index b82387f..763b22d 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -119,7 +119,7 @@ BEGIN { > > &CheckReserves > &CanBookBeReserved >- &CanItemBeReserved >+ &CanItemBeReserved > &CanReserveBeCanceledFromOpac > &CancelReserve > &CancelExpiredReserves >@@ -488,6 +488,7 @@ sub CanBookBeReserved{ > damaged, if the Item is damaged. > cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK. > tooManyReserves, if the borrower has exceeded his maximum reserve amount. >+ notReservable, if holds on this item are not allowed > > =cut > >@@ -578,6 +579,21 @@ sub CanItemBeReserved{ > return 'tooManyReserves'; > } > >+ my $circ_control_branch = C4::Circulation::_GetCircControlBranch($item, >+ $borrower); >+ my $branchitemrule = C4::Circulation::GetBranchItemRule($circ_control_branch, >+ $item->{itype}); >+ >+ if ( $branchitemrule->{holdallowed} == 0 ) { >+ return 'notReservable'; >+ } >+ >+ if ( $branchitemrule->{holdallowed} == 1 >+ && $borrower->{branchcode} ne $item->{homebranch} ) >+ { >+ return 'cannotReserveFromOtherBranches'; >+ } >+ > # If reservecount is ok, we check item branch if IndependentBranches is ON > # and canreservefromotherbranches is OFF > if ( C4::Context->preference('IndependentBranches') >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index c30ee08..bfd0a30 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -519,16 +519,6 @@ foreach my $biblioNum (@biblionumbers) { > my $branch = GetReservesControlBranch( $itemInfo, $borr ); > > my $policy_holdallowed = !$itemLoopIter->{already_reserved}; >- if ($policy_holdallowed) { >- if (my $branchitemrule = GetBranchItemRule( $branch, $itemInfo->{'itype'} )) { >- $policy_holdallowed = >- ($branchitemrule->{'holdallowed'} == 2) || >- ($branchitemrule->{'holdallowed'} == 1 >- && $borr->{'branchcode'} eq $itemInfo->{'homebranch'}); >- } else { >- $policy_holdallowed = 0; # No rule - not allowed >- } >- } > $policy_holdallowed &&= > IsAvailableForItemLevelRequest($itemInfo,$borr) && > CanItemBeReserved($borrowernumber,$itemNum) eq 'OK'; >diff --git a/reserve/request.pl b/reserve/request.pl >index b4b749e..5978d11 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -408,19 +408,11 @@ foreach my $biblionumber (@biblionumbers) { > my $branch = C4::Circulation::_GetCircControlBranch($item, $borrowerinfo); > > my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); >- my $policy_holdallowed = 1; > > $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; > >- if ( $branchitemrule->{'holdallowed'} == 0 || >- ( $branchitemrule->{'holdallowed'} == 1 && >- $borrowerinfo->{'branchcode'} ne $item->{'homebranch'} ) ) { >- $policy_holdallowed = 0; >- } >- > if ( >- $policy_holdallowed >- && !$item->{cantreserve} >+ !$item->{cantreserve} > && IsAvailableForItemLevelRequest($item, $borrowerinfo) > && CanItemBeReserved( > $borrowerinfo->{borrowernumber}, $itemnumber >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index 4aee01b..5a480d8 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -6,7 +6,7 @@ use t::lib::Mocks; > use C4::Context; > use C4::Branch; > >-use Test::More tests => 38; >+use Test::More tests => 41; > use MARC::Record; > use C4::Biblio; > use C4::Items; >@@ -329,6 +329,46 @@ ok( > "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)" > ); > >+ >+# Test branch item rules >+ >+$dbh->do('DELETE FROM issuingrules'); >+$dbh->do( >+ q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) >+ VALUES (?, ?, ?, ?)}, >+ {}, >+ '*', '*', '*', 25 >+); >+$dbh->do('DELETE FROM branch_item_rules'); >+$dbh->do('DELETE FROM default_branch_circ_rules'); >+$dbh->do('DELETE FROM default_branch_item_rules'); >+$dbh->do('DELETE FROM default_circ_rules'); >+$dbh->do(q{ >+ INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, returnbranch) >+ VALUES (?, ?, ?, ?) >+}, {}, 'CPL', 'CANNOT', 0, 'homebranch'); >+$dbh->do(q{ >+ INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, returnbranch) >+ VALUES (?, ?, ?, ?) >+}, {}, 'CPL', 'CAN', 1, 'homebranch'); >+($bibnum, $title, $bibitemnum) = create_helper_biblio('CANNOT'); >+($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem( >+ { homebranch => 'CPL', holdingbranch => 'CPL', itype => 'CANNOT' } , $bibnum); >+is(CanItemBeReserved($borrowernumbers[0], $itemnumber), 'notReservable', >+ "CanItemBeReserved should returns 'notReservable'"); >+ >+($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem( >+ { homebranch => 'MPL', holdingbranch => 'CPL', itype => 'CAN' } , $bibnum); >+is(CanItemBeReserved($borrowernumbers[0], $itemnumber), >+ 'cannotReserveFromOtherBranches', >+ "CanItemBeReserved should returns 'cannotReserveFromOtherBranches'"); >+ >+($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem( >+ { homebranch => 'CPL', holdingbranch => 'CPL', itype => 'CAN' } , $bibnum); >+is(CanItemBeReserved($borrowernumbers[0], $itemnumber), 'OK', >+ "CanItemBeReserved should returns 'OK'"); >+ >+ > # Test CancelExpiredReserves > C4::Context->set_preference('ExpireReservesMaxPickUpDelay', 1); > C4::Context->set_preference('ReservesMaxPickUpDelay', 1); >-- >1.9.1
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 13687
:
35777
|
36014
|
36336
| 38398