Bugzilla – Attachment 192131 Details for
Bug 41728
Add `Koha::Item::Checkin::Availability` to centralize logic
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41728: Add Koha::Item::Checkin::Availability class
c29f343.patch (text/plain), 17.95 KB, created by
Tomás Cohen Arazi (tcohen)
on 2026-01-28 19:12:30 UTC
(
hide
)
Description:
Bug 41728: Add Koha::Item::Checkin::Availability class
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2026-01-28 19:12:30 UTC
Size:
17.95 KB
patch
obsolete
>From c29f343648c1636e73250ae37b719e2f929e8aa9 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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 <https://www.gnu.org/licenses>. >+ >+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 <https://koha-community.org/> >+ >+=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 <https://www.gnu.org/licenses>. >+ >+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) >
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 41728
:
192130
| 192131 |
192132
|
192133