Bugzilla – Attachment 86802 Details for
Bug 22330
Transfer limits should be respected for placing holds in staff interface and APIs
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug-22330-Transfer-limits-should-be-respected-for-.patch (text/plain), 26.79 KB, created by
Josef Moravec
on 2019-03-20 14:37:37 UTC
(
hide
)
Description:
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Filename:
MIME Type:
Creator:
Josef Moravec
Created:
2019-03-20 14:37:37 UTC
Size:
26.79 KB
patch
obsolete
>From 2be1db9b05ff8a6fefd1c21ff4d56e6b564ceb0d 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> > >Signed-off-by: Josef Moravec <josef.moravec@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
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 22330
:
85067
|
85068
|
86479
|
86480
|
86481
|
86482
|
86758
|
86759
|
86760
|
86779
| 86802 |
86803
|
86804
|
86805
|
86806
|
86807