From 67ebde22da6f259434e67c485c27ec106aa8d241 Mon Sep 17 00:00:00 2001 From: Kyle M Hall <kyle@bywatersolutions.com> Date: Fri, 8 Feb 2019 14:46:18 -0500 Subject: [PATCH] Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied. Test Plan: 1) Set up a branch transfer limit from Library A to Library B 2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding ) 3) Verify you cannot place that hold via ILS-DI 4) Verify you cannot place that hold via SIP 4) Verify a forced hold from Library A to Library B will not show up in the holds queue Signed-off-by: Liz Rea <wizzyrea@gmail.com> --- C4/HoldsQueue.pm | 11 +++ C4/ILSDI/Services.pm | 12 ++- C4/Reserves.pm | 40 +++++---- C4/SIP/ILS/Transaction/Hold.pm | 6 ++ .../prog/en/modules/reserve/request.tt | 16 +++- .../prog/js/circ-patron-search-results.js | 14 ++-- reserve/request.pl | 6 +- t/db_dependent/Holds/HoldFulfillmentPolicy.t | 20 ++++- t/db_dependent/HoldsQueue.t | 74 ++++++++++++++++- t/db_dependent/ILSDI_Services.t | 95 +++++++++++++++++++++- t/db_dependent/Reserves.t | 14 ++++ 11 files changed, 275 insertions(+), 33 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 1015763dd8..0028d798dc 100755 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -409,6 +409,8 @@ sub MapItemsToHoldRequests { || ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} ); + next unless Koha::Items->find( $item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ); + my $local_holds_priority_item_branchcode = $item->{$LocalHoldsPriorityItemControl}; @@ -463,6 +465,7 @@ sub MapItemsToHoldRequests { and ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) + and Koha::Items->find( $request->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ) ) { @@ -510,6 +513,8 @@ sub MapItemsToHoldRequests { my $holding_branch_items = $items_by_branch{$pickup_branch}; if ( $holding_branch_items ) { foreach my $item (@$holding_branch_items) { + next unless Koha::Items->find( $item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ); + if ( $request->{borrowerbranch} eq $item->{homebranch} && ( ( $item->{hold_fulfillment_policy} eq 'any' ) # Don't fill item level holds that contravene the hold pickup policy at this time @@ -532,6 +537,7 @@ sub MapItemsToHoldRequests { my $holding_branch_items = $items_by_branch{$holdingbranch}; foreach my $item (@$holding_branch_items) { next if $request->{borrowerbranch} ne $item->{homebranch}; + next unless Koha::Items->find( $item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraris->find( $request->{branchcode} ) } ); # Don't fill item level holds that contravene the hold pickup policy at this time next unless $item->{hold_fulfillment_policy} eq 'any' @@ -568,6 +574,7 @@ sub MapItemsToHoldRequests { foreach my $item (@$holding_branch_items) { next if $pickup_branch ne $item->{homebranch}; next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} ); + next unless Koha::Items->find( $item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ); # Don't fill item level holds that contravene the hold pickup policy at this time next unless $item->{hold_fulfillment_policy} eq 'any' @@ -596,6 +603,8 @@ sub MapItemsToHoldRequests { next unless ( !$request->{itemtype} || $current_item->{itype} eq $request->{itemtype} ); + next unless Koha::Items->find( $current_item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ); + $itemnumber = $current_item->{itemnumber}; last; # quit this loop as soon as we have a suitable item } @@ -620,6 +629,8 @@ sub MapItemsToHoldRequests { next unless ( !$request->{itemtype} || $item->{itype} eq $request->{itemtype} ); + next unless Koha::Items->find( $item->{itemnumber} )->can_be_transferred( { to => scalar Koha::Libraries->find( $request->{branchcode} ) } ); + $itemnumber = $item->{itemnumber}; $holdingbranch = $branch; last PULL_BRANCHES2; diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 2b291b82a4..a1e23cf5de 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -694,6 +694,10 @@ sub HoldTitle { $branch = $patron->branchcode; } + my $destination = Koha::Libraries->find($branch); + return { code => 'libraryNotPickupLocation' } unless $destination->pickup_location; + return { code => 'cannotBeTransferred' } unless $biblio->can_be_transferred({ to => $destination }); + # Add the reserve # $branch, $borrowernumber, $biblionumber, # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, @@ -757,10 +761,6 @@ sub HoldItem { # If the biblio does not match the item, return an error code return { code => 'RecordNotFound' } if $item->biblionumber ne $biblio->biblionumber; - # Check for item disponibility - my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber )->{status}; - return { code => $canitembereserved } unless $canitembereserved eq 'OK'; - # Pickup branch management my $branch; if ( $cgi->param('pickup_location') ) { @@ -770,6 +770,10 @@ sub HoldItem { $branch = $patron->branchcode; } + # Check for item disponibility + my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber, $branch )->{status}; + return { code => $canitembereserved } unless $canitembereserved eq 'OK'; + # Add the reserve # $branch, $borrowernumber, $biblionumber, # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 3617549582..48b4170a69 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -23,37 +23,38 @@ package C4::Reserves; use strict; #use warnings; FIXME - Bug 2505 -use C4::Context; +use C4::Accounts; use C4::Biblio; -use C4::Members; -use C4::Items; use C4::Circulation; -use C4::Accounts; +use C4::Context; +use C4::Items; +use C4::Members; # for _koha_notify_reserve -use C4::Members::Messaging; -use C4::Members qw(); use C4::Letters; use C4::Log; +use C4::Members qw(); +use C4::Members::Messaging; +use Koha::Account::Lines; use Koha::Biblios; -use Koha::DateUtils; use Koha::Calendar; +use Koha::CirculationRules; use Koha::Database; +use Koha::DateUtils; use Koha::Hold; -use Koha::Old::Hold; use Koha::Holds; -use Koha::Libraries; use Koha::IssuingRules; -use Koha::Items; use Koha::ItemTypes; +use Koha::Items; +use Koha::Libraries; +use Koha::Libraries; +use Koha::Old::Hold; use Koha::Patrons; -use Koha::CirculationRules; -use Koha::Account::Lines; -use List::MoreUtils qw( firstidx any ); use Carp; use Data::Dumper; +use List::MoreUtils qw( firstidx any ); use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -482,7 +483,7 @@ sub CanItemBeReserved { return { status => 'libraryNotPickupLocation' }; } unless ($item->can_be_transferred({ to => $destination })) { - return 'cannotBeTransferred'; + return { status => 'cannotBeTransferred' }; } } @@ -801,6 +802,7 @@ sub CheckReserves { next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode)); my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy}; next if ( ($branchitemrule->{hold_fulfillment_policy} ne 'any') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) ); + next unless $item->can_be_transferred( { to => scalar Koha::Libraries->find( $res->{branchcode} ) } ); $priority = $res->{'priority'}; $highest = $res; last if $local_hold_match; @@ -1140,7 +1142,7 @@ sub ModReserveMinusPriority { =head2 IsAvailableForItemLevelRequest - my $is_available = IsAvailableForItemLevelRequest($item_record,$borrower_record); + my $is_available = IsAvailableForItemLevelRequest( $item_record, $borrower_record, $pickup_branchcode ); Checks whether a given item record is available for an item-level hold request. An item is available if @@ -1165,6 +1167,7 @@ and canreservefromotherbranches. sub IsAvailableForItemLevelRequest { my $item = shift; my $borrower = shift; + my $pickup_branchcode = shift; my $dbh = C4::Context->dbh; # must check the notforloan setting of the itemtype @@ -1187,6 +1190,13 @@ sub IsAvailableForItemLevelRequest { my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item_object, patron => $patron } ); + if ($pickup_branchcode) { + my $destination = Koha::Libraries->find($pickup_branchcode); + return 0 unless $destination; + return 0 unless $destination->pickup_location; + return 0 unless $item_object->can_be_transferred( { to => $destination } ); + } + if ( $on_shelf_holds == 1 ) { return 1; } elsif ( $on_shelf_holds == 2 ) { diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm index d834516d10..63e1a06dbb 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/C4/SIP/ILS/Transaction/Hold.pm @@ -60,6 +60,12 @@ sub do_hold { $self->ok(0); return $self; } + unless ( $item->can_be_transferred( { to => scalar Koha::Libraries->find( $branch ) } ) ) { + $self->screen_msg('Item cannot be transferred.'); + $self->ok(0); + return $self; + } + AddReserve( $branch, $patron->borrowernumber, $item->biblionumber ); # unfortunately no meaningful return value diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index c0c7a07508..fd7571ebff 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -199,7 +199,7 @@ <li> <label for="pickup">Pickup at:</label> <select name="pickup" size="1" id="pickup"> - [% PROCESS options_for_libraries libraries => Branches.all({ search_params => { pickup_location => 1 } }) %] + [% PROCESS options_for_libraries libraries => Branches.all({ selected => pickup, search_params => { pickup_location => 1 } }) %] </select> </li> @@ -340,6 +340,8 @@ Patron is from different library [% ELSIF itemloo.not_holdable == 'itemAlreadyOnHold' %] Patron already has hold for this item + [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %] + Cannot be transferred to pickup library [% ELSE %] [% itemloo.not_holdable | html %] [% END %] @@ -661,6 +663,8 @@ [% INCLUDE 'columns_settings.inc' %] [% Asset.js("js/circ-patron-search-results.js") | $raw %] <script> + var biblionumber = "[% biblionumber %]"; + var borrowernumber = "[% patron.borrowernumber %]"; var MSG_CONFIRM_DELETE_HOLD = _("Are you sure you want to cancel this hold?"); var patron_homebranch = "[% Branches.GetName( patron.branchcode ) |replace("'", "\'") |replace('"', '\"') |replace('\n', '\\n') |replace('\r', '\\r') | html %]"; var override_items = {[% FOREACH bibitemloo IN bibitemloop %][% FOREACH itemloo IN bibitemloo.itemloop %][% IF ( itemloo.override ) %] @@ -686,6 +690,16 @@ ToggleHoldsToPlace(); }); + [% IF Koha.Preference('UseBranchTransferLimits') %] + $("#pickup").on('change', function(){ + var pickup = $("#pickup").val(); + var url = "?pickup=" + pickup; + url += "&borrowernumber=" + borrowernumber; + url += "&biblionumber=" + biblionumber; + window.location.replace(url); + }); + [% END %] + [% IF AutoResumeSuspendedHolds %] $(".suspend_until_datepicker, .datepickerfrom, .datepickerto").datepicker("option", "minDate", 1); [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/circ-patron-search-results.js b/koha-tmpl/intranet-tmpl/prog/js/circ-patron-search-results.js index fa44b3e96a..9f429b1de1 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/circ-patron-search-results.js +++ b/koha-tmpl/intranet-tmpl/prog/js/circ-patron-search-results.js @@ -3,11 +3,11 @@ $(document).ready(function() { $(".clickable").click(function() { window.document.location = $(this).data('url'); }); - var table = KohaTable("table_borrowers", - { - "aaSorting": [ 0, "asc" ], - "sDom": "t", - "iDisplayLength": -1 - }, - columns_settings_borrowers_table, null); +// var table = KohaTable("table_borrowers", +// { +// "aaSorting": [ 0, "asc" ], +// "sDom": "t", +// "iDisplayLength": -1 +// }, +// columns_settings_borrowers_table, null); }); diff --git a/reserve/request.pl b/reserve/request.pl index 2ef00c8b95..486c770c75 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -67,6 +67,7 @@ my ( $template, $borrowernumber, $cookie, $flags ) = get_template_and_user( ); my $showallitems = $input->param('showallitems'); +my $pickup = $input->param('pickup') || C4::Context->userenv->{'branch'}; my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; @@ -221,7 +222,7 @@ foreach my $biblionumber (@biblionumbers) { my $force_hold_level; if ( $patron ) { { # CanBookBeReserved - my $canReserve = CanBookBeReserved( $patron->borrowernumber, $biblionumber ); + my $canReserve = CanBookBeReserved( $patron->borrowernumber, $biblionumber, $pickup ); if ( $canReserve->{status} eq 'OK' ) { #All is OK and we can continue @@ -472,7 +473,7 @@ foreach my $biblionumber (@biblionumbers) { $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber ); + my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber, $pickup ); $item->{not_holdable} = $can_item_be_reserved->{status} unless ( $can_item_be_reserved->{status} eq 'OK' ); $item->{item_level_holds} = Koha::IssuingRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); @@ -645,6 +646,7 @@ $template->param( biblionumbers => $biblionumbers ); $template->param( exceeded_maxreserves => $exceeded_maxreserves ); $template->param( exceeded_holds_per_record => $exceeded_holds_per_record ); $template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber)); +$template->param( pickup => $pickup ); if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) { $template->param( reserve_in_future => 1 ); diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t index 34bb3eddfa..cfc725edf4 100755 --- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t +++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t @@ -4,9 +4,10 @@ use Modern::Perl; use C4::Context; -use Test::More tests => 10; +use Test::More tests => 11; use t::lib::TestBuilder; +use t::lib::Mocks; use Koha::Holds; BEGIN { @@ -137,3 +138,20 @@ $reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); ( $status ) = CheckReserves($itemnumber); is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" ); Koha::Holds->find( $reserve_id )->cancel; + +# Test enforement of branch transfer limits +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); +t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); +Koha::Holds->search()->delete(); +my ($item) = Koha::Biblios->find($biblionumber)->items; +my $limit = Koha::Item::Transfer::Limit->new( + { + toBranch => $library_C, + fromBranch => $item->holdingbranch, + itemtype => $item->effective_itemtype, + } +)->store(); +$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 ); +($status) = CheckReserves($itemnumber); +is( $status, '', "No hold where branch transfer is not allowed" ); +Koha::Holds->find($reserve_id)->cancel; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index e2f749a623..1dce46c892 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -8,7 +8,7 @@ use Modern::Perl; -use Test::More tests => 44; +use Test::More tests => 48; use Data::Dumper; use C4::Calendar; @@ -35,6 +35,9 @@ my $dbh = C4::Context->dbh; my $builder = t::lib::TestBuilder->new; +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); +t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); + my $library1 = $builder->build({ source => 'Branch', }); @@ -358,6 +361,49 @@ C4::HoldsQueue::CreateQueue(); $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's home library"); +### Test branch transfer limits ### +t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); +C4::Context->clear_syspref_cache(); +$dbh->do("DELETE FROM reserves"); +$sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); +$sth->execute( $borrower2->{borrowernumber}, $biblionumber, $branchcodes[1], 2 ); + +$dbh->do("DELETE FROM items"); +# barcode, homebranch, holdingbranch, itemtype +$items_insert_sth->execute( $barcode, $branchcodes[2], $branchcodes[2] ); +my $item = Koha::Items->find( { barcode => $barcode } ); + +my $limit1 = Koha::Item::Transfer::Limit->new( + { + toBranch => $branchcodes[0], + fromBranch => $branchcodes[2], + itemtype => $item->effective_itemtype, + } +)->store(); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, $borrower2->{cardnumber}, "Holds queue skips hold transfer that would violate branch transfer limits"); + +my $limit2 = Koha::Item::Transfer::Limit->new( + { + toBranch => $branchcodes[1], + fromBranch => $branchcodes[2], + itemtype => $item->effective_itemtype, + } +)->store(); + +C4::HoldsQueue::CreateQueue(); +$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); +is( $holds_queue->[0]->{cardnumber}, undef, "Holds queue doesn't fill hold where all available items would violate branch transfer limits"); + +$limit1->delete(); +$limit2->delete(); +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); +### END Test branch transfer limits ### + # Test holdingbranch = patron branch t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); @@ -706,7 +752,7 @@ $dbh->do("DELETE FROM default_branch_item_rules"); $dbh->do("DELETE FROM default_circ_rules"); $dbh->do("DELETE FROM branch_item_rules"); -my $item = Koha::Items->find( { biblionumber => $biblionumber } ); +$item = Koha::Items->find( { biblionumber => $biblionumber } ); $item->holdingbranch( $item->homebranch ); $item->store(); @@ -759,6 +805,30 @@ sub test_queue { diag( "Wrong pick-up/hold for first target (pick_branch, hold_branch, reserves, hold_fill_targets, tmp_holdsqueue): " . Dumper ($pick_branch, $hold_branch, map dump_records($_), qw(reserves hold_fill_targets tmp_holdsqueue)) ) unless $ok; + + # Test enforcement of branch transfer limit + if ( $r->{pickbranch} ne $r->{holdingbranch} ) { + t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); + my $limit = Koha::Item::Transfer::Limit->new( + { + toBranch => $r->{pickbranch}, + fromBranch => $r->{holdingbranch}, + itemtype => $r->{itype}, + } + )->store(); + C4::Context->clear_syspref_cache(); + C4::HoldsQueue::CreateQueue(); + $results = $dbh->selectall_arrayref( $test_sth, { Slice => {} } ) + ; # should be only one + my $s = $results->[0]; + isnt( $r->{holdingbranch}, $s->{holdingbranch}, 'Hold is not trapped for pickup at a branch that cannot be transferred to'); + + $limit->delete(); + t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); + C4::Context->clear_syspref_cache(); + C4::HoldsQueue::CreateQueue(); + } + } sub dump_records { diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index f3efde85b3..8d8db44d2d 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -19,7 +19,7 @@ use Modern::Perl; use CGI qw ( -utf8 ); -use Test::More tests => 5; +use Test::More tests => 6; use Test::MockModule; use t::lib::Mocks; use t::lib::TestBuilder; @@ -449,3 +449,96 @@ subtest 'Holds test' => sub { $schema->storage->txn_rollback; }; + +subtest 'Holds test for branch transfer limits' => sub { + + plan tests => 4; + + $schema->storage->txn_begin; + + # Test enforement of branch transfer limits + t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); + t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); + + my $patron = $builder->build({ + source => 'Borrower', + }); + + my $origin_branch = $builder->build( + { + source => 'Branch', + value => { + pickup_location => 1, + } + } + ); + my $pickup_branch = $builder->build( + { + source => 'Branch', + value => { + pickup_location => 1, + } + } + ); + + my $biblio = $builder->build({ + source => 'Biblio', + }); + my $biblioitem = $builder->build({ + source => 'Biblioitem', + value => { + biblionumber => $biblio->{biblionumber}, + } + }); + my $item = $builder->build({ + source => 'Item', + value => { + homebranch => $origin_branch->{branchcode}, + holdingbranch => $origin_branch->{branchcode}, + biblionumber => $biblio->{biblionumber}, + damaged => 0, + itemlost => 0, + } + }); + + Koha::IssuingRules->search()->delete(); + my $issuingrule = $builder->build({ + source => 'Issuingrule', + value => { + categorycode => '*', + itemtype => '*', + branchcode => '*', + reservesallowed => 99, + } + }); + + my $limit = Koha::Item::Transfer::Limit->new({ + toBranch => $pickup_branch->{branchcode}, + fromBranch => $item->{holdingbranch}, + itemtype => $item->{itype}, + })->store(); + + my $query = new CGI; + $query->param( 'pickup_location', $pickup_branch->{branchcode} ); + $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'bib_id', $biblio->{biblionumber}); + $query->param( 'item_id', $item->{itemnumber}); + + my $reply = C4::ILSDI::Services::HoldItem( $query ); + is( $reply->{code}, 'cannotBeTransferred', "Item hold, Item cannot be transferred" ); + + $reply = C4::ILSDI::Services::HoldTitle( $query ); + is( $reply->{code}, 'cannotBeTransferred', "Record hold, Item cannot be transferred" ); + + t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); + + $reply = C4::ILSDI::Services::HoldItem( $query ); + is( $reply->{code}, undef, "Item hold, Item can be transferred" ); + + Koha::Holds->search()->delete(); + + $reply = C4::ILSDI::Services::HoldTitle( $query ); + is( $reply->{code}, undef, "Record hold, Item con be transferred" ); + + $schema->storage->txn_rollback; +} diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 9b526258a7..ba1520e762 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -524,6 +524,20 @@ $item = Koha::Items->find($itemnumber)->unblessed; ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $borrower), "Reserving a book on item level" ); +my $pickup_branch = $builder->build({ source => 'Branch' })->{ branchcode }; +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); +t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); +my ($item_object) = Koha::Biblios->find($biblionumber)->items; +my $limit = Koha::Item::Transfer::Limit->new( + { + toBranch => $pickup_branch, + fromBranch => $item_object->holdingbranch, + itemtype => $item_object->effective_itemtype, + } +)->store(); +is( C4::Reserves::IsAvailableForItemLevelRequest($item, $borrower, $pickup_branch), 0, "Item level request not available due to transfer limit" ); +t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '0' ); + # 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)); -- 2.11.0