From 27e6cac583e157fb5bab59d23ed345ef7936d289 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 9 Oct 2025 09:03:57 +0100 Subject: [PATCH] Bug 40866: Add unit tests for hold override logging This patch adds unit tests to verify the hold override logging functionality implemented in the previous patches. The tests cover the logging of hold overrides in `C4/Reserves.pm` and the `Koha/REST/V1/Holds.pm` API endpoint. To test: 1. Run the following tests and confirm they pass: - prove -lrv t/db_dependent/Circulation.t - prove -lrv t/db_dependent/Reserves.t - prove -lrv t/db_dependent/api/v1/holds.t 2. All tests should pass. Changes: t/db_dependent/Circulation.t: - Added tests for hold override scenarios in circulation. t/db_dependent/Reserves.t: - Added tests to verify that hold overrides are correctly logged when a hold is placed. t/db_dependent/api/v1/holds.t: - Added tests to ensure that the holds API correctly logs overrides when a hold is created with the `x-koha-override` header. Signed-off-by: Andrew Fuerste Henry Signed-off-by: Paul Derscheid --- t/db_dependent/Circulation.t | 21 +++++++- t/db_dependent/Reserves.t | 92 ++++++++++++++++++++++++++++++++++- t/db_dependent/api/v1/holds.t | 72 ++++++++++++++++++++++++++- 3 files changed, 182 insertions(+), 3 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e663e3e6f3f..ade7d897150 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -19,7 +19,7 @@ use Modern::Perl; use utf8; use Test::NoWarnings; -use Test::More tests => 82; +use Test::More tests => 83; use Test::Exception; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -8005,6 +8005,25 @@ subtest 'Bug 9762: AddRenewal override JSON logging' => sub { is_deeply( $log_data->{forced}, ['TOO_MUCH'], 'Forced overrides logged correctly' ); }; +subtest 'Bug 40866: CanBookBeIssued returns correct number of values' => sub { + plan tests => 2; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = + $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->branchcode } } ); + my $item = $builder->build_sample_item( { library => $library->branchcode } ); + + t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } ); + + # CanBookBeIssued should return exactly 4 values after Bug 40866 cleanup + # (issuingimpossible, needsconfirmation, alerts, messages) + # The 5th return value (@message_log) from the original Bug 9762 implementation was removed + my @result = C4::Circulation::CanBookBeIssued( $patron, $item->barcode ); + + is( scalar @result, 4, 'CanBookBeIssued returns exactly 4 values' ); + ok( ref( $result[0] ) eq 'HASH', 'First return value is issuingimpossible hashref' ); +}; + $schema->storage->txn_rollback; C4::Context->clear_syspref_cache(); $branches = Koha::Libraries->search(); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 22778875a2b..33a99d714e5 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 70; +use Test::More tests => 71; use Test::NoWarnings; use Test::MockModule; use Test::Warn; @@ -25,6 +25,7 @@ use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; +use JSON qw( from_json ); use MARC::Record; use DateTime::Duration; @@ -2004,3 +2005,92 @@ subtest 'CheckReserves() item type tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'Bug 40866: AddReserve override JSON logging' => sub { + plan tests => 8; + + $schema->storage->txn_begin; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { branchcode => $library->branchcode } + } + ); + my $biblio = $builder->build_sample_biblio(); + my $item = $builder->build_sample_item( + { + library => $library->branchcode, + biblionumber => $biblio->biblionumber + } + ); + + t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } ); + t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); + + # Clear any existing logs + Koha::ActionLogs->search( { module => 'HOLDS', action => 'CREATE' } )->delete; + + # Test 1: Normal hold without overrides - should log hold ID only + my $hold_id = C4::Reserves::AddReserve( + { + branchcode => $library->branchcode, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + priority => 1, + } + ); + ok( $hold_id, 'Hold created successfully' ); + + my $logs = Koha::ActionLogs->search( + { + module => 'HOLDS', + action => 'CREATE', + object => $hold_id + }, + { order_by => { -desc => 'timestamp' } } + ); + is( $logs->count, 1, 'One log entry created for normal hold' ); + my $log = $logs->next; + is( $log->info, $hold_id, 'Normal hold logs hold ID only' ); + + # Cancel the hold for next test + my $hold = Koha::Holds->find($hold_id); + $hold->cancel; + + # Test 2: Hold with override confirmation - should log JSON with confirmations + my $hold_id2 = C4::Reserves::AddReserve( + { + branchcode => $library->branchcode, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + priority => 1, + confirmations => ['HOLD_POLICY_OVERRIDE'], + forced => [] + } + ); + ok( $hold_id2, 'Hold with override created successfully' ); + + $logs = Koha::ActionLogs->search( + { + module => 'HOLDS', + action => 'CREATE', + object => $hold_id2 + }, + { order_by => { -desc => 'timestamp' } } + ); + is( $logs->count, 1, 'One log entry created for override hold' ); + $log = $logs->next; + + my $log_data = eval { from_json( $log->info ) }; + ok( !$@, 'Log info is valid JSON' ); + ok( exists $log_data->{confirmations}, 'JSON contains confirmations array' ); + is_deeply( + $log_data->{confirmations}, + ['HOLD_POLICY_OVERRIDE'], + 'Confirmations logged correctly' + ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 333a20726e9..a0e15daeb7e 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 18; +use Test::More tests => 19; use Test::NoWarnings; use Test::MockModule; use Test::Mojo; @@ -25,6 +25,7 @@ use t::lib::TestBuilder; use t::lib::Mocks; use DateTime; +use JSON qw( from_json ); use Mojo::JSON qw(encode_json); use C4::Context; @@ -32,6 +33,7 @@ use Koha::Patrons; use C4::Reserves qw( AddReserve CanItemBeReserved CanBookBeReserved ); use C4::Items; +use Koha::ActionLogs; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Biblios; @@ -1885,3 +1887,71 @@ subtest 'PUT /holds/{hold_id}/lowest_priority tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'Bug 40866: API hold creation with override logs confirmations' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 1 } + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $biblio = $builder->build_sample_biblio; + + t::lib::Mocks::mock_preference( 'AllowHoldPolicyOverride', 1 ); + t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); + + # Clear any existing logs + Koha::ActionLogs->search( { module => 'HOLDS', action => 'CREATE' } )->delete; + + my $post_data = { + patron_id => $patron->id, + biblio_id => $biblio->id, + pickup_library_id => $library->branchcode, + }; + + # Create hold with override + my $hold_id = + $t->post_ok( "//$userid:$password@/api/v1/holds" => { 'x-koha-override' => 'any' } => json => $post_data ) + ->status_is(201)->tx->res->json->{hold_id}; + + ok( $hold_id, 'Hold created via API with override' ); + + # Verify the hold was logged with override information + my $logs = Koha::ActionLogs->search( + { + module => 'HOLDS', + action => 'CREATE', + object => $hold_id + }, + { order_by => { -desc => 'timestamp' } } + ); + + is( $logs->count, 1, 'One log entry created for API hold with override' ); + + my $log = $logs->next; + my $log_data = eval { from_json( $log->info ) }; + ok( !$@, 'Log info is valid JSON' ); + is_deeply( + $log_data->{confirmations}, + ['HOLD_POLICY_OVERRIDE'], + 'HOLD_POLICY_OVERRIDE logged in confirmations array' + ); + + $schema->storage->txn_rollback; +}; -- 2.39.5