Bugzilla – Attachment 187683 Details for
Bug 40866
Corrections to override logging
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40866: Add unit tests for hold override logging
Bug-40866-Add-unit-tests-for-hold-override-logging.patch (text/plain), 9.45 KB, created by
Andrew Fuerste-Henry
on 2025-10-09 15:21:30 UTC
(
hide
)
Description:
Bug 40866: Add unit tests for hold override logging
Filename:
MIME Type:
Creator:
Andrew Fuerste-Henry
Created:
2025-10-09 15:21:30 UTC
Size:
9.45 KB
patch
obsolete
>From 2f7b19737d66640aaa6106bcd7f5560715d29360 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 <andrew@bywatersolutions.com> >--- > 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
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 40866
:
186985
|
186986
|
187063
|
187116
|
187627
|
187628
|
187629
|
187630
|
187631
|
187632
|
187633
|
187675
|
187676
|
187677
|
187678
|
187679
|
187680
|
187681
|
187682
|
187683
|
187684
|
187685
|
187686
|
187687
|
187688
|
187689
|
187690
|
187691
|
187692
|
187693
|
187694
|
187707
|
187725