Bug 41728: (follow-up) Add transfer limit check and clean param names
- Rename params: branch => library, add to_library for destination - Move $returnbranch computation before the availability check in AddReturn so it can be passed as to_library - Add transfer limit check (bug 7376) to the Availability class - Rename internal _can_book_be_returned to _check_return_policy - Remove duplicate $returnbranch block and standalone transfer limit check from AddReturn - Update tests for new param names Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Tomás Cohen Arazi
Pedro Amorim
parent
4b277a6e4a
commit
cb3deeadcc
|
|
@ -2402,7 +2402,43 @@ sub AddReturn {
|
|||
}
|
||||
|
||||
# Check availability
|
||||
my $availability = $item->checkin_availability( { branch => $branch } );
|
||||
my $hbr = Koha::CirculationRules->get_return_branch_policy($item);
|
||||
|
||||
# check if returnbranch and homebranch belong to the same float group
|
||||
my $validate_float =
|
||||
Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $branch } );
|
||||
|
||||
# get the proper branch to which to return the item
|
||||
my $returnbranch;
|
||||
if ( $hbr eq 'noreturn' ) {
|
||||
$returnbranch = $branch;
|
||||
} elsif ( $hbr eq 'returnbylibrarygroup' ) {
|
||||
|
||||
# if library isn't in same the float group, transfer item to homebranch
|
||||
$hbr = 'homebranch';
|
||||
$returnbranch = $validate_float ? $branch : $item->$hbr;
|
||||
} else {
|
||||
$returnbranch = $item->$hbr;
|
||||
}
|
||||
|
||||
# if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
|
||||
my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef;
|
||||
|
||||
# check library float limits if enabled if the item isn't being transferred away
|
||||
if ( ( $returnbranch eq $branch ) && C4::Context->preference('UseLibraryFloatLimits') ) {
|
||||
my $effective_itemtype = $item->effective_itemtype;
|
||||
my $limit = Koha::Library::FloatLimits->find( { itemtype => $effective_itemtype, branchcode => $branch } );
|
||||
if ($limit) {
|
||||
my $transfer_library =
|
||||
Koha::Library::FloatLimits->lowest_ratio_library( $item, $branch, $item->holdingbranch );
|
||||
if ( $transfer_library && $transfer_library->branchcode ne $branch ) {
|
||||
$returnbranch = $transfer_library->branchcode;
|
||||
$transfer_trigger = 'LibraryFloatLimit';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
my $availability = $item->checkin_availability( { library => $branch, to_library => $returnbranch } );
|
||||
|
||||
# Extract context objects
|
||||
my $issue = $availability->context->{checkout};
|
||||
|
|
@ -2476,62 +2512,9 @@ sub AddReturn {
|
|||
$messages->{'withdrawn'} = 1;
|
||||
}
|
||||
|
||||
# full item data, but no borrowernumber or checkout info (no issue)
|
||||
my $hbr = Koha::CirculationRules->get_return_branch_policy($item);
|
||||
|
||||
# check if returnbranch and homebranch belong to the same float group
|
||||
my $validate_float =
|
||||
Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $branch } );
|
||||
|
||||
# get the proper branch to which to return the item
|
||||
my $returnbranch;
|
||||
if ( $hbr eq 'noreturn' ) {
|
||||
$returnbranch = $branch;
|
||||
} elsif ( $hbr eq 'returnbylibrarygroup' ) {
|
||||
|
||||
# if library isn't in same the float group, transfer item to homebranch
|
||||
$hbr = 'homebranch';
|
||||
$returnbranch = $validate_float ? $branch : $item->$hbr;
|
||||
} else {
|
||||
$returnbranch = $item->$hbr;
|
||||
}
|
||||
|
||||
# if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
|
||||
my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef;
|
||||
|
||||
# check library float limits if enabled if the item isn't being transferred away
|
||||
if ( ( $returnbranch eq $branch ) && C4::Context->preference('UseLibraryFloatLimits') ) {
|
||||
my $effective_itemtype = $item->effective_itemtype;
|
||||
my $limit = Koha::Library::FloatLimits->find( { itemtype => $effective_itemtype, branchcode => $branch } );
|
||||
if ($limit) {
|
||||
my $transfer_library =
|
||||
Koha::Library::FloatLimits->lowest_ratio_library( $item, $branch, $item->holdingbranch );
|
||||
if ( $transfer_library && $transfer_library->branchcode ne $branch ) {
|
||||
$returnbranch = $transfer_library->branchcode;
|
||||
$transfer_trigger = 'LibraryFloatLimit';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
my $borrowernumber = $patron ? $patron->borrowernumber : undef; # we don't know if we had a borrower or not
|
||||
my $patron_unblessed = $patron ? $patron->unblessed : {};
|
||||
|
||||
# Check branch transfer limits (bug 7376)
|
||||
if ( $doreturn && defined($returnbranch) ) {
|
||||
my $from_library = Koha::Libraries->find($branch);
|
||||
my $to_library = Koha::Libraries->find($returnbranch);
|
||||
if ( !$item->can_be_transferred( { from => $from_library, to => $to_library } ) ) {
|
||||
$messages->{'Wrongbranch'} = {
|
||||
Wrongbranch => $branch,
|
||||
Rightbranch => $returnbranch
|
||||
};
|
||||
$doreturn = 0;
|
||||
my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
|
||||
$indexer->index_records( $item->biblionumber, "specialUpdate", "biblioserver" );
|
||||
return ( $doreturn, $messages, $issue, $patron_unblessed );
|
||||
}
|
||||
}
|
||||
|
||||
# Update item location
|
||||
my $loc_messages = $item->location_update_trigger('checkin');
|
||||
foreach my $loc_msg_key ( keys %$loc_messages ) {
|
||||
|
|
|
|||
|
|
@ -809,7 +809,8 @@ sub check_booking {
|
|||
|
||||
my $availability = $item->checkin_availability(
|
||||
{
|
||||
branch => $branchcode,
|
||||
library => $branchcode,
|
||||
to_library => $destination_branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -817,6 +818,11 @@ Returns check-in availability information for this item.
|
|||
|
||||
This is a convenience method that calls Koha::Item::Checkin::Availability->check().
|
||||
|
||||
Parameters:
|
||||
library => branchcode where the return takes place (required)
|
||||
to_library => branchcode where the item should be sent after return
|
||||
(optional, for transfer limit validation)
|
||||
|
||||
Returns a Koha::Availability::Result object with methods:
|
||||
available() # Boolean: true if check-in allowed
|
||||
needs_confirmation()# Boolean: true if confirmation required
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ use Modern::Perl;
|
|||
|
||||
use C4::Context;
|
||||
use Koha::Availability::Result;
|
||||
use Koha::Libraries;
|
||||
|
||||
=head1 NAME
|
||||
|
||||
|
|
@ -30,12 +31,18 @@ Koha::Item::Checkin::Availability - Check-in availability validation for items
|
|||
|
||||
my $availability = Koha::Item::Checkin::Availability->check( $item,
|
||||
{
|
||||
branch => $branch,
|
||||
library => $branchcode,
|
||||
to_library => $destination_branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
# Or via instance method
|
||||
my $availability = $item->checkin_availability( { branch => $branch } );
|
||||
my $availability = $item->checkin_availability(
|
||||
{
|
||||
library => $branchcode,
|
||||
to_library => $destination_branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
=head1 DESCRIPTION
|
||||
|
||||
|
|
@ -55,7 +62,8 @@ The result categorizes conditions as:
|
|||
|
||||
my $availability = Koha::Item::Checkin::Availability->check( $item,
|
||||
{
|
||||
branch => $branch,
|
||||
library => $branchcode,
|
||||
to_library => $destination_branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -64,7 +72,9 @@ Validates check-in availability for an item.
|
|||
Parameters:
|
||||
$item - Koha::Item object (required)
|
||||
$params - hashref with:
|
||||
branch => $branchcode (required)
|
||||
library => branchcode where the return takes place (required)
|
||||
to_library => branchcode where the item should be sent after return
|
||||
(optional, for transfer limit validation)
|
||||
|
||||
Returns a Koha::Availability::Result object with:
|
||||
blockers => {} # Conditions that prevent check-in
|
||||
|
|
@ -80,7 +90,8 @@ Returns a Koha::Availability::Result object with:
|
|||
sub check {
|
||||
my ( $class, $item, $params ) = @_;
|
||||
|
||||
my $branch = $params->{branch};
|
||||
my $library = $params->{library};
|
||||
my $to_library = $params->{to_library};
|
||||
|
||||
my $result = Koha::Availability::Result->new();
|
||||
|
||||
|
|
@ -96,18 +107,33 @@ sub check {
|
|||
return $result;
|
||||
}
|
||||
|
||||
# Check branch check-in policy
|
||||
my ( $returnallowed, $message ) = _can_book_be_returned( $item, $branch );
|
||||
# Check AllowReturnToBranch policy
|
||||
my ( $returnallowed, $message ) = _check_return_policy( $item, $library );
|
||||
unless ($returnallowed) {
|
||||
$result->add_blocker(
|
||||
Wrongbranch => {
|
||||
Wrongbranch => $branch,
|
||||
Wrongbranch => $library,
|
||||
Rightbranch => $message
|
||||
}
|
||||
);
|
||||
return $result;
|
||||
}
|
||||
|
||||
# Check branch transfer limits (bug 7376)
|
||||
if ( defined $to_library ) {
|
||||
my $from = Koha::Libraries->find($library);
|
||||
my $to = Koha::Libraries->find($to_library);
|
||||
if ( !$item->can_be_transferred( { from => $from, to => $to } ) ) {
|
||||
$result->add_blocker(
|
||||
Wrongbranch => {
|
||||
Wrongbranch => $library,
|
||||
Rightbranch => $to_library
|
||||
}
|
||||
);
|
||||
return $result;
|
||||
}
|
||||
}
|
||||
|
||||
# Check if item is currently checked out
|
||||
my $checkout = $item->checkout;
|
||||
if ($checkout) {
|
||||
|
|
@ -129,33 +155,31 @@ sub check {
|
|||
|
||||
=head2 Internal methods
|
||||
|
||||
=head3 _can_book_be_returned
|
||||
=head3 _check_return_policy
|
||||
|
||||
my ($allowed, $message) = _can_book_be_returned($item, $branch);
|
||||
my ($allowed, $message) = _check_return_policy($item, $library);
|
||||
|
||||
Internal method to check if an item can be checked in to a specific branch
|
||||
based on the AllowReturnToBranch system preference.
|
||||
|
||||
This is extracted from C4::Circulation::CanBookBeReturned.
|
||||
Check if an item can be checked in at the given library based on the
|
||||
AllowReturnToBranch system preference.
|
||||
|
||||
=cut
|
||||
|
||||
sub _can_book_be_returned {
|
||||
my ( $item, $branch ) = @_;
|
||||
sub _check_return_policy {
|
||||
my ( $item, $library ) = @_;
|
||||
my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
|
||||
|
||||
my $allowed = 1;
|
||||
my $message;
|
||||
|
||||
if ( $allowreturntobranch eq 'homebranch' && $branch ne $item->homebranch ) {
|
||||
if ( $allowreturntobranch eq 'homebranch' && $library ne $item->homebranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch;
|
||||
} elsif ( $allowreturntobranch eq 'holdingbranch' && $branch ne $item->holdingbranch ) {
|
||||
} elsif ( $allowreturntobranch eq 'holdingbranch' && $library ne $item->holdingbranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->holdingbranch;
|
||||
} elsif ( $allowreturntobranch eq 'homeorholdingbranch'
|
||||
&& $branch ne $item->homebranch
|
||||
&& $branch ne $item->holdingbranch )
|
||||
&& $library ne $item->homebranch
|
||||
&& $library ne $item->holdingbranch )
|
||||
{
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch;
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ subtest 'check() - item exists' => sub {
|
|||
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
|
||||
my $item = $builder->build_object( { class => 'Koha::Items' } );
|
||||
|
||||
my $result = $item->checkin_availability( { branch => $library->branchcode } );
|
||||
my $result = $item->checkin_availability( { library => $library->branchcode } );
|
||||
|
||||
isa_ok( $result, 'Koha::Availability::Result', 'Returns Result object' );
|
||||
ok( $result->available, 'Item is available for check-in' );
|
||||
|
|
@ -64,7 +64,7 @@ subtest 'check() - BlockedWithdrawn blocker' => sub {
|
|||
}
|
||||
);
|
||||
|
||||
my $result = $item->checkin_availability( { branch => $library->branchcode } );
|
||||
my $result = $item->checkin_availability( { library => $library->branchcode } );
|
||||
|
||||
is( $result->blockers->{BlockedWithdrawn}, 1, 'BlockedWithdrawn blocker set' );
|
||||
is( keys %{ $result->confirmations }, 0, 'no confirmations when blocked' );
|
||||
|
|
@ -90,7 +90,7 @@ subtest 'check() - BlockedLost blocker' => sub {
|
|||
|
||||
my $result = $item->checkin_availability(
|
||||
{
|
||||
branch => $library->branchcode,
|
||||
library => $library->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -125,7 +125,7 @@ subtest 'check() - Wrongbranch blocker' => sub {
|
|||
t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' );
|
||||
my $result = $item->checkin_availability(
|
||||
{
|
||||
branch => $wrongbranch->branchcode,
|
||||
library => $wrongbranch->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -137,7 +137,7 @@ subtest 'check() - Wrongbranch blocker' => sub {
|
|||
t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' );
|
||||
$result = $item->checkin_availability(
|
||||
{
|
||||
branch => $wrongbranch->branchcode,
|
||||
library => $wrongbranch->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -154,7 +154,7 @@ subtest 'check() - Wrongbranch blocker' => sub {
|
|||
t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' );
|
||||
$result = $item->checkin_availability(
|
||||
{
|
||||
branch => $wrongbranch->branchcode,
|
||||
library => $wrongbranch->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -167,7 +167,7 @@ subtest 'check() - Wrongbranch blocker' => sub {
|
|||
t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' );
|
||||
$result = $item->checkin_availability(
|
||||
{
|
||||
branch => $wrongbranch->branchcode,
|
||||
library => $wrongbranch->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -188,7 +188,7 @@ subtest 'check() - NotIssued confirmation' => sub {
|
|||
|
||||
my $result = $item->checkin_availability(
|
||||
{
|
||||
branch => $library->branchcode,
|
||||
library => $library->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
@ -221,7 +221,7 @@ subtest 'check() - checked out item (no confirmations)' => sub {
|
|||
|
||||
my $result = $item->checkin_availability(
|
||||
{
|
||||
branch => $library->branchcode,
|
||||
library => $library->branchcode,
|
||||
}
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue