Bug 41728: (QA follow-up) Move return policy logic to Koha::Item
Move the AllowReturnToBranch and branch transfer limit checks from both C4::Circulation::CanBookBeReturned and Koha::Item::Checkin::Availability::_check_return_policy into a single authoritative method on Koha::Item: can_be_returned_at(). This eliminates the duplicate implementations and follows the Koha convention of placing item-related logic on the Koha::Item object. Changes: - Add Koha::Item->can_be_returned_at() with AllowReturnToBranch policy and transfer limit checks - Make CanBookBeReturned a thin wrapper delegating to the new method - Update Checkin::Availability to call $item->can_be_returned_at() - Remove _check_return_policy and unused withdrawn warning from the Availability class 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:
Martin Renvoize
Pedro Amorim
parent
b689dd07cc
commit
24854abf30
|
|
@ -1478,37 +1478,7 @@ Returns:
|
|||
|
||||
sub CanBookBeReturned {
|
||||
my ( $item, $returnbranch, $transferbranch ) = @_;
|
||||
my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
|
||||
|
||||
my $allowed = 1;
|
||||
my $message;
|
||||
|
||||
# Refuse check-in if it does not respect AllowReturnToBranch rules
|
||||
if ( $allowreturntobranch eq 'homebranch' && $returnbranch ne $item->homebranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch;
|
||||
} elsif ( $allowreturntobranch eq 'holdingbranch' && $returnbranch ne $item->holdingbranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->holdingbranch;
|
||||
} elsif ( $allowreturntobranch eq 'homeorholdingbranch'
|
||||
&& $returnbranch ne $item->homebranch
|
||||
&& $returnbranch ne $item->holdingbranch )
|
||||
{
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch; # FIXME: choice of homebranch is arbitrary
|
||||
}
|
||||
|
||||
# Refuse check-in if book cannot be transferred to $transferbranch due to transfer limits
|
||||
if ( defined($transferbranch) && $allowed ) {
|
||||
my $from_library = Koha::Libraries->find($returnbranch);
|
||||
my $to_library = Koha::Libraries->find($transferbranch);
|
||||
if ( !$item->can_be_transferred( { from => $from_library, to => $to_library } ) ) {
|
||||
$allowed = 0;
|
||||
$message = $transferbranch;
|
||||
}
|
||||
}
|
||||
|
||||
return ( $allowed, $message );
|
||||
return $item->can_be_returned_at( { library => $returnbranch, to_library => $transferbranch } );
|
||||
}
|
||||
|
||||
=head2 CheckHighHolds
|
||||
|
|
|
|||
51
Koha/Item.pm
51
Koha/Item.pm
|
|
@ -805,6 +805,57 @@ sub check_booking {
|
|||
return $bookings_count ? 0 : 1;
|
||||
}
|
||||
|
||||
=head3 can_be_returned_at
|
||||
|
||||
my ($allowed, $message) = $item->can_be_returned_at(
|
||||
{
|
||||
library => $branchcode,
|
||||
to_library => $destination_branchcode, # optional
|
||||
}
|
||||
);
|
||||
|
||||
Checks whether this item can be returned at the given library, based on
|
||||
the AllowReturnToBranch system preference and branch transfer limits.
|
||||
|
||||
Returns a two-element list: ($allowed, $message).
|
||||
|
||||
When the return is not allowed, $message contains the branchcode of
|
||||
the library where the item should be returned instead.
|
||||
|
||||
=cut
|
||||
|
||||
sub can_be_returned_at {
|
||||
my ( $self, $params ) = @_;
|
||||
|
||||
my $library = $params->{library};
|
||||
my $to_library = $params->{to_library};
|
||||
|
||||
my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
|
||||
|
||||
# Refuse check-in if it does not respect AllowReturnToBranch rules
|
||||
if ( $allowreturntobranch eq 'homebranch' && $library ne $self->homebranch ) {
|
||||
return ( 0, $self->homebranch );
|
||||
} elsif ( $allowreturntobranch eq 'holdingbranch' && $library ne $self->holdingbranch ) {
|
||||
return ( 0, $self->holdingbranch );
|
||||
} elsif ( $allowreturntobranch eq 'homeorholdingbranch'
|
||||
&& $library ne $self->homebranch
|
||||
&& $library ne $self->holdingbranch )
|
||||
{
|
||||
return ( 0, $self->homebranch ); # FIXME: choice of homebranch is arbitrary
|
||||
}
|
||||
|
||||
# Refuse check-in if item cannot be transferred to $to_library due to transfer limits
|
||||
if ( defined $to_library ) {
|
||||
my $from = Koha::Libraries->find($library);
|
||||
my $to = Koha::Libraries->find($to_library);
|
||||
if ( !$self->can_be_transferred( { from => $from, to => $to } ) ) {
|
||||
return ( 0, $to_library );
|
||||
}
|
||||
}
|
||||
|
||||
return ( 1, undef );
|
||||
}
|
||||
|
||||
=head3 checkin_availability
|
||||
|
||||
my $availability = $item->checkin_availability(
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ use Modern::Perl;
|
|||
|
||||
use C4::Context;
|
||||
use Koha::Availability::Result;
|
||||
use Koha::Libraries;
|
||||
|
||||
=head1 NAME
|
||||
|
||||
|
|
@ -113,8 +112,9 @@ sub check {
|
|||
return $result;
|
||||
}
|
||||
|
||||
# Check AllowReturnToBranch policy
|
||||
my ( $returnallowed, $message ) = _check_return_policy( $item, $library );
|
||||
# Check AllowReturnToBranch policy and branch transfer limits
|
||||
my ( $returnallowed, $message ) =
|
||||
$item->can_be_returned_at( { library => $library, to_library => $to_library } );
|
||||
unless ($returnallowed) {
|
||||
$result->add_blocker(
|
||||
Wrongbranch => {
|
||||
|
|
@ -125,70 +125,15 @@ sub check {
|
|||
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 lost and blocked
|
||||
if ( $item->itemlost && C4::Context->preference("BlockReturnOfLostItems") ) {
|
||||
$result->add_blocker( BlockedLost => 1 );
|
||||
return $result;
|
||||
}
|
||||
|
||||
# Add warnings for withdrawn (when not blocked)
|
||||
if ( $item->withdrawn ) {
|
||||
$result->add_warning( withdrawn => 1 );
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
=head2 Internal methods
|
||||
|
||||
=head3 _check_return_policy
|
||||
|
||||
my ($allowed, $message) = _check_return_policy($item, $library);
|
||||
|
||||
Check if an item can be checked in at the given library based on the
|
||||
AllowReturnToBranch system preference.
|
||||
|
||||
=cut
|
||||
|
||||
sub _check_return_policy {
|
||||
my ( $item, $library ) = @_;
|
||||
my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere';
|
||||
|
||||
my $allowed = 1;
|
||||
my $message;
|
||||
|
||||
if ( $allowreturntobranch eq 'homebranch' && $library ne $item->homebranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch;
|
||||
} elsif ( $allowreturntobranch eq 'holdingbranch' && $library ne $item->holdingbranch ) {
|
||||
$allowed = 0;
|
||||
$message = $item->holdingbranch;
|
||||
} elsif ( $allowreturntobranch eq 'homeorholdingbranch'
|
||||
&& $library ne $item->homebranch
|
||||
&& $library ne $item->holdingbranch )
|
||||
{
|
||||
$allowed = 0;
|
||||
$message = $item->homebranch;
|
||||
}
|
||||
|
||||
return ( $allowed, $message );
|
||||
}
|
||||
|
||||
=head1 AUTHOR
|
||||
|
||||
Koha Development Team <https://koha-community.org/>
|
||||
|
|
|
|||
Loading…
Reference in New Issue