From c29f343648c1636e73250ae37b719e2f929e8aa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Wed, 28 Jan 2026 15:34:10 -0300 Subject: [PATCH] Bug 41728: Add Koha::Item::Checkin::Availability class This patch introduces a new class to centralize and extract check-in validation logic from C4::Circulation::AddReturn. The class provides a clean interface for checking whether an item can be checked in, returning a Koha::Availability::Result object. The class is designed with the item as the subject of the check-in operation, following the pattern: "Can THIS ITEM be checked in?" The class extracts the following validation logic from AddReturn: - Withdrawn item handling (BlockedWithdrawn blocker) - Lost item handling (BlockedLost blocker) - Branch check-in policy validation (Wrongbranch blocker) - Not issued detection (NotIssued confirmation) This separation enables: - Reusable validation logic for REST API and legacy code - Pre-flight availability checks without side effects - Structured responses for confirmation flow implementation - Better testability and maintainability The check() method accepts an item object and returns a Koha::Availability::Result object with blockers, confirmations, warnings, and context (checkout, patron). A convenience instance method is added to Koha::Item: $item->checkin_availability({ branch => $branch }) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Checkin/Availability.t => SUCCESS: Tests pass! 3. Verify all validation scenarios work correctly: - Withdrawn items blocked when syspref enabled - Lost items blocked when syspref enabled - Wrong branch returns blocked per AllowReturnToBranch - Not issued items handled correctly - Checked out items return checkout and patron objects 4. Sign off :-D --- Koha/Item.pm | 40 ++- Koha/Item/Checkin/Availability.pm | 173 +++++++++++++ .../Koha/Item/Checkin/Availability.t | 236 ++++++++++++++++++ 3 files changed, 444 insertions(+), 5 deletions(-) create mode 100644 Koha/Item/Checkin/Availability.pm create mode 100755 t/db_dependent/Koha/Item/Checkin/Availability.t diff --git a/Koha/Item.pm b/Koha/Item.pm index 52f1d1862a..8d239dc14c 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -179,7 +179,7 @@ sub store { # If the field has changed otherwise, we much update # the 'on' field - elsif (exists $updated_columns{$field} + elsif ( exists $updated_columns{$field} && $updated_columns{$field} && !$pre_mod_item->$field ) { @@ -376,7 +376,7 @@ sub safe_to_delete { { itemnumber => undef, } - )->count; + )->count; if ($error) { return Koha::Result::Boolean->new(0)->add_message( { message => $error } ); @@ -778,6 +778,35 @@ sub check_booking { return $bookings_count ? 0 : 1; } +=head3 checkin_availability + + my $availability = $item->checkin_availability( + { + branch => $branchcode, + } + ); + +Returns check-in availability information for this item. + +This is a convenience method that calls Koha::Item::Checkin::Availability->check(). + +Returns a Koha::Availability::Result object with methods: + available() # Boolean: true if check-in allowed + needs_confirmation()# Boolean: true if confirmation required + blockers() # Hashref of blocking conditions + confirmations() # Hashref of conditions requiring confirmation + warnings() # Hashref of warning messages + context() # Hashref with checkout and patron objects + +=cut + +sub checkin_availability { + my ( $self, $params ) = @_; + + require Koha::Item::Checkin::Availability; + return Koha::Item::Checkin::Availability->check( $self, $params ); +} + =head3 request_transfer my $transfer = $item->request_transfer( @@ -1525,8 +1554,8 @@ sub columns_to_str { $subfield ? $subfield->{authorised_value} ? C4::Biblio::GetAuthorisedValueDesc( - $itemtagfield, - $subfield->{tagsubfield}, $value, '', $tagslib + $itemtagfield, + $subfield->{tagsubfield}, $value, '', $tagslib ) : $value : $value; @@ -1750,7 +1779,8 @@ sub _set_found_trigger { if ($lost_fee_payment) { my $today = dt_from_string(); my $payment_age_in_days = - dt_from_string( $lost_fee_payment->created_on )->delta_days($today) + dt_from_string( $lost_fee_payment->created_on ) + ->delta_days($today) ->in_units('days'); if ( $payment_age_in_days > $no_refund_if_lost_fee_paid_age ) { $self->add_message( diff --git a/Koha/Item/Checkin/Availability.pm b/Koha/Item/Checkin/Availability.pm new file mode 100644 index 0000000000..dde017af96 --- /dev/null +++ b/Koha/Item/Checkin/Availability.pm @@ -0,0 +1,173 @@ +package Koha::Item::Checkin::Availability; + +# Copyright 2026 Koha Development Team +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use C4::Context; +use Koha::Availability::Result; + +=head1 NAME + +Koha::Item::Checkin::Availability - Check-in availability validation for items + +=head1 SYNOPSIS + + my $availability = Koha::Item::Checkin::Availability->check( $item, + { + branch => $branch, + } + ); + + # Or via instance method + my $availability = $item->checkin_availability( { branch => $branch } ); + +=head1 DESCRIPTION + +This class provides validation logic for item check-in operations, extracting +and centralizing checks from C4::Circulation::AddReturn. + +The result categorizes conditions as: +- blockers: Prevent check-in (e.g., BlockReturnOfWithdrawnItems enabled) +- confirmations: Noteworthy conditions (e.g., item not checked out) +- warnings: Informational (e.g., item is withdrawn but check-in allowed) + +=head1 API + +=head2 Class methods + +=head3 check + + my $availability = Koha::Item::Checkin::Availability->check( $item, + { + branch => $branch, + } + ); + +Validates check-in availability for an item. + +Parameters: + $item - Koha::Item object (required) + $params - hashref with: + branch => $branchcode (required) + +Returns a Koha::Availability::Result object with: + blockers => {} # Conditions that prevent check-in + confirmations => {} # Conditions requiring confirmation + warnings => {} # Informational messages + context => { + checkout => $checkout # Koha::Checkout object if checked out + patron => $patron # Koha::Patron object if item is checked out + } + +=cut + +sub check { + my ( $class, $item, $params ) = @_; + + my $branch = $params->{branch}; + + my $result = Koha::Availability::Result->new(); + + # Check if item is withdrawn and blocked + if ( $item->withdrawn && C4::Context->preference("BlockReturnOfWithdrawnItems") ) { + $result->add_blocker( BlockedWithdrawn => 1 ); + return $result; + } + + # Check if item is lost and blocked + if ( $item->itemlost && C4::Context->preference("BlockReturnOfLostItems") ) { + $result->add_blocker( BlockedLost => 1 ); + return $result; + } + + # Check branch check-in policy + my ( $returnallowed, $message ) = _can_book_be_returned( $item, $branch ); + unless ($returnallowed) { + $result->add_blocker( + Wrongbranch => { + Wrongbranch => $branch, + Rightbranch => $message + } + ); + return $result; + } + + # Check if item is currently checked out + my $checkout = $item->checkout; + if ($checkout) { + $result->set_context( checkout => $checkout ); + $result->set_context( patron => $checkout->patron ); + } else { + + # Item not checked out + $result->add_confirmation( NotIssued => $item->barcode ); + } + + # Add warnings for withdrawn (when not blocked) + if ( $item->withdrawn ) { + $result->add_warning( withdrawn => 1 ); + } + + return $result; +} + +=head2 Internal methods + +=head3 _can_book_be_returned + + my ($allowed, $message) = _can_book_be_returned($item, $branch); + +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. + +=cut + +sub _can_book_be_returned { + my ( $item, $branch ) = @_; + my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere'; + + my $allowed = 1; + my $message; + + if ( $allowreturntobranch eq 'homebranch' && $branch ne $item->homebranch ) { + $allowed = 0; + $message = $item->homebranch; + } elsif ( $allowreturntobranch eq 'holdingbranch' && $branch ne $item->holdingbranch ) { + $allowed = 0; + $message = $item->holdingbranch; + } elsif ( $allowreturntobranch eq 'homeorholdingbranch' + && $branch ne $item->homebranch + && $branch ne $item->holdingbranch ) + { + $allowed = 0; + $message = $item->homebranch; + } + + return ( $allowed, $message ); +} + +=head1 AUTHOR + +Koha Development Team + +=cut + +1; diff --git a/t/db_dependent/Koha/Item/Checkin/Availability.t b/t/db_dependent/Koha/Item/Checkin/Availability.t new file mode 100755 index 0000000000..e5b6b66943 --- /dev/null +++ b/t/db_dependent/Koha/Item/Checkin/Availability.t @@ -0,0 +1,236 @@ +#!/usr/bin/perl + +# Copyright 2026 Koha Development team +# +# This file is part of Koha +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 7; +use Test::NoWarnings; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use Koha::Database; +use Koha::Item::Checkin::Availability; + +my $builder = t::lib::TestBuilder->new; +my $schema = Koha::Database->new->schema; + +subtest 'check() - item exists' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_object( { class => 'Koha::Items' } ); + + my $result = $item->checkin_availability( { branch => $library->branchcode } ); + + isa_ok( $result, 'Koha::Availability::Result', 'Returns Result object' ); + ok( $result->available, 'Item is available for check-in' ); + + $schema->storage->txn_rollback; +}; + +subtest 'check() - BlockedWithdrawn blocker' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'BlockReturnOfWithdrawnItems', 1 ); + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { withdrawn => 1 } + } + ); + + my $result = $item->checkin_availability( { branch => $library->branchcode } ); + + is( $result->blockers->{BlockedWithdrawn}, 1, 'BlockedWithdrawn blocker set' ); + is( keys %{ $result->confirmations }, 0, 'no confirmations when blocked' ); + + $schema->storage->txn_rollback; +}; + +subtest 'check() - BlockedLost blocker' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'BlockReturnOfLostItems', 1 ); + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { itemlost => 1 } + } + ); + + my $result = $item->checkin_availability( + { + branch => $library->branchcode, + } + ); + + is( $result->blockers->{BlockedLost}, 1, 'BlockedLost blocker set' ); + + is( keys %{ $result->confirmations }, 0, 'no confirmations when blocked' ); + + $schema->storage->txn_rollback; +}; + +subtest 'check() - Wrongbranch blocker' => sub { + + plan tests => 8; + + $schema->storage->txn_begin; + + my $homebranch = $builder->build_object( { class => 'Koha::Libraries' } ); + my $holdingbranch = $builder->build_object( { class => 'Koha::Libraries' } ); + my $wrongbranch = $builder->build_object( { class => 'Koha::Libraries' } ); + + my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { + homebranch => $homebranch->branchcode, + holdingbranch => $holdingbranch->branchcode, + } + } + ); + + # Test homebranch restriction + t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); + my $result = $item->checkin_availability( + { + branch => $wrongbranch->branchcode, + } + ); + + is( ref( $result->blockers->{Wrongbranch} ), 'HASH', 'Wrongbranch blocker is hashref' ); + is( $result->blockers->{Wrongbranch}->{Wrongbranch}, $wrongbranch->branchcode, 'wrong branch recorded' ); + is( $result->blockers->{Wrongbranch}->{Rightbranch}, $homebranch->branchcode, 'right branch recorded' ); + + # Test holdingbranch restriction + t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); + $result = $item->checkin_availability( + { + branch => $wrongbranch->branchcode, + } + ); + + is( + $result->blockers->{Wrongbranch}->{Wrongbranch}, $wrongbranch->branchcode, + 'wrong branch recorded for holdingbranch' + ); + is( + $result->blockers->{Wrongbranch}->{Rightbranch}, $holdingbranch->branchcode, + 'holding branch recorded as right branch' + ); + + # Test homeorholdingbranch restriction + t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); + $result = $item->checkin_availability( + { + branch => $wrongbranch->branchcode, + } + ); + + is( + $result->blockers->{Wrongbranch}->{Wrongbranch}, $wrongbranch->branchcode, + 'wrong branch recorded for homeorholdingbranch' + ); + + # Test anywhere - should not block + t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); + $result = $item->checkin_availability( + { + branch => $wrongbranch->branchcode, + } + ); + + is( $result->blockers->{Wrongbranch}, undef, 'no Wrongbranch blocker when anywhere allowed' ); + is( keys %{ $result->blockers }, 0, 'no blockers when anywhere allowed' ); + + $schema->storage->txn_rollback; +}; + +subtest 'check() - NotIssued confirmation' => sub { + + plan tests => 4; + + $schema->storage->txn_begin; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_object( { class => 'Koha::Items' } ); + + my $result = $item->checkin_availability( + { + branch => $library->branchcode, + } + ); + + is( keys %{ $result->blockers }, 0, 'no blockers for not issued item' ); + is( $result->confirmations->{NotIssued}, $item->barcode, 'NotIssued confirmation set with barcode' ); + is( $result->{issue}, undef, 'checkout is undef when not checked out' ); + is( $result->context->{patron}, undef, 'patron is undef when not checked out' ); + + $schema->storage->txn_rollback; +}; + +subtest 'check() - checked out item (no confirmations)' => sub { + + plan tests => 5; + + $schema->storage->txn_begin; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $item = $builder->build_object( { class => 'Koha::Items' } ); + my $issue = $builder->build_object( + { + class => 'Koha::Checkouts', + value => { + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + } + } + ); + + my $result = $item->checkin_availability( + { + branch => $library->branchcode, + } + ); + + is( keys %{ $result->blockers }, 0, 'no blockers for checked out item' ); + is( keys %{ $result->confirmations }, 0, 'no confirmations for checked out item' ); + + is( ref( $result->context->{checkout} ), 'Koha::Checkout', 'checkout object returned' ); + is( ref( $result->context->{patron} ), 'Koha::Patron', 'patron object returned' ); + is( $result->context->{patron}->id, $patron->id, 'correct patron returned' ); + + $schema->storage->txn_rollback; +}; -- 2.50.1 (Apple Git-155)