Bugzilla – Attachment 164682 Details for
Bug 25408
CanBookBeReserved & CanItemBeReserved should check "opacitemholds" policy
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25408: (QA follow-up) using perltidy to resolve complaining QA scripts
Bug-25408-QA-follow-up-using-perltidy-to-resolve-c.patch (text/plain), 192.96 KB, created by
Arthur Suzuki
on 2024-04-11 12:41:48 UTC
(
hide
)
Description:
Bug 25408: (QA follow-up) using perltidy to resolve complaining QA scripts
Filename:
MIME Type:
Creator:
Arthur Suzuki
Created:
2024-04-11 12:41:48 UTC
Size:
192.96 KB
patch
obsolete
>From f439e3952fd4c362e76b9a80d902d375454c2c94 Mon Sep 17 00:00:00 2001 >From: Arthur Suzuki <arthur.suzuki@biblibre.com> >Date: Thu, 11 Apr 2024 14:30:18 +0200 >Subject: [PATCH] Bug 25408: (QA follow-up) using perltidy to resolve > complaining QA scripts > >--- > C4/Reserves.pm | 990 +++++++++++++++-------------- > opac/opac-reserve.pl | 293 ++++----- > t/db_dependent/Reserves.t | 1272 +++++++++++++++++++++---------------- > 3 files changed, 1396 insertions(+), 1159 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index bc158b31a2e..2e88165cf76 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -20,11 +20,10 @@ package C4::Reserves; > # You should have received a copy of the GNU General Public License > # along with Koha; if not, see <http://www.gnu.org/licenses>. > >- > use Modern::Perl; > > use C4::Accounts; >-use C4::Biblio qw( GetMarcFromKohaField ); >+use C4::Biblio qw( GetMarcFromKohaField ); > use C4::Circulation qw( CheckIfIssuedToPatron GetAgeRestriction GetBranchItemRule ); > use C4::Context; > use C4::Items qw( CartToShelf get_hostitemnumbers_of ); >@@ -97,53 +96,54 @@ This modules provides somes functions to deal with reservations. > > =cut > >-our (@ISA, @EXPORT_OK); >+our ( @ISA, @EXPORT_OK ); >+ > BEGIN { > require Exporter; >- @ISA = qw(Exporter); >+ @ISA = qw(Exporter); > @EXPORT_OK = qw( >- AddReserve >+ AddReserve > >- GetReserveStatus >+ GetReserveStatus > >- GetOtherReserves >- ChargeReserveFee >- GetReserveFee >+ GetOtherReserves >+ ChargeReserveFee >+ GetReserveFee > >- ModReserveAffect >- ModReserve >- ModReserveStatus >- ModReserveCancelAll >- ModReserveMinusPriority >- MoveReserve >+ ModReserveAffect >+ ModReserve >+ ModReserveStatus >+ ModReserveCancelAll >+ ModReserveMinusPriority >+ MoveReserve > >- CheckReserves >- CanBookBeReserved >- CanItemBeReserved >- CanReserveBeCanceledFromOpac >- CancelExpiredReserves >+ CheckReserves >+ CanBookBeReserved >+ CanItemBeReserved >+ CanReserveBeCanceledFromOpac >+ CancelExpiredReserves > >- AutoUnsuspendReserves >+ AutoUnsuspendReserves > >- IsAvailableForItemLevelRequest >- ItemsAnyAvailableAndNotRestricted >+ IsAvailableForItemLevelRequest >+ ItemsAnyAvailableAndNotRestricted > >- AlterPriority >- ToggleLowestPriority >+ AlterPriority >+ ToggleLowestPriority > >- ReserveSlip >- ToggleSuspend >- SuspendAll >+ ReserveSlip >+ ToggleSuspend >+ SuspendAll > >- CalculatePriority >+ CalculatePriority > >- IsItemOnHoldAndFound >+ IsItemOnHoldAndFound > >- GetMaxPatronHoldsForRecord >+ GetMaxPatronHoldsForRecord > >- MergeHolds >+ MergeHolds > >- RevertWaitingStatus >+ RevertWaitingStatus > ); > } > >@@ -185,27 +185,27 @@ The following tables are available within the HOLDPLACED_PATRON message: > =cut > > sub AddReserve { >- my ($params) = @_; >- my $branch = $params->{branchcode}; >- my $borrowernumber = $params->{borrowernumber}; >- my $biblionumber = $params->{biblionumber}; >- my $priority = $params->{priority}; >- my $resdate = $params->{reservation_date}; >+ my ($params) = @_; >+ my $branch = $params->{branchcode}; >+ my $borrowernumber = $params->{borrowernumber}; >+ my $biblionumber = $params->{biblionumber}; >+ my $priority = $params->{priority}; >+ my $resdate = $params->{reservation_date}; > my $patron_expiration_date = $params->{expiration_date}; >- my $notes = $params->{notes}; >- my $title = $params->{title}; >- my $checkitem = $params->{itemnumber}; >- my $found = $params->{found}; >- my $itemtype = $params->{itemtype}; >- my $non_priority = $params->{non_priority}; >- my $item_group_id = $params->{item_group_id}; >+ my $notes = $params->{notes}; >+ my $title = $params->{title}; >+ my $checkitem = $params->{itemnumber}; >+ my $found = $params->{found}; >+ my $itemtype = $params->{itemtype}; >+ my $non_priority = $params->{non_priority}; >+ my $item_group_id = $params->{item_group_id}; > > $resdate ||= dt_from_string; > > # if we have an item selectionned, and the pickup branch is the same as the holdingbranch > # of the document, we force the value $priority and $found . > if ( $checkitem and not C4::Context->preference('ReservesNeedReturns') ) { >- my $item = Koha::Items->find( $checkitem ); # FIXME Prevent bad calls >+ my $item = Koha::Items->find($checkitem); # FIXME Prevent bad calls > > if ( > # If item is already checked out, it cannot be set waiting >@@ -221,15 +221,17 @@ sub AddReserve { > && ( $item->damaged && C4::Context->preference('AllowHoldsOnDamagedItems') || !$item->damaged ) > > # Lastly, if this already has holds, we shouldn't make it waiting for the new hold >- && !$item->current_holds->count ) >+ && !$item->current_holds->count >+ ) > { > $priority = 0; >- $found = 'W'; >+ $found = 'W'; > } > } >- if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) { >- # Make room in reserves for this if passed a priority >- $priority = _ShiftPriority( $biblionumber, $priority ); >+ if ( C4::Context->preference('AllowHoldDateInFuture') ) { >+ >+ # Make room in reserves for this if passed a priority >+ $priority = _ShiftPriority( $biblionumber, $priority ); > } > > my $waitingdate; >@@ -245,20 +247,20 @@ sub AddReserve { > # updates take place here > my $hold = Koha::Hold->new( > { >- borrowernumber => $borrowernumber, >- biblionumber => $biblionumber, >- item_group_id => $item_group_id, >- reservedate => $resdate, >- branchcode => $branch, >- priority => $priority, >- reservenotes => $notes, >- itemnumber => $checkitem, >- found => $found, >- waitingdate => $waitingdate, >+ borrowernumber => $borrowernumber, >+ biblionumber => $biblionumber, >+ item_group_id => $item_group_id, >+ reservedate => $resdate, >+ branchcode => $branch, >+ priority => $priority, >+ reservenotes => $notes, >+ itemnumber => $checkitem, >+ found => $found, >+ waitingdate => $waitingdate, > patron_expiration_date => $patron_expiration_date, >- itemtype => $itemtype, >- item_level_hold => $checkitem ? 1 : 0, >- non_priority => $non_priority ? 1 : 0, >+ itemtype => $itemtype, >+ item_level_hold => $checkitem ? 1 : 0, >+ non_priority => $non_priority ? 1 : 0, > } > )->store(); > $hold->set_waiting() if $found && $found eq 'W'; >@@ -277,26 +279,29 @@ sub AddReserve { > ChargeReserveFee( $borrowernumber, $reserve_fee, $title ); > } > >- _FixPriority({ biblionumber => $biblionumber}); >+ _FixPriority( { biblionumber => $biblionumber } ); > > # Send e-mail to librarian if syspref is active >- if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){ >- my $patron = $hold->patron; >+ if ( C4::Context->preference("emailLibrarianWhenHoldIsPlaced") ) { >+ my $patron = $hold->patron; > my $library = $patron->library; >- if ( my $letter = C4::Letters::GetPreparedLetter ( >- module => 'reserves', >- letter_code => 'HOLDPLACED', >- branchcode => $branch, >- lang => $patron->lang, >- tables => { >- 'branches' => $library->unblessed, >- 'borrowers' => $patron->unblessed, >- 'biblio' => $biblionumber, >- 'biblioitems' => $biblionumber, >- 'items' => $checkitem, >- 'reserves' => $hold->unblessed, >- }, >- ) ) { >+ if ( >+ my $letter = C4::Letters::GetPreparedLetter( >+ module => 'reserves', >+ letter_code => 'HOLDPLACED', >+ branchcode => $branch, >+ lang => $patron->lang, >+ tables => { >+ 'branches' => $library->unblessed, >+ 'borrowers' => $patron->unblessed, >+ 'biblio' => $biblionumber, >+ 'biblioitems' => $biblionumber, >+ 'items' => $checkitem, >+ 'reserves' => $hold->unblessed, >+ }, >+ ) >+ ) >+ { > > my $branch_email_address = $library->inbound_email_address; > >@@ -338,7 +343,7 @@ sub AddReserve { > } > } > >- Koha::Plugins->call('after_hold_create', $hold); >+ Koha::Plugins->call( 'after_hold_create', $hold ); > Koha::Plugins->call( > 'after_hold_action', > { >@@ -347,11 +352,8 @@ sub AddReserve { > } > ); > >- Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( >- { >- biblio_ids => [ $biblionumber ] >- } >- ) if C4::Context->preference('RealTimeHoldsQueue'); >+ Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [$biblionumber] } ) >+ if C4::Context->preference('RealTimeHoldsQueue'); > > return $reserve_id; > } >@@ -367,13 +369,14 @@ See CanItemBeReserved() for possible return values. > > =cut > >-sub CanBookBeReserved{ >- my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_; >+sub CanBookBeReserved { >+ my ( $borrowernumber, $biblionumber, $pickup_branchcode, $params ) = @_; > > # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) > if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') >- && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) { >- return { status =>'alreadypossession' }; >+ && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) >+ { >+ return { status => 'alreadypossession' }; > } > > if ( $params->{itemtype} ) { >@@ -398,38 +401,43 @@ sub CanBookBeReserved{ > { 'item.itype' => $params->{itemtype} } > ] > }, >- { >- join => ['item'] >- } >+ { join => ['item'] } > )->count; > > return { status => '' } >- if defined $reservesallowed and $reservesallowed < $count + 1; >+ if defined $reservesallowed and $reservesallowed < $count + 1; > } > > my $items; >+ > #get items linked via host records > my @hostitemnumbers = get_hostitemnumbers_of($biblionumber); >- if (@hostitemnumbers){ >- $items = Koha::Items->search({ >- -or => [ >- biblionumber => $biblionumber, >- itemnumber => { -in => @hostitemnumbers } >- ] >- }); >+ if (@hostitemnumbers) { >+ $items = Koha::Items->search( >+ { >+ -or => [ >+ biblionumber => $biblionumber, >+ itemnumber => { -in => @hostitemnumbers } >+ ] >+ } >+ ); > } else { >- $items = Koha::Items->search({ biblionumber => $biblionumber}); >+ $items = Koha::Items->search( { biblionumber => $biblionumber } ); > } > > my $canReserve = { status => '' }; >- my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $patron = Koha::Patrons->find($borrowernumber); > while ( my $item = $items->next ) { > $canReserve = CanItemBeReserved( $patron, $item, $pickup_branchcode, $params ); > if ( C4::Context->interface eq 'opac' ) { >- my $opacitemholds = Koha::CirculationRules->get_opacitemholds_policy({ item => $item, >- patron => $patron }) // 'Y'; >+ my $opacitemholds = Koha::CirculationRules->get_opacitemholds_policy( >+ { >+ item => $item, >+ patron => $patron >+ } >+ ) // 'Y'; > return { status => 'recordHoldNotAllowed' } if $opacitemholds eq 'F'; >- return { status => 'OK' } if $canReserve->{status} eq 'recordHoldsOnly'; >+ return { status => 'OK' } if $canReserve->{status} eq 'recordHoldsOnly'; > } > return { status => 'OK' } if $canReserve->{status} eq 'OK'; > } >@@ -461,8 +469,9 @@ sub CanBookBeReserved{ > =cut > > our $CanItemBeReserved_cache_key; >+ > sub _cache { >- my ( $return ) = @_; >+ my ($return) = @_; > my $memory_cache = Koha::Cache::Memory::Lite->get_instance(); > $memory_cache->set_in_cache( $CanItemBeReserved_cache_key, $return ); > return $return; >@@ -472,15 +481,16 @@ sub CanItemBeReserved { > my ( $patron, $item, $pickup_branchcode, $params ) = @_; > > my $memory_cache = Koha::Cache::Memory::Lite->get_instance(); >- $CanItemBeReserved_cache_key = sprintf "Hold_CanItemBeReserved:%s:%s:%s", $patron->id, $item->itemnumber, $pickup_branchcode || ""; >+ $CanItemBeReserved_cache_key = sprintf "Hold_CanItemBeReserved:%s:%s:%s", $patron->id, $item->itemnumber, >+ $pickup_branchcode || ""; > if ( $params->{get_from_cache} ) { > my $cached = $memory_cache->get_from_cache($CanItemBeReserved_cache_key); > return $cached if $cached; > } > > my $dbh = C4::Context->dbh; >- my $ruleitemtype; # itemtype of the matching issuing rule >- my $allowedreserves = 0; # Total number of holds allowed across all records, default to none >+ my $ruleitemtype; # itemtype of the matching issuing rule >+ my $allowedreserves = 0; # Total number of holds allowed across all records, default to none > > # We check item branch if IndependentBranches is ON > # and canreservefromotherbranches is OFF >@@ -497,31 +507,35 @@ sub CanItemBeReserved { > my $borrower = $patron->unblessed; > > # If an item is damaged and we don't allow holds on damaged items, we can stop right here >- return _cache { status =>'damaged' } >- if ( $item->damaged >+ return _cache { status => 'damaged' } >+ if ( $item->damaged > && !C4::Context->preference('AllowHoldsOnDamagedItems') ); > >- if( GetMarcFromKohaField('biblioitems.agerestriction') ){ >+ if ( GetMarcFromKohaField('biblioitems.agerestriction') ) { > my $biblio = $item->biblio; >+ > # Check for the age restriction > my ( $ageRestriction, $daysToAgeRestriction ) = >- C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction, $borrower ); >+ C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction, $borrower ); > return _cache { status => 'ageRestricted' } if $daysToAgeRestriction && $daysToAgeRestriction > 0; > } > > # Check that the patron doesn't have an item level hold on this item already >- return _cache { status =>'itemAlreadyOnHold' } >- if ( !$params->{ignore_hold_counts} && Koha::Holds->search( { borrowernumber => $patron->borrowernumber, itemnumber => $item->itemnumber } )->count() ); >+ return _cache { status => 'itemAlreadyOnHold' } >+ if ( !$params->{ignore_hold_counts} >+ && Koha::Holds->search( { borrowernumber => $patron->borrowernumber, itemnumber => $item->itemnumber } ) >+ ->count() ); > > # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) > if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') >- && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $item->biblionumber ) ) { >- return _cache { status =>'alreadypossession' }; >+ && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $item->biblionumber ) ) >+ { >+ return _cache { status => 'alreadypossession' }; > } > > # check if a recall exists on this item from this borrower > return _cache { status => 'recall' } >- if $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count; >+ if $patron->recalls->filter_by_current->search( { item_id => $item->itemnumber } )->count; > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > >@@ -529,41 +543,44 @@ sub CanItemBeReserved { > my $branchfield = "reserves.branchcode"; > > if ( $controlbranch eq "ItemHomeLibrary" ) { >- $branchfield = "items.homebranch"; >- $reserves_control_branch = $item->homebranch; >- } >- elsif ( $controlbranch eq "PatronLibrary" ) { >- $branchfield = "borrowers.branchcode"; >- $reserves_control_branch = $borrower->{branchcode}; >+ $branchfield = "items.homebranch"; >+ $reserves_control_branch = $item->homebranch; >+ } elsif ( $controlbranch eq "PatronLibrary" ) { >+ $branchfield = "borrowers.branchcode"; >+ $reserves_control_branch = $borrower->{branchcode}; > } > > # we retrieve rights > if ( >- my $reservesallowed = Koha::CirculationRules->get_effective_rule({ >+ my $reservesallowed = Koha::CirculationRules->get_effective_rule( >+ { > itemtype => $item->effective_itemtype, > categorycode => $borrower->{categorycode}, > branchcode => $reserves_control_branch, > rule_name => 'reservesallowed', >- }) >- ) { >- $ruleitemtype = $reservesallowed->itemtype; >- $allowedreserves = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited >- } >- else { >+ } >+ ) >+ ) >+ { >+ $ruleitemtype = $reservesallowed->itemtype; >+ $allowedreserves = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited >+ } else { > $ruleitemtype = undef; > } > >- my $rights = Koha::CirculationRules->get_effective_rules({ >- categorycode => $borrower->{'categorycode'}, >- itemtype => $item->effective_itemtype, >- branchcode => $reserves_control_branch, >- rules => ['holds_per_record','holds_per_day','opacitemholds'] >- }); >+ my $rights = Koha::CirculationRules->get_effective_rules( >+ { >+ categorycode => $borrower->{'categorycode'}, >+ itemtype => $item->effective_itemtype, >+ branchcode => $reserves_control_branch, >+ rules => [ 'holds_per_record', 'holds_per_day', 'opacitemholds' ] >+ } >+ ); > my $holds_per_record = $rights->{holds_per_record} // 1; > my $holds_per_day = $rights->{holds_per_day}; > my $opacitemholds = $rights->{opacitemholds} // 'Y'; > >- if ( defined $holds_per_record && $holds_per_record ne '' ){ >+ if ( defined $holds_per_record && $holds_per_record ne '' ) { > if ( $holds_per_record == 0 ) { > return _cache { status => "noReservesAllowed" }; > } >@@ -573,25 +590,29 @@ sub CanItemBeReserved { > biblionumber => $item->biblionumber, > }; > my $holds = Koha::Holds->search($search_params); >- return _cache { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } if $holds->count() >= $holds_per_record; >+ return _cache { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } >+ if $holds->count() >= $holds_per_record; > } > } > >- if (!$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '') >- { >- my $today_holds = Koha::Holds->search({ >- borrowernumber => $patron->borrowernumber, >- reservedate => dt_from_string->date >- }); >- return _cache { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds->count() >= $holds_per_day; >+ if ( !$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '' ) { >+ my $today_holds = Koha::Holds->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ reservedate => dt_from_string->date >+ } >+ ); >+ return _cache { status => 'tooManyReservesToday', limit => $holds_per_day } >+ if $today_holds->count() >= $holds_per_day; > } > > # we check if it's ok or not >- if ( defined $allowedreserves && $allowedreserves ne '' ){ >- if( $allowedreserves == 0 ){ >+ if ( defined $allowedreserves && $allowedreserves ne '' ) { >+ if ( $allowedreserves == 0 ) { > return _cache { status => 'noReservesAllowed' }; > } > if ( !$params->{ignore_hold_counts} ) { >+ > # we retrieve count > my $querycount = q{ > SELECT count(*) AS count >@@ -611,8 +632,7 @@ sub CanItemBeReserved { > AND ( COALESCE( items.itype, biblioitems.itemtype ) = ? > OR reserves.itemtype = ? ) > }; >- } >- else { >+ } else { > $querycount .= q{ > AND ( biblioitems.itemtype = ? > OR reserves.itemtype = ? ) >@@ -624,8 +644,7 @@ sub CanItemBeReserved { > > if ( defined $ruleitemtype ) { > $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype, $ruleitemtype ); >- } >- else { >+ } else { > $sthcount->execute( $patron->borrowernumber, $reserves_control_branch ); > } > >@@ -634,7 +653,8 @@ sub CanItemBeReserved { > $reservecount = $rowcount->{count}; > } > >- return _cache { status => 'tooManyReserves', limit => $allowedreserves } if $reservecount >= $allowedreserves; >+ return _cache { status => 'tooManyReserves', limit => $allowedreserves } >+ if $reservecount >= $allowedreserves; > } > } > >@@ -646,18 +666,14 @@ sub CanItemBeReserved { > rule_name => 'max_holds', > } > ); >- if (!$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) { >- my $total_holds_count = Koha::Holds->search( >- { >- borrowernumber => $patron->borrowernumber >- } >- )->count(); >+ if ( !$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) { >+ my $total_holds_count = Koha::Holds->search( { borrowernumber => $patron->borrowernumber } )->count(); > >- return _cache { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value; >+ return _cache { status => 'tooManyReserves', limit => $rule->rule_value } >+ if $total_holds_count >= $rule->rule_value; > } > >- my $branchitemrule = >- C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); >+ my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); > > if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) { > return _cache { status => 'notReservable' }; >@@ -669,36 +685,45 @@ sub CanItemBeReserved { > return _cache { status => 'cannotReserveFromOtherBranches' }; > } > >- my $item_library = Koha::Libraries->find( {branchcode => $item->homebranch} ); >- if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group') { >- if($patron->branchcode ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $patron->branchcode} )) { >+ my $item_library = Koha::Libraries->find( { branchcode => $item->homebranch } ); >+ if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group' ) { >+ if ( $patron->branchcode ne $item->homebranch >+ && !$item_library->validate_hold_sibling( { branchcode => $patron->branchcode } ) ) >+ { > return _cache { status => 'branchNotInHoldGroup' }; > } > } > > if ($pickup_branchcode) { >- my $destination = Koha::Libraries->find({ >- branchcode => $pickup_branchcode, >- }); >+ my $destination = Koha::Libraries->find( >+ { >+ branchcode => $pickup_branchcode, >+ } >+ ); > > unless ($destination) { > return _cache { status => 'libraryNotFound' }; > } >- unless ($destination->pickup_location) { >+ unless ( $destination->pickup_location ) { > return _cache { status => 'libraryNotPickupLocation' }; > } >- unless ($item->can_be_transferred({ to => $destination })) { >+ unless ( $item->can_be_transferred( { to => $destination } ) ) { > return _cache { status => 'cannotBeTransferred' }; > } >- if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) { >+ if ( $branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' >+ && !$item_library->validate_hold_sibling( { branchcode => $pickup_branchcode } ) ) >+ { > return _cache { status => 'pickupNotInHoldGroup' }; > } >- if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $borrower->{branchcode}})->validate_hold_sibling({branchcode => $pickup_branchcode})) { >+ if ( $branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' >+ && !Koha::Libraries->find( { branchcode => $borrower->{branchcode} } ) >+ ->validate_hold_sibling( { branchcode => $pickup_branchcode } ) ) >+ { > return _cache { status => 'pickupNotInHoldGroup' }; > } > } > >- if ( $opacitemholds eq "N" && C4::Context->interface eq 'opac') { >+ if ( $opacitemholds eq "N" && C4::Context->interface eq 'opac' ) { > return _cache { status => "recordHoldsOnly" }; > } > >@@ -716,7 +741,7 @@ sub CanItemBeReserved { > =cut > > sub CanReserveBeCanceledFromOpac { >- my ($reserve_id, $borrowernumber) = @_; >+ my ( $reserve_id, $borrowernumber ) = @_; > > return unless $reserve_id and $borrowernumber; > my $reserve = Koha::Holds->find($reserve_id) or return; >@@ -742,6 +767,7 @@ sub GetOtherReserves { > if ($checkreserves) { > if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) { > $messages->{'transfert'} = $checkreserves->{'branchcode'}; >+ > #minus priorities of others reservs > ModReserveMinusPriority( > $itemnumber, >@@ -754,18 +780,18 @@ sub GetOtherReserves { > $item->holdingbranch, > $checkreserves->{'branchcode'}, > 'Reserve' >- ), >- ; >+ ), >+ ; > } > >- #step 2b : case of a reservation on the same branch, set the waiting status >+ #step 2b : case of a reservation on the same branch, set the waiting status > else { > $messages->{'waiting'} = 1; > ModReserveMinusPriority( > $itemnumber, > $checkreserves->{'reserve_id'}, > ); >- ModReserveStatus($itemnumber,'W'); >+ ModReserveStatus( $itemnumber, 'W' ); > } > > $nextreservinfo = $checkreserves; >@@ -823,21 +849,27 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? > }; > > my $dbh = C4::Context->dbh; >- my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) ); >+ my ($fee) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) ); > $fee += 0; > my $hold_fee_mode = C4::Context->preference('HoldFeeMode') || 'not_always'; >- if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) { >+ if ( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) { >+ > # This is a reconstruction of the old code: > # Compare number of items with items issued, and optionally check holds > # If not all items are issued and there are no holds: charge no fee > # NOTE: Lost, damaged, not-for-loan, etc. are just ignored here > my ( $notissued, $reserved ); >- ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef, >- ( $biblionumber ) ); >- if( $notissued == 0 ) { >+ ($notissued) = $dbh->selectrow_array( >+ $issue_qry, undef, >+ ($biblionumber) >+ ); >+ if ( $notissued == 0 ) { >+ > # all items are issued >- ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef, >- ( $biblionumber, $borrowernumber ) ); >+ ($reserved) = $dbh->selectrow_array( >+ $holds_qry, undef, >+ ( $biblionumber, $borrowernumber ) >+ ); > $fee = 0 if $reserved == 0; > } else { > $fee = 0; >@@ -864,22 +896,22 @@ sub GetReserveStatus { > > my $dbh = C4::Context->dbh; > >- my ($sth, $found, $priority); >- if ( $itemnumber ) { >+ my ( $sth, $found, $priority ); >+ if ($itemnumber) { > $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); > $sth->execute($itemnumber); >- ($found, $priority) = $sth->fetchrow_array; >+ ( $found, $priority ) = $sth->fetchrow_array; > } > >- if(defined $found) { >- return 'Waiting' if $found eq 'W' and $priority == 0; >- return 'Processing' if $found eq 'P'; >- return 'Finished' if $found eq 'F'; >+ if ( defined $found ) { >+ return 'Waiting' if $found eq 'W' and $priority == 0; >+ return 'Processing' if $found eq 'P'; >+ return 'Finished' if $found eq 'F'; > } > > return 'Reserved' if defined $priority && $priority > 0; > >- return ''; # empty string here will remove need for checking undef, or less log lines >+ return ''; # empty string here will remove need for checking undef, or less log lines > } > > =head2 CheckReserves >@@ -912,9 +944,10 @@ table in the Koha database. > > sub CheckReserves { > my ( $item, $lookahead_days, $ignore_borrowers ) = @_; >+ > # note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it. > >- return unless $item; # bail if we got nothing. >+ return unless $item; # bail if we got nothing. > > return if ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); > >@@ -927,16 +960,15 @@ sub CheckReserves { > my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? $item->notforloan > 0 : $item->notforloan; > if ( !$dont_trap ) { > my $item_type = $item->effective_itemtype; >- if ( $item_type ) { >- return if Koha::ItemTypes->find( $item_type )->notforloan; >+ if ($item_type) { >+ return if Koha::ItemTypes->find($item_type)->notforloan; > } >- } >- else { >+ } else { > return; > } > > # Find this item in the reserves >- my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers); >+ my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers ); > > # $priority and $highest are used to find the most important item > # in the list returned by &_Findgroupreserve. (The lower $priority, >@@ -944,19 +976,19 @@ sub CheckReserves { > # $highest is the most important item we've seen so far. > my $highest; > >- if (scalar @reserves) { >- my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); >+ if ( scalar @reserves ) { >+ my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); > my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl'); >- my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl'); >- my $priority = 10000000; >+ my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl'); >+ my $priority = 10000000; > > foreach my $res (@reserves) { >- if ($res->{'found'} && $res->{'found'} eq 'W') { >- return ( "Waiting", $res, \@reserves ); # Found it, it is waiting >- } elsif ($res->{'found'} && $res->{'found'} eq 'P') { >- return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one >- } elsif ($res->{'found'} && $res->{'found'} eq 'T') { >- return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one >+ if ( $res->{'found'} && $res->{'found'} eq 'W' ) { >+ return ( "Waiting", $res, \@reserves ); # Found it, it is waiting >+ } elsif ( $res->{'found'} && $res->{'found'} eq 'P' ) { >+ return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one >+ } elsif ( $res->{'found'} && $res->{'found'} eq 'T' ) { >+ return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one > } else { > my $patron; > my $local_hold_match; >@@ -964,36 +996,46 @@ sub CheckReserves { > if ($LocalHoldsPriority) { > $patron = Koha::Patrons->find( $res->{borrowernumber} ); > >- unless ($item->exclude_from_local_holds_priority || $patron->category->exclude_from_local_holds_priority) { >- my $local_holds_priority_item_branchcode = >- $item->$LocalHoldsPriorityItemControl; >+ unless ( $item->exclude_from_local_holds_priority >+ || $patron->category->exclude_from_local_holds_priority ) >+ { >+ my $local_holds_priority_item_branchcode = $item->$LocalHoldsPriorityItemControl; > my $local_holds_priority_patron_branchcode = >- ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) >- ? $res->{branchcode} >- : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) >- ? $patron->branchcode >- : undef; >+ ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' ) ? $res->{branchcode} >+ : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' ) ? $patron->branchcode >+ : undef; > $local_hold_match = >- $local_holds_priority_item_branchcode eq >- $local_holds_priority_patron_branchcode; >+ $local_holds_priority_item_branchcode eq $local_holds_priority_patron_branchcode; > } > } > > # See if this item is more important than what we've got so far > if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { >- next if $res->{item_group_id} && ( !$item->item_group || $item->item_group->id != $res->{item_group_id} ); >+ next >+ if $res->{item_group_id} >+ && ( !$item->item_group || $item->item_group->id != $res->{item_group_id} ); > next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype; > $patron //= Koha::Patrons->find( $res->{borrowernumber} ); >- my $branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); >- my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); >- next if ($branchitemrule->{'holdallowed'} eq 'not_allowed'); >- next if (($branchitemrule->{'holdallowed'} eq 'from_home_library') && ($item->homebranch ne $patron->branchcode)); >- my $library = Koha::Libraries->find({branchcode=>$item->homebranch}); >- next if (($branchitemrule->{'holdallowed'} eq 'from_local_hold_group') && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) )); >+ my $branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); >+ my $branchitemrule = C4::Circulation::GetBranchItemRule( $branch, $item->effective_itemtype ); >+ next if ( $branchitemrule->{'holdallowed'} eq 'not_allowed' ); >+ next >+ if ( ( $branchitemrule->{'holdallowed'} eq 'from_home_library' ) >+ && ( $item->homebranch ne $patron->branchcode ) ); >+ my $library = Koha::Libraries->find( { branchcode => $item->homebranch } ); >+ next >+ if ( ( $branchitemrule->{'holdallowed'} eq 'from_local_hold_group' ) >+ && ( !$library->validate_hold_sibling( { branchcode => $patron->branchcode } ) ) ); > my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy}; >- next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) ); >- next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) ); >- next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) ); >+ next >+ if ( ( $hold_fulfillment_policy eq 'holdgroup' ) >+ && ( !$library->validate_hold_sibling( { branchcode => $res->{branchcode} } ) ) ); >+ next >+ if ( ( $hold_fulfillment_policy eq 'homebranch' ) >+ && ( $res->{branchcode} ne $item->$hold_fulfillment_policy ) ); >+ next >+ if ( ( $hold_fulfillment_policy eq 'holdingbranch' ) >+ && ( $res->{branchcode} ne $item->$hold_fulfillment_policy ) ); > next unless $item->can_be_transferred( { to => Koha::Libraries->find( $res->{branchcode} ) } ); > $priority = $res->{'priority'}; > $highest = $res; >@@ -1010,7 +1052,7 @@ sub CheckReserves { > return ( "Reserved", $highest, \@reserves ); > } > >- return ( '' ); >+ return (''); > } > > =head2 CancelExpiredReserves >@@ -1023,35 +1065,35 @@ Cancels all reserves with an expiration date from before today. > > sub CancelExpiredReserves { > my $cancellation_reason = shift; >- my $today = dt_from_string(); >- my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); >- my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay'); >+ my $today = dt_from_string(); >+ my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); >+ my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay'); > >- my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+ my $dtf = Koha::Database->new->schema->storage->datetime_parser; > my $params = { > -or => [ >- { expirationdate => { '<', $dtf->format_date($today) } }, >+ { expirationdate => { '<', $dtf->format_date($today) } }, > { patron_expiration_date => { '<' => $dtf->format_date($today) } } > ] > }; > >- $params->{found} = [ { '!=', 'W' }, undef ] unless $expireWaiting; >+ $params->{found} = [ { '!=', 'W' }, undef ] unless $expireWaiting; > > # FIXME To move to Koha::Holds->search_expired (?) >- my $holds = Koha::Holds->search( $params ); >+ my $holds = Koha::Holds->search($params); > > while ( my $hold = $holds->next ) { > my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode ); > >- next if !$cancel_on_holidays && $calendar->is_holiday( $today ); >+ next if !$cancel_on_holidays && $calendar->is_holiday($today); > > my $cancel_params = {}; > $cancel_params->{cancellation_reason} = $cancellation_reason if defined($cancellation_reason); >- if ( defined($hold->found) && $hold->found eq 'W' ) { >+ if ( defined( $hold->found ) && $hold->found eq 'W' ) { > $cancel_params->{charge_cancel_fee} = 1; > } > $cancel_params->{autofill} = C4::Context->preference('ExpireReservesAutoFill'); >- $hold->cancel( $cancel_params ); >+ $hold->cancel($cancel_params); > } > } > >@@ -1111,27 +1153,28 @@ itemnumber and supplying itemnumber. > =cut > > sub ModReserve { >- my ( $params ) = @_; >- >- my $rank = $params->{'rank'}; >- my $reserve_id = $params->{'reserve_id'}; >- my $branchcode = $params->{'branchcode'}; >- my $itemnumber = $params->{'itemnumber'}; >- my $suspend_until = $params->{'suspend_until'}; >- my $borrowernumber = $params->{'borrowernumber'}; >- my $biblionumber = $params->{'biblionumber'}; >+ my ($params) = @_; >+ >+ my $rank = $params->{'rank'}; >+ my $reserve_id = $params->{'reserve_id'}; >+ my $branchcode = $params->{'branchcode'}; >+ my $itemnumber = $params->{'itemnumber'}; >+ my $suspend_until = $params->{'suspend_until'}; >+ my $borrowernumber = $params->{'borrowernumber'}; >+ my $biblionumber = $params->{'biblionumber'}; > my $cancellation_reason = $params->{'cancellation_reason'}; >- my $date = $params->{expirationdate}; >+ my $date = $params->{expirationdate}; > > return if defined $rank && $rank eq "n"; > > return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) ); > > my $hold; >- unless ( $reserve_id ) { >- my $holds = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber }); >- return unless $holds->count; # FIXME Should raise an exception >- $hold = $holds->next; >+ unless ($reserve_id) { >+ my $holds = Koha::Holds->search( >+ { biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber } ); >+ return unless $holds->count; # FIXME Should raise an exception >+ $hold = $holds->next; > $reserve_id = $hold->reserve_id; > } > >@@ -1141,16 +1184,14 @@ sub ModReserve { > Koha::Exceptions::ObjectNotFound->throw( 'No hold with id ' . $reserve_id ) unless $hold; > > if ( $rank eq "del" ) { >- $hold->cancel({ cancellation_reason => $cancellation_reason }); >- } >- elsif ($hold->found && $hold->priority eq '0' && $date) { >+ $hold->cancel( { cancellation_reason => $cancellation_reason } ); >+ } elsif ( $hold->found && $hold->priority eq '0' && $date ) { > logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold ) > if C4::Context->preference('HoldsLog'); > > # The only column that can be updated for a found hold is the expiration date > $hold->expirationdate($date)->store(); >- } >- elsif ($rank =~ /^\d+/ and $rank > 0) { >+ } elsif ( $rank =~ /^\d+/ and $rank > 0 ) { > logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold ) > if C4::Context->preference('HoldsLog'); > >@@ -1161,26 +1202,27 @@ sub ModReserve { > found => undef, > waitingdate => undef > }; >- if (exists $params->{reservedate}) { >+ if ( exists $params->{reservedate} ) { > $properties->{reservedate} = $params->{reservedate} || undef; > } >- if (exists $params->{expirationdate}) { >+ if ( exists $params->{expirationdate} ) { > $properties->{expirationdate} = $params->{expirationdate} || undef; > } > > $hold->set($properties)->store(); > >- if ( defined( $suspend_until ) ) { >- if ( $suspend_until ) { >- $hold->suspend_hold( $suspend_until ); >+ if ( defined($suspend_until) ) { >+ if ($suspend_until) { >+ $hold->suspend_hold($suspend_until); > } else { >+ > # If the hold is suspended leave the hold suspended, but convert it to an indefinite hold. > # If the hold is not suspended, this does nothing. > $hold->set( { suspend_until => undef } )->store(); > } > } > >- _FixPriority({ reserve_id => $reserve_id, rank =>$rank }); >+ _FixPriority( { reserve_id => $reserve_id, rank => $rank } ); > } > } > >@@ -1199,18 +1241,21 @@ $newstatus is the new status. > sub ModReserveStatus { > > #first : check if we have a reservation for this item . >- my ($itemnumber, $newstatus) = @_; >+ my ( $itemnumber, $newstatus ) = @_; > my $dbh = C4::Context->dbh; > >- my $query = "UPDATE reserves SET found = ?, waitingdate = NOW() WHERE itemnumber = ? AND found IS NULL AND priority = 0"; >+ my $query = >+ "UPDATE reserves SET found = ?, waitingdate = NOW() WHERE itemnumber = ? AND found IS NULL AND priority = 0"; > my $sth_set = $dbh->prepare($query); > $sth_set->execute( $newstatus, $itemnumber ); > > my $item = Koha::Items->find($itemnumber); >- if ( $item->location && $item->location eq 'CART' >+ if ( $item->location >+ && $item->location eq 'CART' > && ( !$item->permanent_location || $item->permanent_location ne 'CART' ) >- && $newstatus ) { >- CartToShelf( $itemnumber ); >+ && $newstatus ) >+ { >+ CartToShelf($itemnumber); > } > } > >@@ -1245,10 +1290,13 @@ sub ModReserveAffect { > # waiting in order to not send duplicate hold filled notifications > > my $hold; >+ > # Find hold by id if we have it >- $hold = Koha::Holds->find( $reserve_id ) if $reserve_id; >+ $hold = Koha::Holds->find($reserve_id) if $reserve_id; >+ > # Find item level hold for this item if there is one > $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->next(); >+ > # Find record level hold if there is no item level hold > $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->next(); > >@@ -1260,28 +1308,33 @@ sub ModReserveAffect { > > if ($transferToDo) { > $hold->set_transfer(); >- } elsif (C4::Context->preference('HoldsNeedProcessingSIP') >- && C4::Context->interface eq 'sip' >- && !$already_on_shelf) { >+ } elsif ( C4::Context->preference('HoldsNeedProcessingSIP') >+ && C4::Context->interface eq 'sip' >+ && !$already_on_shelf ) >+ { > $hold->set_processing(); > } else { > $hold->set_waiting($desk_id); > _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf; >+ > # Complete transfer if one exists > my $transfer = $hold->item->get_transfer; > $transfer->receive if $transfer; > } > >- _koha_notify_hold_changed( $hold ) if $notify_library; >+ _koha_notify_hold_changed($hold) if $notify_library; > > _FixPriority( { biblionumber => $biblionumber } ); > my $item = Koha::Items->find($itemnumber); >- if ( $item->location && $item->location eq 'CART' >- && ( !$item->permanent_location || $item->permanent_location ne 'CART' ) ) { >- CartToShelf( $itemnumber ); >+ if ( $item->location >+ && $item->location eq 'CART' >+ && ( !$item->permanent_location || $item->permanent_location ne 'CART' ) ) >+ { >+ CartToShelf($itemnumber); > } > >- my $std = $dbh->prepare(q{ >+ my $std = $dbh->prepare( >+ q{ > DELETE q, t > FROM tmp_holdsqueue q > INNER JOIN hold_fill_targets t >@@ -1291,8 +1344,9 @@ sub ModReserveAffect { > AND q.item_level_request = t.item_level_request > AND q.holdingbranch = t.source_branchcode > WHERE t.reserve_id = ? >- }); >- $std->execute($hold->reserve_id); >+ } >+ ); >+ $std->execute( $hold->reserve_id ); > > logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold ) > if C4::Context->preference('HoldsLog'); >@@ -1314,9 +1368,9 @@ sub ModReserveCancelAll { > my ( $itemnumber, $borrowernumber, $cancellation_reason ) = @_; > > #step 1 : cancel the reservation >- my $holds = Koha::Holds->search({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); >+ my $holds = Koha::Holds->search( { itemnumber => $itemnumber, borrowernumber => $borrowernumber } ); > return unless $holds->count; >- $holds->next->cancel({ cancellation_reason => $cancellation_reason }); >+ $holds->next->cancel( { cancellation_reason => $cancellation_reason } ); > > #step 2 launch the subroutine of the others reserves > ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); >@@ -1344,8 +1398,9 @@ sub ModReserveMinusPriority { > "; > my $sth_upd = $dbh->prepare($query); > $sth_upd->execute( $itemnumber, $reserve_id ); >+ > # second step update all others reserves >- _FixPriority({ reserve_id => $reserve_id, rank => '0' }); >+ _FixPriority( { reserve_id => $reserve_id, rank => '0' } ); > } > > =head2 IsAvailableForItemLevelRequest >@@ -1378,26 +1433,28 @@ checks with CanItemBeReserved or CanBookBeReserved. > =cut > > sub IsAvailableForItemLevelRequest { >- my $item = shift; >- my $patron = shift; >- my $pickup_branchcode = shift; >+ my $item = shift; >+ my $patron = shift; >+ my $pickup_branchcode = shift; > > my $dbh = C4::Context->dbh; >+ > # must check the notforloan setting of the itemtype > # FIXME - a lot of places in the code do this > # or something similar - need to be > # consolidated > my $itemtype = $item->effective_itemtype; > return 0 >- unless defined $itemtype; >+ unless defined $itemtype; > my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan; > >- return 0 if >- $notforloan_per_itemtype || >- $item->itemlost || >- $item->notforloan > 0 || # item with negative or zero notforloan value is holdable >- $item->withdrawn || >- ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems')); >+ return 0 >+ if $notforloan_per_itemtype >+ || $item->itemlost >+ || $item->notforloan > 0 >+ || # item with negative or zero notforloan value is holdable >+ $item->withdrawn >+ || ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); > > if ($pickup_branchcode) { > my $destination = Koha::Libraries->find($pickup_branchcode); >@@ -1405,10 +1462,11 @@ sub IsAvailableForItemLevelRequest { > return 0 unless $destination->pickup_location; > return 0 unless $item->can_be_transferred( { to => $destination } ); > my $reserves_control_branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); >- my $branchitemrule = >- C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); >- my $home_library = Koha::Libraries->find( {branchcode => $item->homebranch} ); >- return 0 unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' || $home_library->validate_hold_sibling( {branchcode => $pickup_branchcode} ); >+ my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); >+ my $home_library = Koha::Libraries->find( { branchcode => $item->homebranch } ); >+ return 0 >+ unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' >+ || $home_library->validate_hold_sibling( { branchcode => $pickup_branchcode } ); > } > > my $on_shelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); >@@ -1454,22 +1512,22 @@ sub ItemsAnyAvailableAndNotRestricted { > > foreach my $i (@items) { > my $reserves_control_branch = Koha::Policy::Holds->holds_control_library( $i, $param->{patron} ); >- my $branchitemrule = >- C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); >- my $item_library = Koha::Libraries->find( { branchcode => $i->homebranch } ); >+ my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); >+ my $item_library = Koha::Libraries->find( { branchcode => $i->homebranch } ); > > # we can return (end the loop) when first one found: > return 1 > unless $i->itemlost >- || $i->notforloan # items with non-zero notforloan cannot be checked out >+ || $i->notforloan # items with non-zero notforloan cannot be checked out > || $i->withdrawn > || $i->onloan > || IsItemOnHoldAndFound( $i->id ) > || ( $i->damaged >- && ! C4::Context->preference('AllowHoldsOnDamagedItems') ) >+ && !C4::Context->preference('AllowHoldsOnDamagedItems') ) > || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan > || $branchitemrule->{holdallowed} eq 'from_home_library' && $param->{patron}->branchcode ne $i->homebranch >- || $branchitemrule->{holdallowed} eq 'from_local_hold_group' && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } ) >+ || $branchitemrule->{holdallowed} eq 'from_local_hold_group' >+ && !$item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } ) > || CanItemBeReserved( $param->{patron}, $i )->{status} ne 'OK'; > } > >@@ -1488,31 +1546,30 @@ Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was > sub AlterPriority { > my ( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ) = @_; > >- my $hold = Koha::Holds->find( $reserve_id ); >+ my $hold = Koha::Holds->find($reserve_id); > return unless $hold; > > if ( $hold->cancellationdate ) { >- warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (" . $hold->cancellationdate . ')'; >+ warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (" >+ . $hold->cancellationdate . ')'; > return; > } > > if ( $where eq 'up' ) { >- return unless $prev_priority; >- _FixPriority({ reserve_id => $reserve_id, rank => $prev_priority }) >+ return unless $prev_priority; >+ _FixPriority( { reserve_id => $reserve_id, rank => $prev_priority } ); > } elsif ( $where eq 'down' ) { >- return unless $next_priority; >- _FixPriority({ reserve_id => $reserve_id, rank => $next_priority }) >+ return unless $next_priority; >+ _FixPriority( { reserve_id => $reserve_id, rank => $next_priority } ); > } elsif ( $where eq 'top' ) { >- _FixPriority({ reserve_id => $reserve_id, rank => $first_priority }) >+ _FixPriority( { reserve_id => $reserve_id, rank => $first_priority } ); > } elsif ( $where eq 'bottom' ) { >- _FixPriority({ reserve_id => $reserve_id, rank => $last_priority }); >+ _FixPriority( { reserve_id => $reserve_id, rank => $last_priority } ); > } > >- Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( >- { >- biblio_ids => [ $hold->biblionumber ] >- } >- ) if C4::Context->preference('RealTimeHoldsQueue'); >+ Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [ $hold->biblionumber ] } ) >+ if C4::Context->preference('RealTimeHoldsQueue'); >+ > # FIXME Should return the new priority > } > >@@ -1525,14 +1582,14 @@ This function sets the lowestPriority field to true if is false, and false if it > =cut > > sub ToggleLowestPriority { >- my ( $reserve_id ) = @_; >+ my ($reserve_id) = @_; > > my $dbh = C4::Context->dbh; > >- my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); >- $sth->execute( $reserve_id ); >+ my $sth = $dbh->prepare("UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); >+ $sth->execute($reserve_id); > >- _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); >+ _FixPriority( { reserve_id => $reserve_id, rank => '999999' } ); > } > > =head2 ToggleSuspend >@@ -1548,12 +1605,12 @@ be cleared when it is unsuspended. > sub ToggleSuspend { > my ( $reserve_id, $suspend_until ) = @_; > >- my $hold = Koha::Holds->find( $reserve_id ); >+ my $hold = Koha::Holds->find($reserve_id); > > if ( $hold->is_suspended ) { >- $hold->resume() >+ $hold->resume(); > } else { >- $hold->suspend_hold( $suspend_until ); >+ $hold->suspend_hold($suspend_until); > } > } > >@@ -1579,26 +1636,24 @@ sub SuspendAll { > my $borrowernumber = $params{'borrowernumber'} || undef; > my $biblionumber = $params{'biblionumber'} || undef; > my $suspend_until = $params{'suspend_until'} || undef; >- my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1; >+ my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1; > > return unless ( $borrowernumber || $biblionumber ); > > my $params; > $params->{found} = undef; > $params->{borrowernumber} = $borrowernumber if $borrowernumber; >- $params->{biblionumber} = $biblionumber if $biblionumber; >+ $params->{biblionumber} = $biblionumber if $biblionumber; > > my @holds = Koha::Holds->search($params)->as_list; > > if ($suspend) { > map { $_->suspend_hold($suspend_until) } @holds; >- } >- else { >+ } else { > map { $_->resume() } @holds; > } > } > >- > =head2 _FixPriority > > _FixPriority({ >@@ -1635,32 +1690,32 @@ when _FixPriority calls itself. > =cut > > sub _FixPriority { >- my ( $params ) = @_; >- my $reserve_id = $params->{reserve_id}; >- my $rank = $params->{rank} // ''; >+ my ($params) = @_; >+ my $reserve_id = $params->{reserve_id}; >+ my $rank = $params->{rank} // ''; > my $ignoreSetLowestRank = $params->{ignoreSetLowestRank}; >- my $biblionumber = $params->{biblionumber}; >+ my $biblionumber = $params->{biblionumber}; > > my $dbh = C4::Context->dbh; > > my $hold; >- if ( $reserve_id ) { >- $hold = Koha::Holds->find( $reserve_id ); >- if (!defined $hold){ >+ if ($reserve_id) { >+ $hold = Koha::Holds->find($reserve_id); >+ if ( !defined $hold ) { >+ > # may have already been checked out and hold fulfilled >- $hold = Koha::Old::Holds->find( $reserve_id ); >+ $hold = Koha::Old::Holds->find($reserve_id); > } > return unless $hold; > } > >- unless ( $biblionumber ) { # FIXME This is a very weird API >+ unless ($biblionumber) { # FIXME This is a very weird API > $biblionumber = $hold->biblionumber; > } > >- if ( $rank eq "del" ) { # FIXME will crash if called without $hold >+ if ( $rank eq "del" ) { # FIXME will crash if called without $hold > $hold->cancel; >- } >- elsif ( $reserve_id && ( $rank eq "W" || $rank eq "0" ) ) { >+ } elsif ( $reserve_id && ( $rank eq "W" || $rank eq "0" ) ) { > > # make sure priority for waiting or in-transit items is 0 > my $query = " >@@ -1670,7 +1725,7 @@ sub _FixPriority { > AND found IN ('W', 'T', 'P') > "; > my $sth = $dbh->prepare($query); >- $sth->execute( $reserve_id ); >+ $sth->execute($reserve_id); > } > my @priority; > >@@ -1683,9 +1738,9 @@ sub _FixPriority { > ORDER BY priority ASC > "; > my $sth = $dbh->prepare($query); >- $sth->execute( $biblionumber ); >+ $sth->execute($biblionumber); > while ( my $line = $sth->fetchrow_hashref ) { >- push( @priority, $line ); >+ push( @priority, $line ); > } > > # FIXME This whole sub must be rewritten, especially to highlight what is done when reserve_id is not given >@@ -1701,7 +1756,7 @@ sub _FixPriority { > > # if index exists in array then move it to new position > if ( $key > -1 && $rank ne 'del' && $rank > 0 ) { >- my $new_rank = $rank - 1; # $new_rank is what you want the new index to be in the array >+ my $new_rank = $rank - 1; # $new_rank is what you want the new index to be in the array > my $moving_item = splice( @priority, $key, 1 ); > $new_rank = scalar @priority if $new_rank > scalar @priority; > splice( @priority, $new_rank, 0, $moving_item ); >@@ -1721,16 +1776,19 @@ sub _FixPriority { > ); > } > >- unless ( $ignoreSetLowestRank ) { >- $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 AND biblionumber = ? ORDER BY priority" ); >+ unless ($ignoreSetLowestRank) { >+ $sth = $dbh->prepare( >+ "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 AND biblionumber = ? ORDER BY priority"); > $sth->execute($biblionumber); >- while ( my $res = $sth->fetchrow_hashref() ) { >- _FixPriority({ >- reserve_id => $res->{'reserve_id'}, >- rank => '999999', >- ignoreSetLowestRank => 1 >- }); >- } >+ while ( my $res = $sth->fetchrow_hashref() ) { >+ _FixPriority( >+ { >+ reserve_id => $res->{'reserve_id'}, >+ rank => '999999', >+ ignoreSetLowestRank => 1 >+ } >+ ); >+ } > } > } > >@@ -1757,8 +1815,8 @@ All return values will respect any borrowernumbers passed as arrayref in $ignore > =cut > > sub _Findgroupreserve { >- my ( $biblionumber, $itemnumber, $lookahead, $ignore_borrowers) = @_; >- my $dbh = C4::Context->dbh; >+ my ( $biblionumber, $itemnumber, $lookahead, $ignore_borrowers ) = @_; >+ my $dbh = C4::Context->dbh; > > # check for targeted match form the holds queue > my $hold_target_query = qq{ >@@ -1788,11 +1846,11 @@ sub _Findgroupreserve { > ORDER BY priority > }; > my $sth = $dbh->prepare($hold_target_query); >- $sth->execute($itemnumber, $lookahead||0); >+ $sth->execute( $itemnumber, $lookahead || 0 ); > my @results; > if ( my $data = $sth->fetchrow_hashref ) { > push( @results, $data ) >- unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; >+ unless any { $data->{borrowernumber} eq $_ } @$ignore_borrowers; > } > return @results if @results; > >@@ -1820,11 +1878,11 @@ sub _Findgroupreserve { > ORDER BY priority > }; > $sth = $dbh->prepare($query); >- $sth->execute( $biblionumber, $itemnumber, $lookahead||0); >+ $sth->execute( $biblionumber, $itemnumber, $lookahead || 0 ); > @results = (); > while ( my $data = $sth->fetchrow_hashref ) { > push( @results, $data ) >- unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; >+ unless any { $data->{borrowernumber} eq $_ } @$ignore_borrowers; > } > return @results; > } >@@ -1860,38 +1918,41 @@ The following tables are availalbe witin the notice: > sub _koha_notify_reserve { > my $reserve_id = shift; > >- my $hold = Koha::Holds->find($reserve_id); >+ my $hold = Koha::Holds->find($reserve_id); > my $borrowernumber = $hold->borrowernumber; > >- my $patron = Koha::Patrons->find( $borrowernumber ); >+ my $patron = Koha::Patrons->find($borrowernumber); > > # Try to get the borrower's email address > my $to_address = $patron->notice_email_address; > >- my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { >+ my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( >+ { > borrowernumber => $borrowernumber, >- message_name => 'Hold_Filled' >- } ); >+ message_name => 'Hold_Filled' >+ } >+ ); > >- my $library = Koha::Libraries->find( $hold->branchcode ); >+ my $library = Koha::Libraries->find( $hold->branchcode ); > my $from_email_address = $library->from_email_address; > > my %letter_params = ( >- module => 'reserves', >+ module => 'reserves', > branchcode => $hold->branchcode, >- lang => $patron->lang, >- tables => { >- 'branches' => $library->unblessed, >- 'borrowers' => $patron->unblessed, >- 'biblio' => $hold->biblionumber, >- 'biblioitems' => $hold->biblionumber, >- 'reserves' => $hold->unblessed, >- 'items' => $hold->itemnumber, >+ lang => $patron->lang, >+ tables => { >+ 'branches' => $library->unblessed, >+ 'borrowers' => $patron->unblessed, >+ 'biblio' => $hold->biblionumber, >+ 'biblioitems' => $hold->biblionumber, >+ 'reserves' => $hold->unblessed, >+ 'items' => $hold->itemnumber, > }, > ); > >- my $notification_sent = 0; #Keeping track if a Hold_filled message is sent. If no message can be sent, then default to a print message. >- my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING}; >+ my $notification_sent = >+ 0; #Keeping track if a Hold_filled message is sent. If no message can be sent, then default to a print message. >+ my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING}; > my $send_notification = sub { > my ( $mtt, $letter_code, $wants_digest ) = (@_); > return unless defined $letter_code; >@@ -1928,19 +1989,23 @@ sub _koha_notify_reserve { > }; > > while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) { >- next if ( >- ( $mtt eq 'email' and not $to_address ) # No email address >- or ( $mtt eq 'sms' and not $patron->smsalertnumber ) # No SMS number >- or ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl >- or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call >- ); >+ next >+ if ( >+ ( $mtt eq 'email' and not $to_address ) # No email address >+ or ( $mtt eq 'sms' and not $patron->smsalertnumber ) # No SMS number >+ or ( $mtt eq 'itiva' >+ and C4::Context->preference('TalkingTechItivaPhoneNotification') >+ ) # Notice is handled by TalkingTech_itiva_outbound.pl >+ or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call >+ ); > >- &$send_notification($mtt, $letter_code, $messagingprefs->{wants_digest}); >+ &$send_notification( $mtt, $letter_code, $messagingprefs->{wants_digest} ); > $notification_sent++; > } >+ > #Making sure that a print notification is sent if no other transport types can be utilized. >- if (! $notification_sent) { >- &$send_notification('print', 'HOLD'); >+ if ( !$notification_sent ) { >+ &$send_notification( 'print', 'HOLD' ); > } > > } >@@ -1954,14 +2019,14 @@ sub _koha_notify_reserve { > sub _koha_notify_hold_changed { > my $hold = shift; > >- my $patron = $hold->patron; >+ my $patron = $hold->patron; > my $library = $hold->branch; > > my $letter = C4::Letters::GetPreparedLetter( > module => 'reserves', > letter_code => 'HOLD_CHANGED', > branchcode => $hold->branchcode, >- substitute => { today => output_pref( dt_from_string ) }, >+ substitute => { today => output_pref(dt_from_string) }, > tables => { > 'branches' => $library->unblessed, > 'borrowers' => $patron->unblessed, >@@ -1974,9 +2039,8 @@ sub _koha_notify_hold_changed { > > return unless $letter; > >- my $email = >- C4::Context->preference('ExpireReservesAutoFillEmail') >- || $library->inbound_email_address; >+ my $email = C4::Context->preference('ExpireReservesAutoFillEmail') >+ || $library->inbound_email_address; > > C4::Letters::EnqueueLetter( > { >@@ -2010,13 +2074,14 @@ the sub accounts for that too. > sub _ShiftPriority { > my ( $biblio, $new_priority ) = @_; > >- my $dbh = C4::Context->dbh; >+ my $dbh = C4::Context->dbh; > my $query = "SELECT priority FROM reserves WHERE biblionumber = ? AND priority > ? ORDER BY priority ASC LIMIT 1"; >- my $sth = $dbh->prepare( $query ); >+ my $sth = $dbh->prepare($query); > $sth->execute( $biblio, $new_priority ); > my $min_priority = $sth->fetchrow; >+ > # if no such matches are found, $new_priority remains as original value >- $new_priority = $min_priority if ( $min_priority ); >+ $new_priority = $min_priority if ($min_priority); > > # Shift the priority up by one; works in conjunction with the next SQL statement > $query = "UPDATE reserves >@@ -2025,17 +2090,18 @@ sub _ShiftPriority { > AND borrowernumber = ? > AND reservedate = ? > AND found IS NULL"; >- my $sth_update = $dbh->prepare( $query ); >+ my $sth_update = $dbh->prepare($query); > > # Select all reserves for the biblio with priority greater than $new_priority, and order greatest to least >- $query = "SELECT borrowernumber, reservedate FROM reserves WHERE priority >= ? AND biblionumber = ? ORDER BY priority DESC"; >- $sth = $dbh->prepare( $query ); >+ $query = >+ "SELECT borrowernumber, reservedate FROM reserves WHERE priority >= ? AND biblionumber = ? ORDER BY priority DESC"; >+ $sth = $dbh->prepare($query); > $sth->execute( $new_priority, $biblio ); > while ( my $row = $sth->fetchrow_hashref ) { > $sth_update->execute( $biblio, $row->{borrowernumber}, $row->{reservedate} ); > } > >- return $new_priority; # so the caller knows what priority they wind up receiving >+ return $new_priority; # so the caller knows what priority they wind up receiving > } > > =head2 MoveReserve >@@ -2052,38 +2118,39 @@ sub MoveReserve { > > $cancelreserve //= 0; > >- my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds >- my $item = Koha::Items->find($itemnumber); >+ my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds >+ my $item = Koha::Items->find($itemnumber); > my ( $restype, $res, undef ) = CheckReserves( $item, $lookahead ); > return unless $res; > > my $biblionumber = $res->{biblionumber}; > >- if ($res->{borrowernumber} == $borrowernumber) { >+ if ( $res->{borrowernumber} == $borrowernumber ) { > my $hold = Koha::Holds->find( $res->{reserve_id} ); >- $hold->fill({ item_id => $itemnumber }); >- } >- else { >+ $hold->fill( { item_id => $itemnumber } ); >+ } else { >+ > # warn "Reserved"; > # The item is reserved by someone else. > # Find this item in the reserves > >- my $borr_res = Koha::Holds->search({ >- borrowernumber => $borrowernumber, >- biblionumber => $biblionumber, >- },{ >- order_by => 'priority' >- })->next(); >+ my $borr_res = Koha::Holds->search( >+ { >+ borrowernumber => $borrowernumber, >+ biblionumber => $biblionumber, >+ }, >+ { order_by => 'priority' } >+ )->next(); >+ >+ if ($borr_res) { > >- if ( $borr_res ) { > # The item is reserved by the current patron >- $borr_res->fill({ item_id => $itemnumber }); >+ $borr_res->fill( { item_id => $itemnumber } ); > } > >- if ( $cancelreserve eq 'revert' ) { ## Revert waiting reserve to priority 1 >- RevertWaitingStatus({ itemnumber => $itemnumber }); >- } >- elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item >+ if ( $cancelreserve eq 'revert' ) { ## Revert waiting reserve to priority 1 >+ RevertWaitingStatus( { itemnumber => $itemnumber } ); >+ } elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item > my $hold = Koha::Holds->find( $res->{reserve_id} ); > $hold->cancel; > } >@@ -2100,28 +2167,25 @@ This shifts the holds from C<$from_biblio> to C<$to_biblio> and reorders them by > > sub MergeHolds { > my ( $dbh, $to_biblio, $from_biblio ) = @_; >- my $sth = $dbh->prepare( >- "SELECT count(*) as reserve_count FROM reserves WHERE biblionumber = ?" >- ); >+ my $sth = $dbh->prepare("SELECT count(*) as reserve_count FROM reserves WHERE biblionumber = ?"); > $sth->execute($from_biblio); > if ( my $data = $sth->fetchrow_hashref() ) { > > # holds exist on old record, if not we don't need to do anything >- $sth = $dbh->prepare( >- "UPDATE reserves SET biblionumber = ? WHERE biblionumber = ?"); >+ $sth = $dbh->prepare("UPDATE reserves SET biblionumber = ? WHERE biblionumber = ?"); > $sth->execute( $to_biblio, $from_biblio ); > > # Reorder by date > # don't reorder those already waiting > > $sth = $dbh->prepare( >-"SELECT * FROM reserves WHERE biblionumber = ? AND (found NOT IN ('W', 'T', 'P') OR found is NULL) ORDER BY reservedate ASC" >+ "SELECT * FROM reserves WHERE biblionumber = ? AND (found NOT IN ('W', 'T', 'P') OR found is NULL) ORDER BY reservedate ASC" > ); > my $upd_sth = $dbh->prepare( >-"UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ? >+ "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ? > AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) " > ); >- $sth->execute( $to_biblio ); >+ $sth->execute($to_biblio); > my $priority = 1; > while ( my $reserve = $sth->fetchrow_hashref() ) { > $upd_sth->execute( >@@ -2149,10 +2213,10 @@ sub MergeHolds { > =cut > > sub RevertWaitingStatus { >- my ( $params ) = @_; >+ my ($params) = @_; > my $itemnumber = $params->{'itemnumber'}; > >- return unless ( $itemnumber ); >+ return unless ($itemnumber); > > my $dbh = C4::Context->dbh; > >@@ -2160,23 +2224,23 @@ sub RevertWaitingStatus { > my $hold = Koha::Holds->search( > { > itemnumber => $itemnumber, >- found => { not => undef }, >+ found => { not => undef }, > } > )->next; > > ## Increment the priority of all other non-waiting > ## reserves for this bib record >- my $holds = Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } }) >- ->update({ priority => \'priority + 1' }, { no_triggers => 1 }); >+ my $holds = Koha::Holds->search( { biblionumber => $hold->biblionumber, priority => { '>' => 0 } } ) >+ ->update( { priority => \'priority + 1' }, { no_triggers => 1 } ); > > ## Fix up the currently waiting reserve > $hold->set( > { >- priority => 1, >- found => undef, >- waitingdate => undef, >+ priority => 1, >+ found => undef, >+ waitingdate => undef, > expirationdate => $hold->patron_expiration_date, >- itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, >+ itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, > } > )->store(); > >@@ -2185,12 +2249,8 @@ sub RevertWaitingStatus { > > _FixPriority( { biblionumber => $hold->biblionumber } ); > >- Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( >- { >- biblio_ids => [ $hold->biblionumber ] >- } >- ) if C4::Context->preference('RealTimeHoldsQueue'); >- >+ Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [ $hold->biblionumber ] } ) >+ if C4::Context->preference('RealTimeHoldsQueue'); > > return $hold; > } >@@ -2222,23 +2282,23 @@ available within the slip: > =cut > > sub ReserveSlip { >- my ($args) = @_; >- my $branchcode = $args->{branchcode}; >+ my ($args) = @_; >+ my $branchcode = $args->{branchcode}; > my $reserve_id = $args->{reserve_id}; > my $itemnumber = $args->{itemnumber}; > > my $hold = Koha::Holds->find($reserve_id); > return unless $hold; > >- my $patron = $hold->borrower; >+ my $patron = $hold->borrower; > my $reserve = $hold->unblessed; > >- return C4::Letters::GetPreparedLetter ( >- module => 'circulation', >+ return C4::Letters::GetPreparedLetter( >+ module => 'circulation', > letter_code => 'HOLD_SLIP', >- branchcode => $branchcode, >- lang => $patron->lang, >- tables => { >+ branchcode => $branchcode, >+ lang => $patron->lang, >+ tables => { > 'reserves' => $reserve, > 'branches' => $reserve->{branchcode}, > 'borrowers' => $reserve->{borrowernumber}, >@@ -2277,21 +2337,21 @@ sub CalculatePriority { > AND priority > 0 > AND (found IS NULL OR found = '') > }; >+ > #skip found==W or found==T or found==P (waiting, transit or processing holds) >- if( $resdate ) { >- $sql.= ' AND ( reservedate <= ? )'; >- } >- else { >- $sql.= ' AND ( reservedate < NOW() )'; >+ if ($resdate) { >+ $sql .= ' AND ( reservedate <= ? )'; >+ } else { >+ $sql .= ' AND ( reservedate < NOW() )'; > } > my $dbh = C4::Context->dbh(); > my @row = $dbh->selectrow_array( > $sql, > undef, >- $resdate ? ($biblionumber, $resdate) : ($biblionumber) >+ $resdate ? ( $biblionumber, $resdate ) : ($biblionumber) > ); > >- return @row ? $row[0]+1 : 1; >+ return @row ? $row[0] + 1 : 1; > } > > =head2 IsItemOnHoldAndFound >@@ -2334,7 +2394,7 @@ sub GetMaxPatronHoldsForRecord { > my ( $borrowernumber, $biblionumber ) = @_; > > my $patron = Koha::Patrons->find($borrowernumber); >- my @items = Koha::Items->search( { biblionumber => $biblionumber } )->as_list; >+ my @items = Koha::Items->search( { biblionumber => $biblionumber } )->as_list; > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > >@@ -2348,12 +2408,14 @@ sub GetMaxPatronHoldsForRecord { > > $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" ); > >- my $rule = Koha::CirculationRules->get_effective_rule({ >- categorycode => $categorycode, >- itemtype => $itemtype, >- branchcode => $branchcode, >- rule_name => 'holds_per_record' >- }); >+ my $rule = Koha::CirculationRules->get_effective_rule( >+ { >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ branchcode => $branchcode, >+ rule_name => 'holds_per_record' >+ } >+ ); > my $holds_per_record = $rule ? $rule->rule_value : 0; > $max = $holds_per_record if $holds_per_record > $max; > } >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index f340a1441ae..4423b83df8e 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -1,6 +1,5 @@ > #!/usr/bin/perl > >- > # Copyright Katipo Communications 2002 > # Copyright Koha Development team 2012 > # >@@ -21,13 +20,13 @@ > > use Modern::Perl; > >-use CGI qw ( -utf8 ); >-use C4::Auth qw( get_template_and_user ); >-use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); >+use CGI qw ( -utf8 ); >+use C4::Auth qw( get_template_and_user ); >+use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); > use C4::Circulation qw( GetBranchItemRule ); >-use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ); >-use C4::Biblio qw( GetBiblioData GetFrameworkCode ); >-use C4::Output qw( output_html_with_http_headers ); >+use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ); >+use C4::Biblio qw( GetBiblioData GetFrameworkCode ); >+use C4::Output qw( output_html_with_http_headers ); > use C4::Context; > use C4::Members; > use C4::Overdues; >@@ -48,26 +47,26 @@ my $query = CGI->new; > my $op = $query->param('op') // q{}; > > # if OPACHoldRequests (for placing holds) is disabled, leave immediately >-if ( ! C4::Context->preference('OPACHoldRequests') ) { >+if ( !C4::Context->preference('OPACHoldRequests') ) { > print $query->redirect("/cgi-bin/koha/errors/404.pl"); > exit; > } > > my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > { >- template_name => "opac-reserve.tt", >- query => $query, >- type => "opac", >+ template_name => "opac-reserve.tt", >+ query => $query, >+ type => "opac", > } > ); > >-my $patron = Koha::Patrons->find( $borrowernumber, { prefetch => ['categorycode'] } ); >+my $patron = Koha::Patrons->find( $borrowernumber, { prefetch => ['categorycode'] } ); > my $category = $patron->category; > > my $can_place_hold_if_available_at_pickup = C4::Context->preference('OPACHoldsIfAvailableAtPickup'); >-unless ( $can_place_hold_if_available_at_pickup ) { >+unless ($can_place_hold_if_available_at_pickup) { > my @patron_categories = split ',', C4::Context->preference('OPACHoldsIfAvailableAtPickupExceptions'); >- if ( @patron_categories ) { >+ if (@patron_categories) { > my $categorycode = $patron->categorycode; > $can_place_hold_if_available_at_pickup = grep { $_ eq $categorycode } @patron_categories; > } >@@ -78,12 +77,12 @@ my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_l > # There are two ways of calling this script, with a single biblio num > # or multiple biblio nums. > my $biblionumbers = $query->param('biblionumbers'); >-my $reserveMode = $query->param('reserve_mode'); >-if ($reserveMode && ($reserveMode eq 'single')) { >+my $reserveMode = $query->param('reserve_mode'); >+if ( $reserveMode && ( $reserveMode eq 'single' ) ) { > my $bib = $query->param('single_bib'); > $biblionumbers = "$bib/"; > } >-if (! $biblionumbers) { >+if ( !$biblionumbers ) { > $biblionumbers = $query->param('biblionumber'); > } > >@@ -100,8 +99,9 @@ $template->param( biblionumbers => $biblionumbers ); > # Each biblio number is suffixed with '/', e.g. "1/2/3/" > my @biblionumbers = split /\//, $biblionumbers; > if ( $#biblionumbers < 0 && $op ne 'cud-place_reserve' ) { >+ > # TODO: New message? >- $template->param(message=>1, no_biblionumber=>1); >+ $template->param( message => 1, no_biblionumber => 1 ); > output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; > exit; > } >@@ -111,25 +111,26 @@ if ( $#biblionumbers < 0 && $op ne 'cud-place_reserve' ) { > # Here we check that the borrower can actually make reserves Stage 1. > # > # >-my $noreserves = 0; >+my $noreserves = 0; > if ( $category->effective_BlockExpiredPatronOpacActions ) { > if ( $patron->is_expired ) { >+ > # cannot reserve, their card has expired and the rules set mean this is not allowed > $noreserves = 1; > $template->param( message => 1, expired_patron => 1 ); > } > } > >-my $maxoutstanding = C4::Context->preference("maxoutstanding"); >+my $maxoutstanding = C4::Context->preference("maxoutstanding"); > my $amountoutstanding = $patron->account->balance; >-if ( $amountoutstanding && ($amountoutstanding > $maxoutstanding) ) { >+if ( $amountoutstanding && ( $amountoutstanding > $maxoutstanding ) ) { > my $amount = sprintf "%.02f", $amountoutstanding; > $template->param( message => 1 ); > $noreserves = 1; > $template->param( too_much_oweing => $amount ); > } > >-if ( $patron->gonenoaddress && ($patron->gonenoaddress == 1) ) { >+if ( $patron->gonenoaddress && ( $patron->gonenoaddress == 1 ) ) { > $noreserves = 1; > $template->param( > message => 1, >@@ -137,7 +138,7 @@ if ( $patron->gonenoaddress && ($patron->gonenoaddress == 1) ) { > ); > } > >-if ( $patron->lost && ($patron->lost == 1) ) { >+if ( $patron->lost && ( $patron->lost == 1 ) ) { > $noreserves = 1; > $template->param( > message => 1, >@@ -155,7 +156,7 @@ if ( $patron->is_debarred ) { > ); > } > >-my $holds = $patron->holds; >+my $holds = $patron->holds; > my $reserves_count = $holds->count; > $template->param( RESERVES => $holds->unblessed ); > if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) { >@@ -164,14 +165,13 @@ if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) { > $template->param( too_many_reserves => $holds->count ); > } > >-if( $noreserves ){ >+if ($noreserves) { > output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; > exit; > } > >- > # pass the pickup branch along.... >-my $branch = $query->param('branch') || $patron->branchcode || C4::Context->userenv->{branch} || '' ; >+my $branch = $query->param('branch') || $patron->branchcode || C4::Context->userenv->{branch} || ''; > $template->param( branch => $branch ); > > # >@@ -189,12 +189,13 @@ if ( $op eq 'cud-place_reserve' ) { > # List is composed of alternating biblio/item/branch > my $selectedItems = $query->param('selecteditems'); > >- if ($query->param('reserve_mode') eq 'single') { >+ if ( $query->param('reserve_mode') eq 'single' ) { >+ > # This indicates non-JavaScript mode, so there was > # only a single biblio number selected. >- my $bib = $query->param('single_bib'); >+ my $bib = $query->param('single_bib'); > my $item = $query->param("checkitem_$bib"); >- if ($item eq 'any') { >+ if ( $item eq 'any' ) { > $item = ''; > } > my $branch = $query->param('branch'); >@@ -207,8 +208,8 @@ if ( $op eq 'cud-place_reserve' ) { > # Make sure there is a biblionum/itemnum/branch triplet for each item. > # The itemnum can be 'any', meaning next available. > my $selectionCount = @selectedItems; >- if (($selectionCount == 0) || (($selectionCount % 3) != 0)) { >- $template->param(message=>1, bad_data=>1); >+ if ( ( $selectionCount == 0 ) || ( ( $selectionCount % 3 ) != 0 ) ) { >+ $template->param( message => 1, bad_data => 1 ); > output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; > exit; > } >@@ -222,22 +223,23 @@ if ( $op eq 'cud-place_reserve' ) { > my $canreserve = 0; > > my $singleBranchMode = Koha::Libraries->search->count == 1; >- if ( $singleBranchMode || ! C4::Context->preference("OPACAllowUserToChooseBranch") ) >- { # single branch mode or disabled user choosing >+ if ( $singleBranchMode || !C4::Context->preference("OPACAllowUserToChooseBranch") ) >+ { # single branch mode or disabled user choosing > $branch = $patron->branchcode; > } > > # FIXME We shouldn't need to fetch the item here >- my $item = $itemNum ? Koha::Items->find( $itemNum ) : undef; >+ my $item = $itemNum ? Koha::Items->find($itemNum) : undef; >+ > # When choosing a specific item, the default pickup library should be dictated by the default hold policy >- if ( ! C4::Context->preference("OPACAllowUserToChooseBranch") && $item ) { >+ if ( !C4::Context->preference("OPACAllowUserToChooseBranch") && $item ) { > my $type = $item->effective_itemtype; > my $reserves_control_branch = Koha::Policy::Holds->holds_control_library( $item, $patron ); > my $rule = GetBranchItemRule( $reserves_control_branch, $type ); > > if ( $rule->{hold_fulfillment_policy} eq 'any' || $rule->{hold_fulfillment_policy} eq 'patrongroup' ) { > $branch = $patron->branchcode; >- } elsif ( $rule->{hold_fulfillment_policy} eq 'holdgroup' ){ >+ } elsif ( $rule->{hold_fulfillment_policy} eq 'holdgroup' ) { > $branch = $item->homebranch; > } else { > my $policy = $rule->{hold_fulfillment_policy}; >@@ -246,7 +248,7 @@ if ( $op eq 'cud-place_reserve' ) { > } > > # if we have an item, we are placing the hold on the item's bib, in case of analytics >- if ( $item ) { >+ if ($item) { > $biblioNum = $item->biblionumber; > } > >@@ -264,19 +266,18 @@ if ( $op eq 'cud-place_reserve' ) { > $itemtype = undef if $itemNum; > > my $biblio = Koha::Biblios->find($biblioNum); >- my $rank = $biblio->holds->search( { found => [ { "!=" => "W" }, undef ] } )->count + 1; >- if ( $item ) { >+ my $rank = $biblio->holds->search( { found => [ { "!=" => "W" }, undef ] } )->count + 1; >+ if ($item) { > my $status = CanItemBeReserved( $patron, $item, $branch, { get_from_cache => 1 } )->{status}; >- if( $status eq 'OK' ){ >+ if ( $status eq 'OK' ) { > $canreserve = 1; > } else { > push @failed_holds, $status; > } > >- } >- else { >+ } else { > my $status = CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $itemtype } )->{status}; >- if( $status eq 'OK'){ >+ if ( $status eq 'OK' ) { > $canreserve = 1; > } else { > push @failed_holds, $status; >@@ -285,7 +286,7 @@ if ( $op eq 'cud-place_reserve' ) { > # Inserts a null into the 'itemnumber' field of 'reserves' table. > $itemNum = undef; > } >- my $notes = $query->param('notes_'.$biblioNum)||''; >+ my $notes = $query->param( 'notes_' . $biblioNum ) || ''; > my $item_group_id = $query->param("item_group_id_$biblioNum") || undef; > > if ( $maxreserves >@@ -295,10 +296,13 @@ if ( $op eq 'cud-place_reserve' ) { > $canreserve = 0; > } > >- unless ( $can_place_hold_if_available_at_pickup ) { >- my $items_in_this_library = Koha::Items->search({ biblionumber => $biblioNum, holdingbranch => $branch }); >- my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count; >- my $nb_of_items_unavailable = $items_in_this_library->search({ -or => { lost => { '!=' => 0 }, damaged => { '!=' => 0 }, } }); >+ unless ($can_place_hold_if_available_at_pickup) { >+ my $items_in_this_library = Koha::Items->search( { biblionumber => $biblioNum, holdingbranch => $branch } ); >+ my $nb_of_items_issued = >+ $items_in_this_library->search( { 'issue.itemnumber' => { not => undef } }, { join => 'issue' } ) >+ ->count; >+ my $nb_of_items_unavailable = >+ $items_in_this_library->search( { -or => { lost => { '!=' => 0 }, damaged => { '!=' => 0 }, } } ); > if ( $items_in_this_library->count > $nb_of_items_issued + $nb_of_items_unavailable ) { > $canreserve = 0; > push @failed_holds, 'items_available'; >@@ -323,7 +327,7 @@ if ( $op eq 'cud-place_reserve' ) { > item_group_id => $item_group_id, > } > ); >- if( $reserve_id ){ >+ if ($reserve_id) { > ++$reserve_cnt; > } else { > push @failed_holds, 'not_placed'; >@@ -331,7 +335,9 @@ if ( $op eq 'cud-place_reserve' ) { > } > } > >- print $query->redirect("/cgi-bin/koha/opac-user.pl?" . ( @failed_holds ? "failed_holds=" . join('|',@failed_holds) : q|| ) . "&opac-user-holds=1"); >+ print $query->redirect( "/cgi-bin/koha/opac-user.pl?" >+ . ( @failed_holds ? "failed_holds=" . join( '|', @failed_holds ) : q|| ) >+ . "&opac-user-holds=1" ); > exit; > } > >@@ -341,45 +347,39 @@ if ( $op eq 'cud-place_reserve' ) { > # > # > >-my %biblioDataHash; # Hash of biblionumber to biblio/biblioitems record. >+my %biblioDataHash; # Hash of biblionumber to biblio/biblioitems record. > foreach my $biblioNumber (@biblionumbers) { > > my $biblioData = GetBiblioData($biblioNumber); > $biblioDataHash{$biblioNumber} = $biblioData; > >- my $biblio = Koha::Biblios->find( $biblioNumber ); >+ my $biblio = Koha::Biblios->find($biblioNumber); > next unless $biblio; > > my $items = Koha::Items->search_ordered( > [ > 'me.biblionumber' => $biblioNumber, >- 'me.itemnumber' => { >- -in => [ >- $biblio->host_items->get_column('itemnumber') >- ] >- } >+ 'me.itemnumber' => { -in => [ $biblio->host_items->get_column('itemnumber') ] } > ], > { prefetch => [ 'issue', 'homebranch', 'holdingbranch' ] } >- )->filter_by_visible_in_opac({ patron => $patron }); >+ )->filter_by_visible_in_opac( { patron => $patron } ); > >- $biblioData->{items} = [$items->as_list]; # FIXME Potentially a lot in memory here! >+ $biblioData->{items} = [ $items->as_list ]; # FIXME Potentially a lot in memory here! > > # Compute the priority rank. > $biblioData->{object} = $biblio; >- my $reservecount = $biblio->holds->search({ found => [ {"!=" => "W"},undef] })->count; >+ my $reservecount = $biblio->holds->search( { found => [ { "!=" => "W" }, undef ] } )->count; > $biblioData->{reservecount} = $reservecount; >- $biblioData->{rank} = $reservecount + 1; >+ $biblioData->{rank} = $reservecount + 1; > } > >- >-my $requested_reserves_count = scalar( @biblionumbers ); >+my $requested_reserves_count = scalar(@biblionumbers); > if ( $maxreserves && ( $reserves_count + $requested_reserves_count > $maxreserves ) ) { > $template->param( new_reserves_allowed => $maxreserves - $reserves_count ); > } > > $template->param( select_item_types => 1 ); > >- > # > # > # Build the template parameters that will show the info >@@ -387,14 +387,14 @@ $template->param( select_item_types => 1 ); > # > # > >-my $biblioLoop = []; >-my $numBibsAvailable = 0; >+my $biblioLoop = []; >+my $numBibsAvailable = 0; > my $itemdata_enumchron = 0; >-my $itemdata_ccode = 0; >-my $anyholdable = 0; >-my $itemLevelTypes = C4::Context->preference('item-level_itypes'); >-my $pickup_locations = Koha::Libraries->search({ pickup_location => 1 }); >-$template->param('item_level_itypes' => $itemLevelTypes); >+my $itemdata_ccode = 0; >+my $anyholdable = 0; >+my $itemLevelTypes = C4::Context->preference('item-level_itypes'); >+my $pickup_locations = Koha::Libraries->search( { pickup_location => 1 } ); >+$template->param( 'item_level_itypes' => $itemLevelTypes ); > > my $patron_unblessed = $patron->unblessed; > foreach my $biblioNum (@biblionumbers) { >@@ -404,56 +404,57 @@ foreach my $biblioNum (@biblionumbers) { > > # Get relevant biblio data. > my $biblioData = $biblioDataHash{$biblioNum}; >- if (! $biblioData) { >- $template->param(message=>1, bad_biblionumber=>$biblioNum); >+ if ( !$biblioData ) { >+ $template->param( message => 1, bad_biblionumber => $biblioNum ); > output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; > exit; > } > > my @not_available_at = (); >- my $biblio = $biblioData->{object}; >+ my $biblio = $biblioData->{object}; > foreach my $library ( $pickup_locations->as_list ) { >- push( @not_available_at, $library->branchcode ) unless $biblio->can_be_transferred({ to => $library }); >+ push( @not_available_at, $library->branchcode ) unless $biblio->can_be_transferred( { to => $library } ); > } > > my $frameworkcode = GetFrameworkCode( $biblioData->{biblionumber} ); >- $biblioLoopIter{biblionumber} = $biblioData->{biblionumber}; >- $biblioLoopIter{title} = $biblioData->{title}; >- $biblioLoopIter{subtitle} = $biblioData->{'subtitle'}; >- $biblioLoopIter{medium} = $biblioData->{medium}; >- $biblioLoopIter{part_number} = $biblioData->{part_number}; >- $biblioLoopIter{part_name} = $biblioData->{part_name}; >- $biblioLoopIter{author} = $biblioData->{author}; >- $biblioLoopIter{rank} = $biblioData->{rank}; >- $biblioLoopIter{reservecount} = $biblioData->{reservecount}; >+ $biblioLoopIter{biblionumber} = $biblioData->{biblionumber}; >+ $biblioLoopIter{title} = $biblioData->{title}; >+ $biblioLoopIter{subtitle} = $biblioData->{'subtitle'}; >+ $biblioLoopIter{medium} = $biblioData->{medium}; >+ $biblioLoopIter{part_number} = $biblioData->{part_number}; >+ $biblioLoopIter{part_name} = $biblioData->{part_name}; >+ $biblioLoopIter{author} = $biblioData->{author}; >+ $biblioLoopIter{rank} = $biblioData->{rank}; >+ $biblioLoopIter{reservecount} = $biblioData->{reservecount}; > $biblioLoopIter{already_reserved} = $biblioData->{already_reserved}; >- $biblioLoopIter{object} = $biblio; >+ $biblioLoopIter{object} = $biblio; > >- if (!$itemLevelTypes && $biblioData->{itemtype}) { >- $biblioLoopIter{translated_description} = $itemtypes->{$biblioData->{itemtype}}{translated_description}; >- $biblioLoopIter{imageurl} = getitemtypeimagesrc() . "/". $itemtypes->{$biblioData->{itemtype}}{imageurl}; >+ if ( !$itemLevelTypes && $biblioData->{itemtype} ) { >+ $biblioLoopIter{translated_description} = $itemtypes->{ $biblioData->{itemtype} }{translated_description}; >+ $biblioLoopIter{imageurl} = getitemtypeimagesrc() . "/" . $itemtypes->{ $biblioData->{itemtype} }{imageurl}; > } > >- >- > $biblioLoopIter{itemLoop} = []; >- my $numCopiesAvailable = 0; >+ my $numCopiesAvailable = 0; > my $numCopiesOPACAvailable = 0; >+ > # iterating through all items first to check if any of them available > # to pass this value further inside down to IsAvailableForItemLevelRequest to > # it's complicated logic to analyse. > # (before this loop was inside that sub loop so it was O(n^2) ) >- foreach my $item (@{$biblioData->{items}}) { >+ foreach my $item ( @{ $biblioData->{items} } ) { > > my $item_info = $item->unblessed; > $item_info->{holding_branch} = $item->holding_branch; > $item_info->{home_branch} = $item->home_branch; > if ($itemLevelTypes) { > my $itemtype = $item->itemtype; >- $item_info->{'imageurl'} = getitemtypeimagelocation( 'opac', >- $itemtypes->{ $itemtype->itemtype }->{'imageurl'} ); >+ $item_info->{'imageurl'} = getitemtypeimagelocation( >+ 'opac', >+ $itemtypes->{ $itemtype->itemtype }->{'imageurl'} >+ ); > $item_info->{'translated_description'} = >- $itemtypes->{ $itemtype->itemtype }->{translated_description}; >+ $itemtypes->{ $itemtype->itemtype }->{translated_description}; > } > > # checking for holds >@@ -469,8 +470,8 @@ foreach my $biblioNum (@biblionumbers) { > if ( $transfer && $transfer->in_transit ) { > $item_info->{transfertwhen} = $transfer->datesent; > $item_info->{transfertfrom} = $transfer->frombranch; >- $item_info->{transfertto} = $transfer->tobranch; >- $item_info->{nocancel} = 1; >+ $item_info->{transfertto} = $transfer->tobranch; >+ $item_info->{nocancel} = 1; > } > > # if the items belongs to a host record, show link to host record >@@ -483,14 +484,14 @@ foreach my $biblioNum (@biblionumbers) { > > my $item_status = CanItemBeReserved( $patron, $item, undef, { get_from_cache => 1 } )->{status}; > >- if ($item_status eq 'recordHoldsOnly') { >- $biblioLoopIter{force_hold} = 1; >- $biblioLoopIter{itemholdable} = 0; >+ if ( $item_status eq 'recordHoldsOnly' ) { >+ $biblioLoopIter{force_hold} = 1; >+ $biblioLoopIter{itemholdable} = 0; > $biblioLoopIter{forced_hold_level} = 'record'; > } > >- my $policy_holdallowed = IsAvailableForItemLevelRequest($item, $patron, undef) && >- ($item_status eq 'OK' or $item_status eq 'recordHoldsOnly'); >+ my $policy_holdallowed = IsAvailableForItemLevelRequest( $item, $patron, undef ) >+ && ( $item_status eq 'OK' or $item_status eq 'recordHoldsOnly' ); > > if ($policy_holdallowed) { > $numCopiesAvailable++; >@@ -498,9 +499,12 @@ foreach my $biblioNum (@biblionumbers) { > $numCopiesOPACAvailable++; > $item_info->{available} = 1; > } >- unless ( $can_place_hold_if_available_at_pickup ) { >- my $items_in_this_library = Koha::Items->search({ biblionumber => $item->biblionumber, holdingbranch => $item->holdingbranch, notforloan => 0 }); >- my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count; >+ unless ($can_place_hold_if_available_at_pickup) { >+ my $items_in_this_library = Koha::Items->search( >+ { biblionumber => $item->biblionumber, holdingbranch => $item->holdingbranch, notforloan => 0 } ); >+ my $nb_of_items_issued = >+ $items_in_this_library->search( { 'issue.itemnumber' => { not => undef } }, { join => 'issue' } ) >+ ->count; > if ( $items_in_this_library->count > $nb_of_items_issued ) { > push @not_available_at, $item->holdingbranch; > } >@@ -508,39 +512,41 @@ foreach my $biblioNum (@biblionumbers) { > } > > # Show serial enumeration when needed >- if ($item_info->{enumchron}) { >+ if ( $item_info->{enumchron} ) { > $itemdata_enumchron = 1; > } >+ > # Show collection when needed >- if ($item_info->{ccode}) { >+ if ( $item_info->{ccode} ) { > $itemdata_ccode = 1; > } > >- push @{$biblioLoopIter{itemLoop}}, $item_info; >+ push @{ $biblioLoopIter{itemLoop} }, $item_info; > } > $template->param( > itemdata_enumchron => $itemdata_enumchron, > itemdata_ccode => $itemdata_ccode, > ); > >- if ($numCopiesAvailable > 0) { >+ if ( $numCopiesAvailable > 0 ) { > $numBibsAvailable++; > $biblioLoopIter{bib_available} = 1; >- $biblioLoopIter{holdable} = 1; >- $biblioLoopIter{itemholdable} = 1 if $numCopiesOPACAvailable; >+ $biblioLoopIter{holdable} = 1; >+ $biblioLoopIter{itemholdable} = 1 if $numCopiesOPACAvailable; > } >- if ($biblioLoopIter{already_reserved}) { >- $biblioLoopIter{holdable} = undef; >+ if ( $biblioLoopIter{already_reserved} ) { >+ $biblioLoopIter{holdable} = undef; > $biblioLoopIter{itemholdable} = undef; > } > if ( $biblioLoopIter{holdable} ) { > @not_available_at = uniq @not_available_at; >- $biblioLoopIter{not_available_at} = \@not_available_at ; >+ $biblioLoopIter{not_available_at} = \@not_available_at; > } > >- unless ( $can_place_hold_if_available_at_pickup ) { >+ unless ($can_place_hold_if_available_at_pickup) { > @not_available_at = uniq @not_available_at; >- $biblioLoopIter{not_available_at} = \@not_available_at ; >+ $biblioLoopIter{not_available_at} = \@not_available_at; >+ > # The record is not holdable is not available at any of the libraries > if ( Koha::Libraries->search->count == @not_available_at ) { > $biblioLoopIter{holdable} = 0; >@@ -548,28 +554,29 @@ foreach my $biblioNum (@biblionumbers) { > } > > my $record_status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status}; >- if ($record_status eq 'recordHoldNotAllowed') { >- $biblioLoopIter{force_hold} = 1; >- $biblioLoopIter{itemholdable} = 1; >+ if ( $record_status eq 'recordHoldNotAllowed' ) { >+ $biblioLoopIter{force_hold} = 1; >+ $biblioLoopIter{itemholdable} = 1; > $biblioLoopIter{forced_hold_level} = 'item'; > $biblioLoopIter{holdable} &&= 1; >- } >- else { >+ } else { > $biblioLoopIter{holdable} &&= $record_status eq 'OK'; > } > > if ( $biblioLoopIter{holdable} and C4::Context->preference('AllowHoldItemTypeSelection') ) { >+ > # build the allowed item types loop > my $rs = $biblio->items->search_ordered( > undef, >- { select => [ { distinct => 'itype' } ], >+ { >+ select => [ { distinct => 'itype' } ], > as => 'item_type' > } > ); > > my @item_types = >- grep { CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $_ } )->{status} eq 'OK' } >- $rs->get_column('item_type'); >+ grep { CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $_ } )->{status} eq 'OK' } >+ $rs->get_column('item_type'); > > $biblioLoopIter{allowed_item_types} = \@item_types; > } >@@ -591,10 +598,10 @@ foreach my $biblioNum (@biblionumbers) { > } > )->forced_hold_level(); > if ($forced_hold_level) { >- $biblioLoopIter{force_hold} = 1 if $forced_hold_level eq 'item'; >- $biblioLoopIter{force_hold} = 0 if $forced_hold_level eq 'item_group'; >- $biblioLoopIter{itemholdable} = 0 if $forced_hold_level eq 'record'; >- $biblioLoopIter{itemholdable} = 0 if $forced_hold_level eq 'item_group'; >+ $biblioLoopIter{force_hold} = 1 if $forced_hold_level eq 'item'; >+ $biblioLoopIter{force_hold} = 0 if $forced_hold_level eq 'item_group'; >+ $biblioLoopIter{itemholdable} = 0 if $forced_hold_level eq 'record'; >+ $biblioLoopIter{itemholdable} = 0 if $forced_hold_level eq 'item_group'; > $biblioLoopIter{forced_hold_level} = $forced_hold_level; > } > >@@ -606,35 +613,35 @@ foreach my $biblioNum (@biblionumbers) { > $anyholdable = 1 if $biblioLoopIter{holdable}; > } > >-unless ($pickup_locations->count) { >+unless ( $pickup_locations->count ) { > $numBibsAvailable = 0; >- $anyholdable = 0; >+ $anyholdable = 0; > $template->param( >- message => 1, >+ message => 1, > no_pickup_locations => 1 > ); > } > >-if ( $numBibsAvailable == 0 || $anyholdable == 0) { >+if ( $numBibsAvailable == 0 || $anyholdable == 0 ) { > $template->param( none_available => 1 ); > } > >-if (scalar @biblionumbers > 1) { >- $template->param( multi_hold => 1); >+if ( scalar @biblionumbers > 1 ) { >+ $template->param( multi_hold => 1 ); > } > >-my $show_notes=C4::Context->preference('OpacHoldNotes'); >-$template->param(OpacHoldNotes=>$show_notes); >+my $show_notes = C4::Context->preference('OpacHoldNotes'); >+$template->param( OpacHoldNotes => $show_notes ); > > # display infos >-$template->param(bibitemloop => $biblioLoop); >+$template->param( bibitemloop => $biblioLoop ); >+ > # can set reserve date in future >-if ( >- C4::Context->preference( 'AllowHoldDateInFuture' ) && >- C4::Context->preference( 'OPACAllowHoldDateInFuture' ) >- ) { >+if ( C4::Context->preference('AllowHoldDateInFuture') >+ && C4::Context->preference('OPACAllowHoldDateInFuture') ) >+{ > $template->param( >- reserve_in_future => 1, >+ reserve_in_future => 1, > ); > } > >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index ed4b64db465..923d608a326 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -32,7 +32,8 @@ use C4::Items; > use C4::Biblio qw( GetMarcFromKohaField ModBiblio ); > use C4::HoldsQueue; > use C4::Members; >-use C4::Reserves qw( AddReserve AlterPriority CheckReserves ModReserve ModReserveAffect ReserveSlip CalculatePriority CanReserveBeCanceledFromOpac CanBookBeReserved IsAvailableForItemLevelRequest MoveReserve ChargeReserveFee RevertWaitingStatus CanItemBeReserved MergeHolds ); >+use C4::Reserves >+ qw( AddReserve AlterPriority CheckReserves ModReserve ModReserveAffect ReserveSlip CalculatePriority CanReserveBeCanceledFromOpac CanBookBeReserved IsAvailableForItemLevelRequest MoveReserve ChargeReserveFee RevertWaitingStatus CanItemBeReserved MergeHolds ); > use Koha::ActionLogs; > use Koha::Biblios; > use Koha::Caches; >@@ -51,7 +52,7 @@ BEGIN { > > # Start transaction > my $database = Koha::Database->new(); >-my $schema = $database->schema(); >+my $schema = $database->schema(); > $schema->storage->txn_begin(); > my $dbh = C4::Context->dbh; > $dbh->do('DELETE FROM circulation_rules'); >@@ -60,11 +61,13 @@ my $builder = t::lib::TestBuilder->new; > > my $frameworkcode = q//; > >- >-t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); >+t::lib::Mocks::mock_preference( 'ReservesNeedReturns', 1 ); > > # Somewhat arbitrary field chosen for age restriction unit tests. Must be added to db before the framework is cached >-$dbh->do("update marc_subfield_structure set kohafield='biblioitems.agerestriction' where tagfield='521' and tagsubfield='a' and frameworkcode=?", undef, $frameworkcode); >+$dbh->do( >+ "update marc_subfield_structure set kohafield='biblioitems.agerestriction' where tagfield='521' and tagsubfield='a' and frameworkcode=?", >+ undef, $frameworkcode >+); > my $cache = Koha::Caches->get_instance; > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >@@ -72,41 +75,41 @@ $cache->clear_from_cache("MarcSubfieldStructure-$frameworkcode"); > > ## Setup Test > # Add branches >-my $branch_1 = $builder->build({ source => 'Branch' })->{ branchcode }; >-my $branch_2 = $builder->build({ source => 'Branch' })->{ branchcode }; >-my $branch_3 = $builder->build({ source => 'Branch' })->{ branchcode }; >+my $branch_1 = $builder->build( { source => 'Branch' } )->{branchcode}; >+my $branch_2 = $builder->build( { source => 'Branch' } )->{branchcode}; >+my $branch_3 = $builder->build( { source => 'Branch' } )->{branchcode}; >+ > # Add categories >-my $category_1 = $builder->build({ source => 'Category' })->{ categorycode }; >-my $category_2 = $builder->build({ source => 'Category' })->{ categorycode }; >+my $category_1 = $builder->build( { source => 'Category' } )->{categorycode}; >+my $category_2 = $builder->build( { source => 'Category' } )->{categorycode}; >+ > # Add an item type >-my $itemtype = $builder->build( >- { source => 'Itemtype', value => { notforloan => undef } } )->{itemtype}; >+my $itemtype = $builder->build( { source => 'Itemtype', value => { notforloan => undef } } )->{itemtype}; > >-t::lib::Mocks::mock_userenv({ branchcode => $branch_1 }); >+t::lib::Mocks::mock_userenv( { branchcode => $branch_1 } ); > >-my $bibnum = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; >+my $bibnum = $builder->build_sample_biblio( { frameworkcode => $frameworkcode } )->biblionumber; > > # Create a helper item instance for testing >-my $item = $builder->build_sample_item({ biblionumber => $bibnum, library => $branch_1, itype => $itemtype }); >+my $item = $builder->build_sample_item( { biblionumber => $bibnum, library => $branch_1, itype => $itemtype } ); > > my $biblio_with_no_item = $builder->build_sample_biblio; > > # Modify item; setting barcode. > my $testbarcode = '97531'; >-$item->barcode($testbarcode)->store; # FIXME We should not hardcode a barcode! Also, what's the purpose of this? >- >+$item->barcode($testbarcode)->store; # FIXME We should not hardcode a barcode! Also, what's the purpose of this? > > # Create a borrower > my %data = ( >- firstname => 'my firstname', >- surname => 'my surname', >+ firstname => 'my firstname', >+ surname => 'my surname', > categorycode => $category_1, >- branchcode => $branch_1, >+ branchcode => $branch_1, > ); >-Koha::Patron::Categories->find($category_1)->set({ enrolmentfee => 0})->store; >-my $borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber; >-my $patron = Koha::Patrons->find( $borrowernumber ); >-my $borrower = $patron->unblessed; >+Koha::Patron::Categories->find($category_1)->set( { enrolmentfee => 0 } )->store; >+my $borrowernumber = Koha::Patron->new( \%data )->store->borrowernumber; >+my $patron = Koha::Patrons->find($borrowernumber); >+my $borrower = $patron->unblessed; > my $biblionumber = $bibnum; > > my $branchcode = Koha::Libraries->search->next->branchcode; >@@ -120,41 +123,49 @@ AddReserve( > } > ); > >-my ($status, $reserve, $all_reserves) = CheckReserves( $item ); >+my ( $status, $reserve, $all_reserves ) = CheckReserves($item); > >-is($status, "Reserved", "CheckReserves Test 1"); >+is( $status, "Reserved", "CheckReserves Test 1" ); > >-ok(exists($reserve->{reserve_id}), 'CheckReserves() include reserve_id in its response'); >+ok( exists( $reserve->{reserve_id} ), 'CheckReserves() include reserve_id in its response' ); > >-($status, $reserve, $all_reserves) = CheckReserves( $item ); >-is($status, "Reserved", "CheckReserves Test 2"); >+( $status, $reserve, $all_reserves ) = CheckReserves($item); >+is( $status, "Reserved", "CheckReserves Test 2" ); > > ### > ### Regression test for bug 10272 > ### > my %requesters = (); >-$requesters{$branch_1} = Koha::Patron->new({ >- branchcode => $branch_1, >- categorycode => $category_2, >- surname => "borrower from $branch_1", >-})->store->borrowernumber; >-for my $i ( 2 .. 5 ) { >- $requesters{"CPL$i"} = Koha::Patron->new({ >+$requesters{$branch_1} = Koha::Patron->new( >+ { > branchcode => $branch_1, > categorycode => $category_2, >- surname => "borrower $i from $branch_1", >- })->store->borrowernumber; >+ surname => "borrower from $branch_1", >+ } >+)->store->borrowernumber; >+for my $i ( 2 .. 5 ) { >+ $requesters{"CPL$i"} = Koha::Patron->new( >+ { >+ branchcode => $branch_1, >+ categorycode => $category_2, >+ surname => "borrower $i from $branch_1", >+ } >+ )->store->borrowernumber; > } >-$requesters{$branch_2} = Koha::Patron->new({ >- branchcode => $branch_2, >- categorycode => $category_2, >- surname => "borrower from $branch_2", >-})->store->borrowernumber; >-$requesters{$branch_3} = Koha::Patron->new({ >- branchcode => $branch_3, >- categorycode => $category_2, >- surname => "borrower from $branch_3", >-})->store->borrowernumber; >+$requesters{$branch_2} = Koha::Patron->new( >+ { >+ branchcode => $branch_2, >+ categorycode => $category_2, >+ surname => "borrower from $branch_2", >+ } >+)->store->borrowernumber; >+$requesters{$branch_3} = Koha::Patron->new( >+ { >+ branchcode => $branch_3, >+ categorycode => $category_2, >+ surname => "borrower from $branch_3", >+ } >+)->store->borrowernumber; > > # Configure rules so that $branch_1 allows only $branch_1 patrons > # to request its items, while $branch_2 will allow its items >@@ -167,7 +178,7 @@ Koha::CirculationRules->set_rules( > categorycode => undef, > itemtype => undef, > rules => { >- reservesallowed => 25, >+ reservesallowed => 25, > holds_per_record => 1, > } > } >@@ -176,9 +187,9 @@ Koha::CirculationRules->set_rules( > # CPL allows only its own patrons to request its items > Koha::CirculationRules->set_rules( > { >- branchcode => $branch_1, >- itemtype => undef, >- rules => { >+ branchcode => $branch_1, >+ itemtype => undef, >+ rules => { > holdallowed => 'from_home_library', > returnbranch => 'homebranch', > } >@@ -188,18 +199,18 @@ Koha::CirculationRules->set_rules( > # ... while FPL allows anybody to request its items > Koha::CirculationRules->set_rules( > { >- branchcode => $branch_2, >- itemtype => undef, >- rules => { >+ branchcode => $branch_2, >+ itemtype => undef, >+ rules => { > holdallowed => 'from_any_library', > returnbranch => 'homebranch', > } > } > ); > >-my $bibnum2 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; >+my $bibnum2 = $builder->build_sample_biblio( { frameworkcode => $frameworkcode } )->biblionumber; > >-my ($itemnum_cpl, $itemnum_fpl); >+my ( $itemnum_cpl, $itemnum_fpl ); > $itemnum_cpl = $builder->build_sample_item( > { > biblionumber => $bibnum2, >@@ -219,7 +230,7 @@ $itemnum_fpl = $builder->build_sample_item( > > # Ensure that priorities are numbered correcly when a hold is moved to waiting > # (bug 11947) >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum2) ); > AddReserve( > { > branchcode => $branch_3, >@@ -244,21 +255,20 @@ AddReserve( > priority => 3, > } > ); >-ModReserveAffect($itemnum_cpl, $requesters{$branch_3}, 0); >+ModReserveAffect( $itemnum_cpl, $requesters{$branch_3}, 0 ); > > # Now it should have different priorities. >-my $biblio = Koha::Biblios->find( $bibnum2 ); >-my $holds = $biblio->holds({}, { order_by => 'reserve_id' });; >-is($holds->next->priority, 0, 'Item is correctly waiting'); >-is($holds->next->priority, 1, 'Item is correctly priority 1'); >-is($holds->next->priority, 2, 'Item is correctly priority 2'); >- >-my @reserves = Koha::Holds->search({ borrowernumber => $requesters{$branch_3} })->waiting->as_list; >-is( @reserves, 1, 'GetWaiting got only the waiting reserve' ); >+my $biblio = Koha::Biblios->find($bibnum2); >+my $holds = $biblio->holds( {}, { order_by => 'reserve_id' } ); >+is( $holds->next->priority, 0, 'Item is correctly waiting' ); >+is( $holds->next->priority, 1, 'Item is correctly priority 1' ); >+is( $holds->next->priority, 2, 'Item is correctly priority 2' ); >+ >+my @reserves = Koha::Holds->search( { borrowernumber => $requesters{$branch_3} } )->waiting->as_list; >+is( @reserves, 1, 'GetWaiting got only the waiting reserve' ); > is( $reserves[0]->borrowernumber(), $requesters{$branch_3}, 'GetWaiting got the reserve for the correct borrower' ); > >- >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum2) ); > AddReserve( > { > branchcode => $branch_3, >@@ -289,13 +299,16 @@ AddReserve( > t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); > > my $messages; >+ > # Return the CPL item at FPL. The hold that should be triggered is > # 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}, >+( 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)'); >+ 'restrictive library\'s items only fill requests by own patrons (bug 10272)' >+); > > # Return the FPL item at FPL. The hold that should be triggered is > # the one placed by the RPL patron, as that patron is first in line >@@ -304,20 +317,22 @@ is( $messages->{ResFound}->{borrowernumber}, > # Ensure that the preference 'LocalHoldsPriority' is not set (Bug 15244): > t::lib::Mocks::mock_preference( 'LocalHoldsPriority', '' ); > >-(undef, $messages, undef, undef) = AddReturn('bug10272_FPL', $branch_2); >-is( $messages->{ResFound}->{borrowernumber}, >+( undef, $messages, undef, undef ) = AddReturn( 'bug10272_FPL', $branch_2 ); >+is( >+ $messages->{ResFound}->{borrowernumber}, > $requesters{$branch_3}, >- 'for generous library, its items fill first hold request in line (bug 10272)'); >+ 'for generous library, its items fill first hold request in line (bug 10272)' >+); > >-$biblio = Koha::Biblios->find( $biblionumber ); >-$holds = $biblio->holds; >-is($holds->count, 1, "Only one reserves for this biblio"); >+$biblio = Koha::Biblios->find($biblionumber); >+$holds = $biblio->holds; >+is( $holds->count, 1, "Only one reserves for this biblio" ); > $holds->next->reserve_id; > > # Tests for bug 9761 (ConfirmFutureHolds): new CheckReserves lookahead parameter, and corresponding change in AddReturn > # Note that CheckReserve uses its lookahead parameter and does not check ConfirmFutureHolds pref (it should be passed if needed like AddReturn does) > # Test 9761a: Add a reserve without date, CheckReserve should return it >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); > AddReserve( > { > branchcode => $branch_1, >@@ -326,16 +341,16 @@ AddReserve( > priority => 1, > } > ); >-($status)=CheckReserves( $item ); >-is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead'); >-($status)=CheckReserves( $item, 7 ); >-is( $status, 'Reserved', 'CheckReserves also returns reserve with lookahead'); >+($status) = CheckReserves($item); >+is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead' ); >+($status) = CheckReserves( $item, 7 ); >+is( $status, 'Reserved', 'CheckReserves also returns reserve with lookahead' ); > > # Test 9761b: Add a reserve with future date, CheckReserve should not return it >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >-t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); >-my $resdate= dt_from_string(); >-$resdate->add_duration(DateTime::Duration->new(days => 4)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); >+t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 ); >+my $resdate = dt_from_string(); >+$resdate->add_duration( DateTime::Duration->new( days => 4 ) ); > my $reserve_id = AddReserve( > { > branchcode => $branch_1, >@@ -345,31 +360,36 @@ my $reserve_id = AddReserve( > reservation_date => $resdate, > } > ); >-($status)=CheckReserves( $item ); >-is( $status, '', 'CheckReserves returns no future reserve without lookahead'); >+($status) = CheckReserves($item); >+is( $status, '', 'CheckReserves returns no future reserve without lookahead' ); > > # Test 9761c: Add a reserve with future date, CheckReserve should return it if lookahead is high enough >-($status)=CheckReserves( $item, 3 ); >-is( $status, '', 'CheckReserves returns no future reserve with insufficient lookahead'); >-($status)=CheckReserves( $item, 4 ); >-is( $status, 'Reserved', 'CheckReserves returns future reserve with sufficient lookahead'); >+($status) = CheckReserves( $item, 3 ); >+is( $status, '', 'CheckReserves returns no future reserve with insufficient lookahead' ); >+($status) = CheckReserves( $item, 4 ); >+is( $status, 'Reserved', 'CheckReserves returns future reserve with sufficient lookahead' ); > > # Test 9761d: Check ResFound message of AddReturn for future hold > # Note that AddReturn is in Circulation.pm, but this test really pertains to reserves; AddReturn uses the ConfirmFutureHolds pref when calling CheckReserves > # In this test we do not need an issued item; it is just a 'checkin' >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0); >-(my $doreturn, $messages)= AddReturn($testbarcode,$branch_1); >-is($messages->{ResFound}//'', '', 'AddReturn does not care about future reserve when ConfirmFutureHolds is off'); >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 3); >-($doreturn, $messages)= AddReturn($testbarcode,$branch_1); >-is(exists $messages->{ResFound}?1:0, 0, 'AddReturn ignores future reserve beyond ConfirmFutureHolds days'); >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 7); >-($doreturn, $messages)= AddReturn($testbarcode,$branch_1); >-is(exists $messages->{ResFound}?1:0, 1, 'AddReturn considers future reserve within ConfirmFutureHolds days'); >- >-my $now_holder = $builder->build_object({ class => 'Koha::Patrons', value => { >- branchcode => $branch_1, >-}}); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 0 ); >+( my $doreturn, $messages ) = AddReturn( $testbarcode, $branch_1 ); >+is( $messages->{ResFound} // '', '', 'AddReturn does not care about future reserve when ConfirmFutureHolds is off' ); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 3 ); >+( $doreturn, $messages ) = AddReturn( $testbarcode, $branch_1 ); >+is( exists $messages->{ResFound} ? 1 : 0, 0, 'AddReturn ignores future reserve beyond ConfirmFutureHolds days' ); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 7 ); >+( $doreturn, $messages ) = AddReturn( $testbarcode, $branch_1 ); >+is( exists $messages->{ResFound} ? 1 : 0, 1, 'AddReturn considers future reserve within ConfirmFutureHolds days' ); >+ >+my $now_holder = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { >+ branchcode => $branch_1, >+ } >+ } >+); > my $now_reserve_id = AddReserve( > { > branchcode => $branch_1, >@@ -380,46 +400,50 @@ my $now_reserve_id = AddReserve( > } > ); > my $which_highest; >-($status,$which_highest)=CheckReserves( $item, 3 ); >-is( $which_highest->{reserve_id}, $now_reserve_id, 'CheckReserves returns lower priority current reserve with insufficient lookahead'); >-($status, $which_highest)=CheckReserves( $item, 4 ); >-is( $which_highest->{reserve_id}, $reserve_id, 'CheckReserves returns higher priority future reserve with sufficient lookahead'); >-ModReserve({ reserve_id => $now_reserve_id, rank => 'del', cancellation_reason => 'test reserve' }); >- >+( $status, $which_highest ) = CheckReserves( $item, 3 ); >+is( >+ $which_highest->{reserve_id}, $now_reserve_id, >+ 'CheckReserves returns lower priority current reserve with insufficient lookahead' >+); >+( $status, $which_highest ) = CheckReserves( $item, 4 ); >+is( >+ $which_highest->{reserve_id}, $reserve_id, >+ 'CheckReserves returns higher priority future reserve with sufficient lookahead' >+); >+ModReserve( { reserve_id => $now_reserve_id, rank => 'del', cancellation_reason => 'test reserve' } ); > > # End of tests for bug 9761 (ConfirmFutureHolds) > >- > # test marking a hold as captured > my $hold_notice_count = count_hold_print_messages(); >-ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 0 ); > my $new_count = count_hold_print_messages(); >-is($new_count, $hold_notice_count + 1, 'patron notified when item set to waiting'); >+is( $new_count, $hold_notice_count + 1, 'patron notified when item set to waiting' ); > > # test that duplicate notices aren't generated >-ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 0 ); > $new_count = count_hold_print_messages(); >-is($new_count, $hold_notice_count + 1, 'patron not notified a second time (bug 11445)'); >+is( $new_count, $hold_notice_count + 1, 'patron not notified a second time (bug 11445)' ); > > # avoiding the not_same_branch error >-t::lib::Mocks::mock_preference('IndependentBranches', 0); >-$item = Koha::Items->find($item->itemnumber); >+t::lib::Mocks::mock_preference( 'IndependentBranches', 0 ); >+$item = Koha::Items->find( $item->itemnumber ); > is( >- @{$item->safe_delete->messages}[0]->message, >+ @{ $item->safe_delete->messages }[0]->message, > 'book_reserved', > 'item that is captured to fill a hold cannot be deleted', > ); > > my $letter = ReserveSlip( { branchcode => $branch_1, reserve_id => $reserve_id } ); >-ok(defined($letter), 'can successfully generate hold slip (bug 10949)'); >+ok( defined($letter), 'can successfully generate hold slip (bug 10949)' ); > > # Tests for bug 9788: Does Koha::Item->current_holds return a future wait? > # 9788a: current_holds does not return future next available hold >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 2); >-t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); >-$resdate= dt_from_string(); >-$resdate->add_duration(DateTime::Duration->new(days => 2)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 2 ); >+t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 ); >+$resdate = dt_from_string(); >+$resdate->add_duration( DateTime::Duration->new( days => 2 ) ); > AddReserve( > { > branchcode => $branch_1, >@@ -431,11 +455,12 @@ AddReserve( > ); > > $holds = $item->current_holds; >-my $dtf = Koha::Database->new->schema->storage->datetime_parser; >-my $future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); >-is( $future_holds->count, 0, 'current_holds does not return a future next available hold'); >+my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+my $future_holds = $holds->search( { reservedate => { '>' => $dtf->format_date(dt_from_string) } } ); >+is( $future_holds->count, 0, 'current_holds does not return a future next available hold' ); >+ > # 9788b: current_holds does not return future item level hold >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); > AddReserve( > { > branchcode => $branch_1, >@@ -445,19 +470,22 @@ AddReserve( > reservation_date => $resdate, > itemnumber => $item->itemnumber, > } >-); #item level hold >-$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); >+); #item level hold >+$future_holds = $holds->search( { reservedate => { '>' => $dtf->format_date(dt_from_string) } } ); > is( $future_holds->count, 0, 'current_holds does not return a future item level hold' ); >+ > # 9788c: current_holds returns future wait (confirmed future hold) >-ModReserveAffect( $item->itemnumber, $requesters{$branch_1} , 0); #confirm hold >-$future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } ); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 0 ); #confirm hold >+$future_holds = $holds->search( { reservedate => { '>' => $dtf->format_date(dt_from_string) } } ); > is( $future_holds->count, 1, 'current_holds returns a future wait (confirmed future hold)' ); >+ > # End of tests for bug 9788 > >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); >+ > # Tests for CalculatePriority (bug 8918) > my $p = C4::Reserves::CalculatePriority($bibnum2); >-is($p, 4, 'CalculatePriority should now return priority 4'); >+is( $p, 4, 'CalculatePriority should now return priority 4' ); > AddReserve( > { > branchcode => $branch_1, >@@ -467,11 +495,13 @@ AddReserve( > } > ); > $p = C4::Reserves::CalculatePriority($bibnum2); >-is($p, 5, 'CalculatePriority should now return priority 5'); >+is( $p, 5, 'CalculatePriority should now return priority 5' ); >+ > #some tests on bibnum >-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); >+$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, ($bibnum) ); > $p = C4::Reserves::CalculatePriority($bibnum); >-is($p, 1, 'CalculatePriority should now return priority 1'); >+is( $p, 1, 'CalculatePriority should now return priority 1' ); >+ > #add a new reserve and confirm it to waiting > AddReserve( > { >@@ -483,10 +513,11 @@ AddReserve( > } > ); > $p = C4::Reserves::CalculatePriority($bibnum); >-is($p, 2, 'CalculatePriority should now return priority 2'); >-ModReserveAffect( $item->itemnumber, $requesters{$branch_1} , 0); >+is( $p, 2, 'CalculatePriority should now return priority 2' ); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 0 ); > $p = C4::Reserves::CalculatePriority($bibnum); >-is($p, 1, 'CalculatePriority should now return priority 1'); >+is( $p, 1, 'CalculatePriority should now return priority 1' ); >+ > #add another biblio hold, no resdate > AddReserve( > { >@@ -497,68 +528,86 @@ AddReserve( > } > ); > $p = C4::Reserves::CalculatePriority($bibnum); >-is($p, 2, 'CalculatePriority should now return priority 2'); >+is( $p, 2, 'CalculatePriority should now return priority 2' ); >+ > #add another future hold >-t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); >-$resdate= dt_from_string(); >-$resdate->add_duration(DateTime::Duration->new(days => 1)); >+t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 ); >+$resdate = dt_from_string(); >+$resdate->add_duration( DateTime::Duration->new( days => 1 ) ); > AddReserve( > { >- branchcode => $branch_1, >- borrowernumber => $requesters{'CPL2'}, >- biblionumber => $bibnum, >- priority => $p, >+ branchcode => $branch_1, >+ borrowernumber => $requesters{'CPL2'}, >+ biblionumber => $bibnum, >+ priority => $p, > reservation_date => $resdate, > } > ); > $p = C4::Reserves::CalculatePriority($bibnum); >-is($p, 2, 'CalculatePriority should now still return priority 2'); >+is( $p, 2, 'CalculatePriority should now still return priority 2' ); >+ > #calc priority with future resdate >-$p = C4::Reserves::CalculatePriority($bibnum, $resdate); >-is($p, 3, 'CalculatePriority should now return priority 3'); >+$p = C4::Reserves::CalculatePriority( $bibnum, $resdate ); >+is( $p, 3, 'CalculatePriority should now return priority 3' ); >+ > # End of tests for bug 8918 > > # regression test for bug 12630 > # Now there are 2 reserves on $bibnum >-t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); >-my $bor_tmp_1 = $builder->build_object({ class => 'Koha::Patrons',value =>{ >- firstname => 'my firstname tmp 1', >- surname => 'my surname tmp 1', >- categorycode => 'S', >- branchcode => 'CPL', >-}}); >-my $bor_tmp_2 = $builder->build_object({ class => 'Koha::Patrons',value =>{ >- firstname => 'my firstname tmp 2', >- surname => 'my surname tmp 2', >- categorycode => 'S', >- branchcode => 'CPL', >-}}); >+t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 ); >+my $bor_tmp_1 = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { >+ firstname => 'my firstname tmp 1', >+ surname => 'my surname tmp 1', >+ categorycode => 'S', >+ branchcode => 'CPL', >+ } >+ } >+); >+my $bor_tmp_2 = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { >+ firstname => 'my firstname tmp 2', >+ surname => 'my surname tmp 2', >+ categorycode => 'S', >+ branchcode => 'CPL', >+ } >+ } >+); > my $borrowernumber_tmp_1 = $bor_tmp_1->borrowernumber; > my $borrowernumber_tmp_2 = $bor_tmp_2->borrowernumber; >-my $date_in_future = dt_from_string(); >-$date_in_future = $date_in_future->add_duration(DateTime::Duration->new(days => 1)); >-AddReserve({ >- branchcode => 'CPL', >- borrowernumber => $borrowernumber_tmp_1, >- biblionumber => $bibnum, >- priority => 3, >- reservation_date => $date_in_future >-}); >-AddReserve({ >- branchcode => 'CPL', >- borrowernumber => $borrowernumber_tmp_2, >- biblionumber => $bibnum, >- priority => 4, >- reservation_date => $date_in_future >-}); >-my @r1 = Koha::Holds->search({ borrowernumber => $borrowernumber_tmp_1 })->as_list; >-my @r2 = Koha::Holds->search({ borrowernumber => $borrowernumber_tmp_2 })->as_list; >-is( $r1[0]->priority, 3, 'priority for hold in future should be correct'); >-is( $r2[0]->priority, 4, 'priority for hold not in future should be correct'); >+my $date_in_future = dt_from_string(); >+$date_in_future = $date_in_future->add_duration( DateTime::Duration->new( days => 1 ) ); >+AddReserve( >+ { >+ branchcode => 'CPL', >+ borrowernumber => $borrowernumber_tmp_1, >+ biblionumber => $bibnum, >+ priority => 3, >+ reservation_date => $date_in_future >+ } >+); >+AddReserve( >+ { >+ branchcode => 'CPL', >+ borrowernumber => $borrowernumber_tmp_2, >+ biblionumber => $bibnum, >+ priority => 4, >+ reservation_date => $date_in_future >+ } >+); >+my @r1 = Koha::Holds->search( { borrowernumber => $borrowernumber_tmp_1 } )->as_list; >+my @r2 = Koha::Holds->search( { borrowernumber => $borrowernumber_tmp_2 } )->as_list; >+is( $r1[0]->priority, 3, 'priority for hold in future should be correct' ); >+is( $r2[0]->priority, 4, 'priority for hold not in future should be correct' ); >+ > # end of tests for bug 12630 > > # Tests for cancel reserves by users from OPAC. >-$dbh->do('DELETE FROM reserves', undef, ($bibnum)); >+$dbh->do( 'DELETE FROM reserves', undef, ($bibnum) ); > AddReserve( > { > branchcode => $branch_1, >@@ -567,9 +616,10 @@ AddReserve( > priority => 1, > } > ); >-my (undef, $canres, undef) = CheckReserves( $item ); >+my ( undef, $canres, undef ) = CheckReserves($item); > >-is( CanReserveBeCanceledFromOpac(), undef, >+is( >+ CanReserveBeCanceledFromOpac(), undef, > 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' > ); > is( >@@ -583,19 +633,21 @@ is( > 'CanReserveBeCanceledFromOpac should return undef if called without borrowernumber' > ); > >-my $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); >-is($cancancel, 1, 'Can user cancel its own reserve'); >+my $cancancel = CanReserveBeCanceledFromOpac( $canres->{reserve_id}, $requesters{$branch_1} ); >+is( $cancancel, 1, 'Can user cancel its own reserve' ); > >-$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_2}); >-is($cancancel, 0, 'Other user cant cancel reserve'); >+$cancancel = CanReserveBeCanceledFromOpac( $canres->{reserve_id}, $requesters{$branch_2} ); >+is( $cancancel, 0, 'Other user cant cancel reserve' ); > >-ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 1); >-$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); >-is($cancancel, 0, 'Reserve in transfer status cant be canceled'); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 1 ); >+$cancancel = CanReserveBeCanceledFromOpac( $canres->{reserve_id}, $requesters{$branch_1} ); >+is( $cancancel, 0, 'Reserve in transfer status cant be canceled' ); > >-$dbh->do('DELETE FROM reserves', undef, ($bibnum)); >-is( CanReserveBeCanceledFromOpac($canres->{resserve_id}, $requesters{$branch_1}), undef, >- 'Cannot cancel a deleted hold' ); >+$dbh->do( 'DELETE FROM reserves', undef, ($bibnum) ); >+is( >+ CanReserveBeCanceledFromOpac( $canres->{resserve_id}, $requesters{$branch_1} ), undef, >+ 'Cannot cancel a deleted hold' >+); > > AddReserve( > { >@@ -605,17 +657,17 @@ AddReserve( > priority => 1, > } > ); >-(undef, $canres, undef) = CheckReserves( $item ); >+( undef, $canres, undef ) = CheckReserves($item); > >-ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); >-$cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); >-is($cancancel, 0, 'Reserve in waiting status cant be canceled'); >+ModReserveAffect( $item->itemnumber, $requesters{$branch_1}, 0 ); >+$cancancel = CanReserveBeCanceledFromOpac( $canres->{reserve_id}, $requesters{$branch_1} ); >+is( $cancancel, 0, 'Reserve in waiting status cant be canceled' ); > > # End of tests for bug 12876 > >- #### >+#### > ####### Testing Bug 13113 - Prevent juvenile/children from reserving ageRestricted material >>> >- #### >+#### > > t::lib::Mocks::mock_preference( 'AgeRestrictionMarker', 'FSK|PEGI|Age|K' ); > >@@ -624,32 +676,44 @@ t::lib::Mocks::mock_preference( 'AgeRestrictionMarker', 'FSK|PEGI|Age|K' ); > #Set the ageRestriction for the Biblio > $biblio = Koha::Biblios->find($bibnum); > my $record = $biblio->metadata->record; >-my ( $ageres_tagid, $ageres_subfieldid ) = GetMarcFromKohaField( "biblioitems.agerestriction" ); >-$record->append_fields( MARC::Field->new($ageres_tagid, '', '', $ageres_subfieldid => 'PEGI 16') ); >+my ( $ageres_tagid, $ageres_subfieldid ) = GetMarcFromKohaField("biblioitems.agerestriction"); >+$record->append_fields( MARC::Field->new( $ageres_tagid, '', '', $ageres_subfieldid => 'PEGI 16' ) ); > C4::Biblio::ModBiblio( $record, $bibnum, $frameworkcode ); > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber)->{status} , 'OK', "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" ); >+is( >+ C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber )->{status}, 'OK', >+ "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" >+); > > #Set the dateofbirth for the Borrower making them "too young". > $borrower->{dateofbirth} = DateTime->now->add( years => -15 ); >-Koha::Patrons->find( $borrowernumber )->set({ dateofbirth => $borrower->{dateofbirth} })->store; >+Koha::Patrons->find($borrowernumber)->set( { dateofbirth => $borrower->{dateofbirth} } )->store; > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber)->{status} , 'ageRestricted', "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); >+is( >+ C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber )->{status}, 'ageRestricted', >+ "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails" >+); > > #Set the dateofbirth for the Borrower making them "too old". > $borrower->{dateofbirth} = DateTime->now->add( years => -30 ); >-Koha::Patrons->find( $borrowernumber )->set({ dateofbirth => $borrower->{dateofbirth} })->store; >+Koha::Patrons->find($borrowernumber)->set( { dateofbirth => $borrower->{dateofbirth} } )->store; > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber)->{status} , 'OK', "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); >+is( >+ C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber )->{status}, 'OK', >+ "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds" >+); > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblio_with_no_item->biblionumber)->{status} , '', "Biblio with no item. Status is empty"); >- #### >+is( >+ C4::Reserves::CanBookBeReserved( $borrowernumber, $biblio_with_no_item->biblionumber )->{status}, '', >+ "Biblio with no item. Status is empty" >+); >+#### > ####### EO Bug 13113 <<< >- #### >+#### > >-ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $patron), "Reserving a book on item level" ); >+ok( C4::Reserves::IsAvailableForItemLevelRequest( $item, $patron ), "Reserving a book on item level" ); > >-my $pickup_branch = $builder->build({ source => 'Branch' })->{ branchcode }; >+my $pickup_branch = $builder->build( { source => 'Branch' } )->{branchcode}; > t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); > my $limit = Koha::Item::Transfer::Limit->new( >@@ -659,17 +723,20 @@ my $limit = Koha::Item::Transfer::Limit->new( > itemtype => $item->effective_itemtype, > } > )->store(); >-is( C4::Reserves::IsAvailableForItemLevelRequest($item, $patron, $pickup_branch), 0, "Item level request not available due to transfer limit" ); >-t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); >+is( >+ C4::Reserves::IsAvailableForItemLevelRequest( $item, $patron, $pickup_branch ), 0, >+ "Item level request not available due to transfer limit" >+); >+t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); > >-my $categorycode = $borrower->{categorycode}; >+my $categorycode = $borrower->{categorycode}; > my $holdingbranch = $item->{holdingbranch}; > Koha::CirculationRules->set_rules( > { > categorycode => $categorycode, > itemtype => $item->effective_itemtype, > branchcode => $holdingbranch, >- rules => { >+ rules => { > onshelfholds => 1, > } > } >@@ -677,9 +744,9 @@ Koha::CirculationRules->set_rules( > > # tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526) > # hold from A pos 1, today, no fut holds: MoveReserve should fill it >-$dbh->do('DELETE FROM reserves', undef, ($bibnum)); >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0); >-t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); >+$dbh->do( 'DELETE FROM reserves', undef, ($bibnum) ); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 0 ); >+t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 ); > AddReserve( > { > branchcode => $branch_1, >@@ -689,10 +756,11 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item ); >-is( $status, '', 'MoveReserve filled hold'); >+($status) = CheckReserves($item); >+is( $status, '', 'MoveReserve filled hold' ); >+ > # hold from A waiting, today, no fut holds: MoveReserve should fill it >-my $other_item = $builder->build_sample_item({ biblionumber => $biblio->id }); >+my $other_item = $builder->build_sample_item( { biblionumber => $biblio->id } ); > AddReserve( > { > branchcode => $branch_1, >@@ -704,67 +772,71 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item ); >-is( $status, '', 'MoveReserve filled waiting hold'); >+($status) = CheckReserves($item); >+is( $status, '', 'MoveReserve filled waiting hold' ); >+ > # hold from A pos 1, tomorrow, no fut holds: not filled >-$resdate= dt_from_string(); >-$resdate->add_duration(DateTime::Duration->new(days => 1)); >+$resdate = dt_from_string(); >+$resdate->add_duration( DateTime::Duration->new( days => 1 ) ); > AddReserve( > { >- branchcode => $branch_1, >- borrowernumber => $borrowernumber, >- biblionumber => $bibnum, >- priority => 1, >+ branchcode => $branch_1, >+ borrowernumber => $borrowernumber, >+ biblionumber => $bibnum, >+ priority => 1, > reservation_date => $resdate, > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item, 1 ); >-is( $status, 'Reserved', 'MoveReserve did not fill future hold'); >-$dbh->do('DELETE FROM reserves', undef, ($bibnum)); >+($status) = CheckReserves( $item, 1 ); >+is( $status, 'Reserved', 'MoveReserve did not fill future hold' ); >+$dbh->do( 'DELETE FROM reserves', undef, ($bibnum) ); >+ > # hold from A pos 1, tomorrow, fut holds=2: MoveReserve should fill it >-t::lib::Mocks::mock_preference('ConfirmFutureHolds', 2); >+t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 2 ); > AddReserve( > { >- branchcode => $branch_1, >- borrowernumber => $borrowernumber, >- biblionumber => $bibnum, >- priority => 1, >+ branchcode => $branch_1, >+ borrowernumber => $borrowernumber, >+ biblionumber => $bibnum, >+ priority => 1, > reservation_date => $resdate, > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item, undef, 2 ); >-is( $status, '', 'MoveReserve filled future hold now'); >+($status) = CheckReserves( $item, undef, 2 ); >+is( $status, '', 'MoveReserve filled future hold now' ); >+ > # hold from A waiting, tomorrow, fut holds=2: MoveReserve should fill it > AddReserve( > { >- branchcode => $branch_1, >- borrowernumber => $borrowernumber, >- biblionumber => $bibnum, >- priority => 1, >+ branchcode => $branch_1, >+ borrowernumber => $borrowernumber, >+ biblionumber => $bibnum, >+ priority => 1, > reservation_date => $resdate, > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item, undef, 2 ); >-is( $status, '', 'MoveReserve filled future waiting hold now'); >+($status) = CheckReserves( $item, undef, 2 ); >+is( $status, '', 'MoveReserve filled future waiting hold now' ); >+ > # hold from A pos 1, today+3, fut holds=2: MoveReserve should not fill it >-$resdate= dt_from_string(); >-$resdate->add_duration(DateTime::Duration->new(days => 3)); >+$resdate = dt_from_string(); >+$resdate->add_duration( DateTime::Duration->new( days => 3 ) ); > AddReserve( > { >- branchcode => $branch_1, >- borrowernumber => $borrowernumber, >- biblionumber => $bibnum, >- priority => 1, >+ branchcode => $branch_1, >+ borrowernumber => $borrowernumber, >+ biblionumber => $bibnum, >+ priority => 1, > reservation_date => $resdate, > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item, 3 ); >-is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days'); >-$dbh->do('DELETE FROM reserves', undef, ($bibnum)); >+($status) = CheckReserves( $item, 3 ); >+is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days' ); >+$dbh->do( 'DELETE FROM reserves', undef, ($bibnum) ); > > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >@@ -774,63 +846,73 @@ subtest '_koha_notify_reserve() tests' => sub { > > plan tests => 3; > >- my $branch = $builder->build_object({ >- class => 'Koha::Libraries', >- value => { >- branchemail => 'branch@e.mail', >- branchreplyto => 'branch@reply.to', >- pickup_location => 1 >+ my $branch = $builder->build_object( >+ { >+ class => 'Koha::Libraries', >+ value => { >+ branchemail => 'branch@e.mail', >+ branchreplyto => 'branch@reply.to', >+ pickup_location => 1 >+ } > } >- }); >- my $item = $builder->build_sample_item({ >- homebranch => $branch->branchcode, >- holdingbranch => $branch->branchcode >- }); >+ ); >+ my $item = $builder->build_sample_item( >+ { >+ homebranch => $branch->branchcode, >+ holdingbranch => $branch->branchcode >+ } >+ ); > > my $wants_hold_and_email = { > wants_digest => '0', >- transports => { >- sms => 'HOLD', >+ transports => { >+ sms => 'HOLD', > email => 'HOLD', >- }, >+ }, > letter_code => 'HOLD' > }; > >- my $mp = Test::MockModule->new( 'C4::Members::Messaging' ); >+ my $mp = Test::MockModule->new('C4::Members::Messaging'); > >- $mp->mock("GetMessagingPreferences",$wants_hold_and_email); >+ $mp->mock( "GetMessagingPreferences", $wants_hold_and_email ); > > $dbh->do('DELETE FROM letter'); > >- my $email_hold_notice = $builder->build({ >+ my $email_hold_notice = $builder->build( >+ { > source => 'Letter', >- value => { >+ value => { > message_transport_type => 'email', >- branchcode => '', >- code => 'HOLD', >- module => 'reserves', >- lang => 'default', >+ branchcode => '', >+ code => 'HOLD', >+ module => 'reserves', >+ lang => 'default', > } >- }); >+ } >+ ); > >- my $sms_hold_notice = $builder->build({ >+ my $sms_hold_notice = $builder->build( >+ { > source => 'Letter', >- value => { >+ value => { > message_transport_type => 'sms', >- branchcode => '', >- code => 'HOLD', >- module => 'reserves', >- lang=>'default', >+ branchcode => '', >+ code => 'HOLD', >+ module => 'reserves', >+ lang => 'default', > } >- }); >+ } >+ ); > >- my $hold_borrower = $builder->build({ >+ my $hold_borrower = $builder->build( >+ { > source => 'Borrower', >- value => { >- smsalertnumber=>'5555555555', >- email=>'a@b.com', >+ value => { >+ smsalertnumber => '5555555555', >+ email => 'a@b.com', > } >- })->{borrowernumber}; >+ } >+ )->{borrowernumber}; > > C4::Reserves::AddReserve( > { >@@ -840,42 +922,49 @@ subtest '_koha_notify_reserve() tests' => sub { > } > ); > >- ModReserveAffect($item->itemnumber, $hold_borrower, 0); >- my $sms_message_address = $schema->resultset('MessageQueue')->search({ >- letter_code => 'HOLD', >+ ModReserveAffect( $item->itemnumber, $hold_borrower, 0 ); >+ my $sms_message_address = $schema->resultset('MessageQueue')->search( >+ { >+ letter_code => 'HOLD', > message_transport_type => 'sms', >- borrowernumber => $hold_borrower, >- })->next()->to_address(); >- is($sms_message_address, undef ,"We should not populate the sms message with the sms number, sending will do so"); >+ borrowernumber => $hold_borrower, >+ } >+ )->next()->to_address(); >+ is( $sms_message_address, undef, "We should not populate the sms message with the sms number, sending will do so" ); > >- my $email = $schema->resultset('MessageQueue')->search({ >- letter_code => 'HOLD', >+ my $email = $schema->resultset('MessageQueue')->search( >+ { >+ letter_code => 'HOLD', > message_transport_type => 'email', >- borrowernumber => $hold_borrower, >- })->next(); >+ borrowernumber => $hold_borrower, >+ } >+ )->next(); > my $email_to_address = $email->to_address(); >- is($email_to_address, undef ,"We should not populate the hold message with the email address, sending will do so"); >+ is( >+ $email_to_address, undef, >+ "We should not populate the hold message with the email address, sending will do so" >+ ); > my $email_from_address = $email->from_address(); >- is($email_from_address,'branch@e.mail',"Library's from address is used for sending"); >+ is( $email_from_address, 'branch@e.mail', "Library's from address is used for sending" ); > > }; > > subtest 'ReservesNeedReturns' => sub { > plan tests => 18; > >- my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $item_info = { >- homebranch => $library->branchcode, >- holdingbranch => $library->branchcode, >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $item_info = { >+ homebranch => $library->branchcode, >+ holdingbranch => $library->branchcode, > }; >- my $item = $builder->build_sample_item($item_info); >- my $patron = $builder->build_object( >+ my $item = $builder->build_sample_item($item_info); >+ my $patron = $builder->build_object( > { > class => 'Koha::Patrons', > value => { branchcode => $library->branchcode, } > } > ); >- my $patron_2 = $builder->build_object( >+ my $patron_2 = $builder->build_object( > { > class => 'Koha::Patrons', > value => { branchcode => $library->branchcode, } >@@ -884,16 +973,17 @@ subtest 'ReservesNeedReturns' => sub { > > my $priority = 1; > >- t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Test with feature disabled >+ t::lib::Mocks::mock_preference( 'ReservesNeedReturns', 1 ); # Test with feature disabled > my $hold = place_item_hold( $patron, $item, $library, $priority ); > is( $hold->priority, $priority, 'If ReservesNeedReturns is 1, priority must not have been set to changed' ); >- is( $hold->found, undef, 'If ReservesNeedReturns is 1, found must not have been set waiting' ); >+ is( $hold->found, undef, 'If ReservesNeedReturns is 1, found must not have been set waiting' ); > $hold->delete; > >- t::lib::Mocks::mock_preference('ReservesNeedReturns', 0); # '0' means 'Automatically mark a hold as found and waiting' >+ t::lib::Mocks::mock_preference( 'ReservesNeedReturns', 0 ) >+ ; # '0' means 'Automatically mark a hold as found and waiting' > $hold = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); >- is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); >+ is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); >+ is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); > $hold->delete; > > $item->onloan('2010-01-01')->store; >@@ -901,63 +991,71 @@ subtest 'ReservesNeedReturns' => sub { > is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item onloan priority must be set to 1' ); > $hold->delete; > >- t::lib::Mocks::mock_preference('AllowHoldsOnDamagedItems', 0); # '0' means damaged holds not allowed >+ t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); # '0' means damaged holds not allowed > $item->onloan(undef)->damaged(1)->store; > $hold = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item damaged and not allowed holds on damaged items priority must be set to 1' ); >+ is( >+ $hold->priority, 1, >+ 'If ReservesNeedReturns is 0 but item damaged and not allowed holds on damaged items priority must be set to 1' >+ ); > $hold->delete; >- t::lib::Mocks::mock_preference('AllowHoldsOnDamagedItems', 1); # '0' means damaged holds not allowed >+ t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 ); # '0' means damaged holds not allowed > $hold = place_item_hold( $patron, $item, $library, $priority ); > is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and damaged holds allowed, priority must have been set to 0' ); >- is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and damaged holds allowed, found must have been set waiting' ); >+ is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and damaged holds allowed, found must have been set waiting' ); > $hold->delete; > > my $hold_1 = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold_1->found, 'W', 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); >- is( $hold_1->priority, 0, 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); >+ is( $hold_1->found, 'W', 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); >+ is( $hold_1->priority, 0, 'First hold on item is set to waiting with ReservesNeedReturns set to 0' ); > $hold = place_item_hold( $patron_2, $item, $library, $priority ); > is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item already on hold priority must be set to 1' ); > $hold->delete; > $hold_1->delete; > >- my $transfer = $builder->build_object({ >- class => "Koha::Item::Transfers", >- value => { >- itemnumber => $item->itemnumber, >- datearrived => undef, >- datecancelled => undef >+ my $transfer = $builder->build_object( >+ { >+ class => "Koha::Item::Transfers", >+ value => { >+ itemnumber => $item->itemnumber, >+ datearrived => undef, >+ datecancelled => undef >+ } > } >- }); >+ ); > $item->damaged(0)->store; > $hold = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold->found, undef, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); >- is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); >+ is( $hold->found, undef, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); >+ is( $hold->priority, 1, 'If ReservesNeedReturns is 0 but item in transit the hold must not be set to waiting' ); > $hold->delete; > $transfer->delete; > > $hold = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); >- is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); >+ is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and no other status, priority must have been set to 0' ); >+ is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and no other status, found must have been set waiting' ); > $hold_1 = place_item_hold( $patron, $item, $library, $priority ); > is( $hold_1->priority, 1, 'If ReservesNeedReturns is 0 but item has a hold priority is 1' ); >- $hold_1->suspend(1)->store; # We suspend the hold >- $hold->delete; # Delete the waiting hold >+ $hold_1->suspend(1)->store; # We suspend the hold >+ $hold->delete; # Delete the waiting hold > $hold = place_item_hold( $patron, $item, $library, $priority ); >- is( $hold->priority, 0, 'If ReservesNeedReturns is 0 and other hold(s) suspended, priority must have been set to 0' ); >- is( $hold->found, 'W', 'If ReservesNeedReturns is 0 and other hold(s) suspended, found must have been set waiting' ); >- >- >- >+ is( >+ $hold->priority, 0, >+ 'If ReservesNeedReturns is 0 and other hold(s) suspended, priority must have been set to 0' >+ ); >+ is( >+ $hold->found, 'W', >+ 'If ReservesNeedReturns is 0 and other hold(s) suspended, found must have been set waiting' >+ ); > >- t::lib::Mocks::mock_preference('ReservesNeedReturns', 1); # Don't affect other tests >+ t::lib::Mocks::mock_preference( 'ReservesNeedReturns', 1 ); # Don't affect other tests > }; > > subtest 'ChargeReserveFee tests' => sub { > > plan tests => 8; > >- my $library = $builder->build_object({ class => 'Koha::Libraries' }); >- my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); > > my $fee = 20; > my $title = 'A title'; >@@ -967,21 +1065,21 @@ subtest 'ChargeReserveFee tests' => sub { > > my $line = C4::Reserves::ChargeReserveFee( $patron->id, $fee, $title ); > >- is( ref($line), 'Koha::Account::Line' , 'Returns a Koha::Account::Line object'); >+ is( ref($line), 'Koha::Account::Line', 'Returns a Koha::Account::Line object' ); > ok( $line->is_debit, 'Generates a debit line' ); >- is( $line->debit_type_code, 'RESERVE' , 'generates RESERVE debit_type'); >- is( $line->borrowernumber, $patron->id , 'generated line belongs to the passed patron'); >- is( $line->amount, $fee , 'amount set correctly'); >- is( $line->amountoutstanding, $fee , 'amountoutstanding set correctly'); >- is( $line->description, "$title" , 'description is title of reserved item'); >- is( $line->branchcode, $library->id , "Library id is picked from userenv and stored correctly" ); >+ is( $line->debit_type_code, 'RESERVE', 'generates RESERVE debit_type' ); >+ is( $line->borrowernumber, $patron->id, 'generated line belongs to the passed patron' ); >+ is( $line->amount, $fee, 'amount set correctly' ); >+ is( $line->amountoutstanding, $fee, 'amountoutstanding set correctly' ); >+ is( $line->description, "$title", 'description is title of reserved item' ); >+ is( $line->branchcode, $library->id, "Library id is picked from userenv and stored correctly" ); > }; > > subtest 'reserves.item_level_hold' => sub { > plan tests => 4; > > my $item = $builder->build_sample_item; >- my $patron = $builder->build_object({ class => "Koha::Patrons" }); >+ my $patron = $builder->build_object( { class => "Koha::Patrons" } ); > $patron->branchcode( $item->homebranch ); > > subtest 'item level hold' => sub { >@@ -1003,14 +1101,17 @@ subtest 'reserves.item_level_hold' => sub { > ModReserveAffect( $item->itemnumber, $patron->id, 1 ); > > my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); >- $mock->mock( 'enqueue', sub { >- my ( $self, $args ) = @_; >- is_deeply( >- $args->{biblio_ids}, >- [ $hold->biblionumber ], >- "AlterPriority triggers a holds queue update for the related biblio" >- ); >- } ); >+ $mock->mock( >+ 'enqueue', >+ sub { >+ my ( $self, $args ) = @_; >+ is_deeply( >+ $args->{biblio_ids}, >+ [ $hold->biblionumber ], >+ "AlterPriority triggers a holds queue update for the related biblio" >+ ); >+ } >+ ); > > t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 1 ); > t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); >@@ -1060,7 +1161,10 @@ subtest 'reserves.item_level_hold' => sub { > ); > > my $hold = Koha::Holds->find($reserve_id); >- is( $hold->item_level_hold, 0, 'item_level_hold should not be set when AddReserve is called without a specific item' ); >+ is( >+ $hold->item_level_hold, 0, >+ 'item_level_hold should not be set when AddReserve is called without a specific item' >+ ); > > # Mark it waiting > ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 1 ); >@@ -1087,21 +1191,25 @@ subtest 'reserves.item_level_hold' => sub { > itemtype => undef, > rules => { > reservesallowed => 25, >- opacitemholds => 'F', >+ opacitemholds => 'F', > } > } > ); > > my $canreserve = C4::Reserves::CanBookBeReserved( $patron->id, $item->biblionumber ); > >- is( $canreserve->{status}, >+ is( >+ $canreserve->{status}, > 'recordHoldNotAllowed', >- 'record-level holds should not be possible with opacitemholds set to "Force"' ); >+ 'record-level holds should not be possible with opacitemholds set to "Force"' >+ ); > > $canreserve = C4::Reserves::CanItemBeReserved( $patron, $item ); > >- is( $canreserve->{status}, 'OK', >- 'item-level holds should be possible with opacitemholds set to "Force"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'item-level holds should be possible with opacitemholds set to "Force"' >+ ); > > Koha::CirculationRules->set_rules( > { >@@ -1110,20 +1218,24 @@ subtest 'reserves.item_level_hold' => sub { > itemtype => undef, > rules => { > reservesallowed => 25, >- opacitemholds => 'N', >+ opacitemholds => 'N', > } > } >- ); >+ ); > > $canreserve = C4::Reserves::CanBookBeReserved( $patron->id, $item->biblionumber ); > >- is( $canreserve->{status}, 'OK', >- 'record-level holds should be possible with opacitemholds set to "No"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'record-level holds should be possible with opacitemholds set to "No"' >+ ); > > $canreserve = C4::Reserves::CanItemBeReserved( $patron, $item ); > >- is( $canreserve->{status}, 'recordHoldsOnly', >- 'item-level holds should not be possible with opacitemholds set to "No"' ); >+ is( >+ $canreserve->{status}, 'recordHoldsOnly', >+ 'item-level holds should not be possible with opacitemholds set to "No"' >+ ); > > Koha::CirculationRules->set_rules( > { >@@ -1132,20 +1244,24 @@ subtest 'reserves.item_level_hold' => sub { > itemtype => undef, > rules => { > reservesallowed => 25, >- opacitemholds => 'Y', >+ opacitemholds => 'Y', > } > } >- ); >+ ); > > $canreserve = C4::Reserves::CanBookBeReserved( $patron->id, $item->biblionumber ); > >- is( $canreserve->{status}, 'OK', >- 'record-level holds should be possible with opacitemholds set to "Yes"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'record-level holds should be possible with opacitemholds set to "Yes"' >+ ); > > $canreserve = C4::Reserves::CanItemBeReserved( $patron, $item ); > >- is( $canreserve->{status}, 'OK', >- 'item-level holds should be possible with opacitemholds set to "Yes"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'item-level holds should be possible with opacitemholds set to "Yes"' >+ ); > }; > > subtest 'test opacitemholds rules in staff context' => sub { >@@ -1159,30 +1275,34 @@ subtest 'reserves.item_level_hold' => sub { > itemtype => undef, > rules => { > reservesallowed => 25, >- opacitemholds => 'N', >+ opacitemholds => 'N', > } > } > ); > > my $canreserve = C4::Reserves::CanBookBeReserved( $patron->id, $item->biblionumber ); > >- is( $canreserve->{status}, 'OK', >- 'record-level holds should be possible with opacitemholds set to "No"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'record-level holds should be possible with opacitemholds set to "No"' >+ ); > > $canreserve = C4::Reserves::CanItemBeReserved( $patron, $item ); > >- is( $canreserve->{status}, 'OK', >- 'item-level holds should still be possible in staff context, even with opacitemholds set to "No"' ); >+ is( >+ $canreserve->{status}, 'OK', >+ 'item-level holds should still be possible in staff context, even with opacitemholds set to "No"' >+ ); > }; > > Koha::CirculationRules->set_rules( > { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rules => { >- opacitemholds => undef, >- } >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rules => { >+ opacitemholds => undef, >+ } > } > ); > }; >@@ -1193,11 +1313,13 @@ subtest 'MoveReserve additional test' => sub { > > # Create the items and patrons we need > my $biblio = $builder->build_sample_biblio(); >- my $itype = $builder->build_object({ class => "Koha::ItemTypes", value => { notforloan => 0 } }); >- my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber,notforloan => 0, itype => $itype->itemtype }); >- my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, notforloan => 0, itype => $itype->itemtype }); >- my $patron_1 = $builder->build_object({ class => "Koha::Patrons" }); >- my $patron_2 = $builder->build_object({ class => "Koha::Patrons" }); >+ my $itype = $builder->build_object( { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >+ my $item_1 = $builder->build_sample_item( >+ { biblionumber => $biblio->biblionumber, notforloan => 0, itype => $itype->itemtype } ); >+ my $item_2 = $builder->build_sample_item( >+ { biblionumber => $biblio->biblionumber, notforloan => 0, itype => $itype->itemtype } ); >+ my $patron_1 = $builder->build_object( { class => "Koha::Patrons" } ); >+ my $patron_2 = $builder->build_object( { class => "Koha::Patrons" } ); > > # Place a hold on the title for both patrons > my $reserve_1 = AddReserve( >@@ -1218,16 +1340,22 @@ subtest 'MoveReserve additional test' => sub { > itemnumber => $item_1->itemnumber, > } > ); >- is($patron_1->holds->next()->reserve_id, $reserve_1, "The 1st patron has a hold"); >- is($patron_2->holds->next()->reserve_id, $reserve_2, "The 2nd patron has a hold"); >+ is( $patron_1->holds->next()->reserve_id, $reserve_1, "The 1st patron has a hold" ); >+ is( $patron_2->holds->next()->reserve_id, $reserve_2, "The 2nd patron has a hold" ); > > # Fake the holds queue >- $dbh->do(q{INSERT INTO hold_fill_targets VALUES (?, ?, ?, ?, ?,?)},undef,($patron_1->borrowernumber,$biblio->biblionumber,$item_1->itemnumber,$item_1->homebranch,0,$reserve_1)); >+ $dbh->do( >+ q{INSERT INTO hold_fill_targets VALUES (?, ?, ?, ?, ?,?)}, undef, >+ ( $patron_1->borrowernumber, $biblio->biblionumber, $item_1->itemnumber, $item_1->homebranch, 0, $reserve_1 ) >+ ); > > # The 2nd hold should be filed even if the item is preselected for the first hold >- MoveReserve($item_1->itemnumber,$patron_2->borrowernumber); >- is($patron_2->holds->count, 0, "The 2nd patrons no longer has a hold"); >- is($patron_2->old_holds->next()->reserve_id, $reserve_2, "The 2nd patrons hold was filled and moved to old holds"); >+ MoveReserve( $item_1->itemnumber, $patron_2->borrowernumber ); >+ is( $patron_2->holds->count, 0, "The 2nd patrons no longer has a hold" ); >+ is( >+ $patron_2->old_holds->next()->reserve_id, $reserve_2, >+ "The 2nd patrons hold was filled and moved to old holds" >+ ); > > }; > >@@ -1238,9 +1366,8 @@ subtest 'RevertWaitingStatus' => sub { > # Create the items and patrons we need > my $biblio = $builder->build_sample_biblio(); > my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $itype = $builder->build_object( >- { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >- my $item_1 = $builder->build_sample_item( >+ my $itype = $builder->build_object( { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >+ my $item_1 = $builder->build_sample_item( > { > biblionumber => $biblio->biblionumber, > itype => $itype->itemtype, >@@ -1254,10 +1381,10 @@ subtest 'RevertWaitingStatus' => sub { > > # Place a hold on the title for both patrons > my $priority = 1; >- my $hold_1 = place_item_hold( $patron_1, $item_1, $library, $priority ); >- my $hold_2 = place_item_hold( $patron_2, $item_1, $library, $priority ); >- my $hold_3 = place_item_hold( $patron_3, $item_1, $library, $priority ); >- my $hold_4 = place_item_hold( $patron_4, $item_1, $library, $priority ); >+ my $hold_1 = place_item_hold( $patron_1, $item_1, $library, $priority ); >+ my $hold_2 = place_item_hold( $patron_2, $item_1, $library, $priority ); >+ my $hold_3 = place_item_hold( $patron_3, $item_1, $library, $priority ); >+ my $hold_4 = place_item_hold( $patron_4, $item_1, $library, $priority ); > > $hold_1->set_waiting; > AddIssue( $patron_3, $item_1->barcode, undef, 'revert' ); >@@ -1278,7 +1405,7 @@ subtest 'CheckReserves additional tests' => sub { > > plan tests => 8; > >- my $item = $builder->build_sample_item; >+ my $item = $builder->build_sample_item; > my $reserve1 = $builder->build_object( > { > class => "Koha::Holds", >@@ -1340,40 +1467,47 @@ subtest 'CheckReserves additional tests' => sub { > } > ); > >- ModReserveAffect( $item->itemnumber, $reserve1->borrowernumber, 1, >- $reserve1->reserve_id ); >- my ( $status, $matched_reserve, $possible_reserves ) = >- CheckReserves( $item ); >+ ModReserveAffect( >+ $item->itemnumber, $reserve1->borrowernumber, 1, >+ $reserve1->reserve_id >+ ); >+ my ( $status, $matched_reserve, $possible_reserves ) = CheckReserves($item); > > is( $status, 'Transferred', "We found a reserve" ); >- is( $matched_reserve->{reserve_id}, >- $reserve1->reserve_id, "We got the Transit reserve" ); >+ is( >+ $matched_reserve->{reserve_id}, >+ $reserve1->reserve_id, "We got the Transit reserve" >+ ); > is( scalar @$possible_reserves, 2, 'We do get both reserves' ); > >- my $patron_B = $builder->build_object({ class => "Koha::Patrons" }); >- my $item_A = $builder->build_sample_item; >- my $item_B = $builder->build_sample_item({ >- homebranch => $patron_B->branchcode, >- biblionumber => $item_A->biblionumber, >- itype => $item_A->itype >- }); >+ my $patron_B = $builder->build_object( { class => "Koha::Patrons" } ); >+ my $item_A = $builder->build_sample_item; >+ my $item_B = $builder->build_sample_item( >+ { >+ homebranch => $patron_B->branchcode, >+ biblionumber => $item_A->biblionumber, >+ itype => $item_A->itype >+ } >+ ); > Koha::CirculationRules->set_rules( > { > branchcode => undef, > categorycode => undef, > itemtype => $item_A->itype, > rules => { >- reservesallowed => 25, >+ reservesallowed => 25, > holds_per_record => 1, > } > } > ); >- Koha::CirculationRules->set_rule({ >- branchcode => undef, >- itemtype => $item_A->itype, >- rule_name => 'holdallowed', >- rule_value => 'from_home_library' >- }); >+ Koha::CirculationRules->set_rule( >+ { >+ branchcode => undef, >+ itemtype => $item_A->itype, >+ rule_name => 'holdallowed', >+ rule_value => 'from_home_library' >+ } >+ ); > my $reserve_id = AddReserve( > { > branchcode => $patron_B->branchcode, >@@ -1384,32 +1518,35 @@ subtest 'CheckReserves additional tests' => sub { > } > ); > >- ok( $reserve_id, "We can place a record level hold because one item is owned by patron's home library"); >- t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); >- is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); >- Koha::CirculationRules->set_rule({ >- branchcode => $item_A->homebranch, >- itemtype => $item_A->itype, >- rule_name => 'holdallowed', >- rule_value => 'from_any_library' >- }); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); >- is( $status, "Reserved", "We fill the hold with item A because item's branch rule says allow any"); >- >+ ok( $reserve_id, "We can place a record level hold because one item is owned by patron's home library" ); >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves($item_A); >+ is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch" ); >+ Koha::CirculationRules->set_rule( >+ { >+ branchcode => $item_A->homebranch, >+ itemtype => $item_A->itype, >+ rule_name => 'holdallowed', >+ rule_value => 'from_any_library' >+ } >+ ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves($item_A); >+ is( $status, "Reserved", "We fill the hold with item A because item's branch rule says allow any" ); > > # Changing the control branch should change only the rule we get >- t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); >- is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); >- Koha::CirculationRules->set_rule({ >- branchcode => $patron_B->branchcode, >- itemtype => $item_A->itype, >- rule_name => 'holdallowed', >- rule_value => 'from_any_library' >- }); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); >- is( $status, "Reserved", "We fill the hold with item A because patron's branch rule says allow any"); >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves($item_A); >+ is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch" ); >+ Koha::CirculationRules->set_rule( >+ { >+ branchcode => $patron_B->branchcode, >+ itemtype => $item_A->itype, >+ rule_name => 'holdallowed', >+ rule_value => 'from_any_library' >+ } >+ ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves($item_A); >+ is( $status, "Reserved", "We fill the hold with item A because patron's branch rule says allow any" ); > > }; > >@@ -1419,46 +1556,64 @@ subtest 'AllowHoldOnPatronPossession test' => sub { > > # Create the items and patrons we need > my $biblio = $builder->build_sample_biblio(); >- my $itype = $builder->build_object({ class => "Koha::ItemTypes", value => { notforloan => 0 } }); >- my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber,notforloan => 0, itype => $itype->itemtype }); >- my $patron = $builder->build_object({ class => "Koha::Patrons", >- value => { branchcode => $item->homebranch }}); >- >- C4::Circulation::AddIssue($patron, >- $item->barcode); >- t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 0); >- >- is(C4::Reserves::CanBookBeReserved($patron->borrowernumber, >- $item->biblionumber)->{status}, >- 'alreadypossession', >- 'Patron cannot place hold on a book loaned to itself'); >- >- is(C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, >- 'alreadypossession', >- 'Patron cannot place hold on an item loaned to itself'); >- >- t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 1); >- >- is(C4::Reserves::CanBookBeReserved($patron->borrowernumber, >- $item->biblionumber)->{status}, >- 'OK', >- 'Patron can place hold on a book loaned to itself'); >- >- is(C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, >- 'OK', >- 'Patron can place hold on an item loaned to itself'); >+ my $itype = $builder->build_object( { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >+ my $item = $builder->build_sample_item( >+ { biblionumber => $biblio->biblionumber, notforloan => 0, itype => $itype->itemtype } ); >+ my $patron = $builder->build_object( >+ { >+ class => "Koha::Patrons", >+ value => { branchcode => $item->homebranch } >+ } >+ ); >+ >+ C4::Circulation::AddIssue( >+ $patron, >+ $item->barcode >+ ); >+ t::lib::Mocks::mock_preference( 'AllowHoldsOnPatronsPossessions', 0 ); >+ >+ is( >+ C4::Reserves::CanBookBeReserved( >+ $patron->borrowernumber, >+ $item->biblionumber >+ )->{status}, >+ 'alreadypossession', >+ 'Patron cannot place hold on a book loaned to itself' >+ ); >+ >+ is( >+ C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, >+ 'alreadypossession', >+ 'Patron cannot place hold on an item loaned to itself' >+ ); >+ >+ t::lib::Mocks::mock_preference( 'AllowHoldsOnPatronsPossessions', 1 ); >+ >+ is( >+ C4::Reserves::CanBookBeReserved( >+ $patron->borrowernumber, >+ $item->biblionumber >+ )->{status}, >+ 'OK', >+ 'Patron can place hold on a book loaned to itself' >+ ); >+ >+ is( >+ C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, >+ 'OK', >+ 'Patron can place hold on an item loaned to itself' >+ ); > }; > > subtest 'MergeHolds' => sub { > > plan tests => 1; > >- my $biblio_1 = $builder->build_sample_biblio(); >- my $biblio_2 = $builder->build_sample_biblio(); >- my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $itype = $builder->build_object( >- { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >- my $item_1 = $builder->build_sample_item( >+ my $biblio_1 = $builder->build_sample_biblio(); >+ my $biblio_2 = $builder->build_sample_biblio(); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $itype = $builder->build_object( { class => "Koha::ItemTypes", value => { notforloan => 0 } } ); >+ my $item_1 = $builder->build_sample_item( > { > biblionumber => $biblio_1->biblionumber, > itype => $itype->itemtype, >@@ -1472,7 +1627,7 @@ subtest 'MergeHolds' => sub { > place_item_hold( $patron_1, $item_1, $library, $priority ); > > # Move and make sure hold is now on $biblio_2 >- C4::Reserves::MergeHolds($dbh, $biblio_2->biblionumber, $biblio_1->biblionumber); >+ C4::Reserves::MergeHolds( $dbh, $biblio_2->biblionumber, $biblio_1->biblionumber ); > is( $biblio_2->holds->count, 1, 'Hold has been transferred' ); > }; > >@@ -1480,7 +1635,7 @@ subtest 'ModReserveAffect logging' => sub { > > plan tests => 4; > >- my $item = $builder->build_sample_item; >+ my $item = $builder->build_sample_item; > my $patron = $builder->build_object( > { > class => "Koha::Patrons", >@@ -1488,8 +1643,8 @@ subtest 'ModReserveAffect logging' => sub { > } > ); > >- t::lib::Mocks::mock_userenv({ patron => $patron }); >- t::lib::Mocks::mock_preference('HoldsLog', 1); >+ t::lib::Mocks::mock_userenv( { patron => $patron } ); >+ t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); > > my $reserve_id = AddReserve( > { >@@ -1501,7 +1656,7 @@ subtest 'ModReserveAffect logging' => sub { > } > ); > >- my $hold = Koha::Holds->find($reserve_id); >+ my $hold = Koha::Holds->find($reserve_id); > my $previous_timestamp = '1970-01-01 12:34:56'; > $hold->timestamp($previous_timestamp)->store; > >@@ -1519,23 +1674,25 @@ subtest 'ModReserveAffect logging' => sub { > ok( $hold->is_waiting, 'Hold has been set waiting' ); > isnt( $hold->timestamp, $previous_timestamp, 'The timestamp has been modified' ); > >- my $log = Koha::ActionLogs->search({ module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id })->next; >+ my $log = Koha::ActionLogs->search( { module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id } )->next; > my $expected = sprintf q{'timestamp' => '%s'}, $hold->timestamp; > like( $log->info, qr{$expected}, 'Timestamp logged is the current one' ); > }; > > sub count_hold_print_messages { >- my $message_count = $dbh->selectall_arrayref(q{ >+ my $message_count = $dbh->selectall_arrayref( >+ q{ > SELECT COUNT(*) > FROM message_queue > WHERE letter_code = 'HOLD' > AND message_transport_type = 'print' >- }); >+ } >+ ); > return $message_count->[0]->[0]; > } > > sub place_item_hold { >- my ($patron,$item,$library,$priority) = @_; >+ my ( $patron, $item, $library, $priority ) = @_; > > my $hold_id = C4::Reserves::AddReserve( > { >@@ -1620,14 +1777,17 @@ subtest 'AddReserve() tests' => sub { > my $biblio = $builder->build_sample_biblio; > > my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); >- $mock->mock( 'enqueue', sub { >- my ( $self, $args ) = @_; >- is_deeply( >- $args->{biblio_ids}, >- [ $biblio->id ], >- "AddReserve triggers a holds queue update for the related biblio" >- ); >- } ); >+ $mock->mock( >+ 'enqueue', >+ sub { >+ my ( $self, $args ) = @_; >+ is_deeply( >+ $args->{biblio_ids}, >+ [ $biblio->id ], >+ "AddReserve triggers a holds queue update for the related biblio" >+ ); >+ } >+ ); > > t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 1 ); > >@@ -1661,11 +1821,11 @@ subtest 'AlterPriorty() tests' => sub { > > $schema->storage->txn_begin; > >- my $library = $builder->build_object({ class => 'Koha::Libraries' }); >- my $patron_1 = $builder->build_object({ class => 'Koha::Patrons' }); >- my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); >- my $patron_3 = $builder->build_object({ class => 'Koha::Patrons' }); >- my $biblio = $builder->build_sample_biblio; >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $patron_2 = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $patron_3 = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $biblio = $builder->build_sample_biblio; > > my $reserve_id = AddReserve( > { >@@ -1690,14 +1850,17 @@ subtest 'AlterPriorty() tests' => sub { > ); > > my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); >- $mock->mock( 'enqueue', sub { >- my ( $self, $args ) = @_; >- is_deeply( >- $args->{biblio_ids}, >- [ $biblio->id ], >- "AlterPriority triggers a holds queue update for the related biblio" >- ); >- } ); >+ $mock->mock( >+ 'enqueue', >+ sub { >+ my ( $self, $args ) = @_; >+ is_deeply( >+ $args->{biblio_ids}, >+ [ $biblio->id ], >+ "AlterPriority triggers a holds queue update for the related biblio" >+ ); >+ } >+ ); > > t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 1 ); > >@@ -1705,7 +1868,7 @@ subtest 'AlterPriorty() tests' => sub { > > my $hold = Koha::Holds->find($reserve_id); > >- is($hold->priority,3,'Successfully altered priority to bottom'); >+ is( $hold->priority, 3, 'Successfully altered priority to bottom' ); > > t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 0 ); > >@@ -1720,16 +1883,13 @@ subtest 'CanBookBeReserved() tests' => sub { > > $schema->storage->txn_begin; > >- my $library = $builder->build_object( >- { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); >- my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >- my $itype = $builder->build_object( { class => 'Koha::ItemTypes' } ); >+ my $library = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $itype = $builder->build_object( { class => 'Koha::ItemTypes' } ); > > my $biblio = $builder->build_sample_biblio(); >- my $item_1 = $builder->build_sample_item( >- { biblionumber => $biblio->id, itype => $itype->id } ); >- my $item_2 = $builder->build_sample_item( >- { biblionumber => $biblio->id, itype => $itype->id } ); >+ my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->id, itype => $itype->id } ); >+ my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->id, itype => $itype->id } ); > > Koha::CirculationRules->delete; > Koha::CirculationRules->set_rules( >@@ -1765,10 +1925,14 @@ subtest 'CanBookBeReserved() tests' => sub { > > ## Limit on item type is 2, only one hold, success tests > >- my $res = CanBookBeReserved( $patron->id, $biblio->id, $library->id, >- { itemtype => $itype->id } ); >- is_deeply( $res, { status => 'OK' }, >- 'Holds on itemtype limit not reached' ); >+ my $res = CanBookBeReserved( >+ $patron->id, $biblio->id, $library->id, >+ { itemtype => $itype->id } >+ ); >+ is_deeply( >+ $res, { status => 'OK' }, >+ 'Holds on itemtype limit not reached' >+ ); > > # Add a second hold, biblio-level and item type-constrained > C4::Reserves::AddReserve( >@@ -1783,8 +1947,10 @@ subtest 'CanBookBeReserved() tests' => sub { > > ## Limit on item type is 2, two holds, one of them biblio-level/item type-constrained > >- $res = CanBookBeReserved( $patron->id, $biblio->id, $library->id, >- { itemtype => $itype->id } ); >+ $res = CanBookBeReserved( >+ $patron->id, $biblio->id, $library->id, >+ { itemtype => $itype->id } >+ ); > is_deeply( $res, { status => '' }, 'Holds on itemtype limit reached' ); > > $schema->storage->txn_rollback; >@@ -1801,12 +1967,13 @@ subtest 'CanItemBeReserved() tests' => sub { > my $itype = $builder->build_object( { class => 'Koha::ItemTypes' } ); > > my $biblio = $builder->build_sample_biblio(); >- my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->id, itype => $itype->id }); >- my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->id, itype => $itype->id }); >+ my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->id, itype => $itype->id } ); >+ my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->id, itype => $itype->id } ); > > Koha::CirculationRules->delete; > Koha::CirculationRules->set_rules( >- { branchcode => undef, >+ { >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rules => { >@@ -1815,7 +1982,8 @@ subtest 'CanItemBeReserved() tests' => sub { > } > ); > Koha::CirculationRules->set_rules( >- { branchcode => undef, >+ { >+ branchcode => undef, > categorycode => undef, > itemtype => $itype->id, > rules => { >@@ -1862,24 +2030,26 @@ subtest 'DefaultHoldExpiration tests' => sub { > plan tests => 2; > $schema->storage->txn_begin; > >- t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdate', 1 ); >+ t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdate', 1 ); > t::lib::Mocks::mock_preference( 'DefaultHoldExpirationdatePeriod', 365 ); > t::lib::Mocks::mock_preference( 'DefaultHoldExpirationUnitOfTime', 'days;' ); > >- my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >- my $item = $builder->build_sample_item(); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $item = $builder->build_sample_item(); > >- my $reserve_id = AddReserve({ >- branchcode => $item->homebranch, >- borrowernumber => $patron->id, >- biblionumber => $item->biblionumber, >- }); >+ my $reserve_id = AddReserve( >+ { >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->id, >+ biblionumber => $item->biblionumber, >+ } >+ ); > > my $today = dt_from_string(); >- my $hold = Koha::Holds->find( $reserve_id ); >+ my $hold = Koha::Holds->find($reserve_id); > >- is( $hold->reservedate, $today->ymd, "Hold created today" ); >- is( $hold->expirationdate, $today->add( days => 365)->ymd, "Reserve date set 1 year from today" ); >+ is( $hold->reservedate, $today->ymd, "Hold created today" ); >+ is( $hold->expirationdate, $today->add( days => 365 )->ymd, "Reserve date set 1 year from today" ); > > $schema->txn_rollback; > }; >@@ -1912,8 +2082,8 @@ subtest '_Findgroupreserves' => sub { > C4::HoldsQueue::AddToHoldTargetMap( > { > $item->id => { >- borrowernumber => $patron_1->id, biblionumber => $item->biblionumber, >- holdingbranch => $item->holdingbranch, item_level => 0, reserve_id => $reserve_id_1 >+ borrowernumber => $patron_1->id, biblionumber => $item->biblionumber, >+ holdingbranch => $item->holdingbranch, item_level => 0, reserve_id => $reserve_id_1 > } > } > ); >@@ -1951,8 +2121,6 @@ subtest '_Findgroupreserves' => sub { > is( scalar @reserves, 1, "We should still only get the item level hold that is in the map" ); > is( $reserves[0]->{reserve_id}, $reserve_id_1, "We got the expected reserve which has been updated" ); > >- >- > $schema->txn_rollback; > }; > >-- >2.30.2
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 25408
:
104488
|
104496
|
105988
|
105989
|
105990
|
106413
|
106414
|
106521
|
106535
|
106536
|
106537
|
107777
|
107778
|
107779
|
107780
|
111681
|
111682
|
111693
|
111696
|
111716
|
111717
|
111718
|
111719
|
111720
|
111721
|
115125
|
115126
|
115127
|
115128
|
115129
|
115130
|
121524
|
121525
|
121526
|
121527
|
121528
|
121530
|
128505
|
130180
|
130181
|
132658
|
132660
|
132661
|
164680
|
164681
|
164682
|
164684
|
164685
|
164686
|
164693
|
164694
|
164878
|
164879
|
165111
|
165112
|
165442
|
165443
|
165444
|
165445