gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 9762: (follow-up) Add integration tests and improve AddRenewal logging consistency 

This commit adds two key improvements to the Bug 9762 circulation override logging:

1. **Consistent JSON Logging**: Updated AddRenewal to always log in JSON format,
   matching AddIssue behavior. Previously AddRenewal only logged JSON when
   confirmations/forced were present, otherwise logged just the item number.

2. **Integration Tests**: Added comprehensive tests for both AddIssue and AddRenewal
   JSON logging functionality:
   - Test normal operations (empty confirmations/forced arrays)
   - Test override operations (populated confirmations/forced arrays)
   - Verify JSON structure and content accuracy

The AddRenewal function now consistently logs:
```json
{
  "issue": 123,
  "itemnumber": 456,
  "confirmations": [...],
  "forced": [...]
}
```

This ensures uniform action log format across all circulation operations and
provides better integration test coverage for the override logging functionality.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b31f8c566d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This commit is contained in:
Martin Renvoize 2025-08-27 15:24:21 +01:00 committed by Paul Derscheid
parent df6a9ab122
commit 3334d14cae
No known key found for this signature in database
GPG Key ID: C25F9FD44CED595B
2 changed files with 151 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
C4/Circulation.pm
View File

@ -3683,21 +3683,18 @@ sub AddRenewal {
#Log the renewal
if ( C4::Context->preference("RenewalLog") ) {
my $info = $itemnumber;
if ( defined($confirmations) || defined($forced) ) {
$info = to_json(
{
issue => $issue->issue_id,
itemnumber => $itemnumber,
confirmations => $confirmations,
forced => $forced
},
{
pretty => 1,
canonical => 1
}
);
}
my $info = to_json(
{
issue => $issue->issue_id,
itemnumber => $itemnumber,
confirmations => $confirmations || [],
forced => $forced || []
},
{
pretty => 1,
canonical => 1
}
);
logaction( "CIRCULATION", "RENEWAL", $borrowernumber, $info );
}

140
t/db_dependent/Circulation.t
View File

@ -19,7 +19,7 @@ use Modern::Perl;
use utf8;
use Test::NoWarnings;
use Test::More tests => 78;
use Test::More tests => 80;
use Test::Exception;
use Test::MockModule;
use Test::Deep qw( cmp_deeply );
@ -28,6 +28,7 @@ use Test::Warn;
use Data::Dumper;
use DateTime;
use Time::Fake;
use JSON qw( from_json );
use POSIX qw( floor );
use t::lib::Mocks;
use t::lib::TestBuilder;
@ -7769,6 +7770,143 @@ subtest 'ChildNeedsGuarantor' => sub {
"AddReturn generates no warning when can_be_guarantee type patron has a guarantor and not ChildNeedsGuarantor";
};
subtest 'Bug 9762: AddIssue override JSON logging' => sub {
plan tests => 8;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item( { library => $library->branchcode } );
t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } );
t::lib::Mocks::mock_preference( 'IssueLog', 1 );
# Clear any existing logs
Koha::ActionLogs->search( { module => 'CIRCULATION', action => 'ISSUE' } )->delete;
# Test 1: Normal checkout without overrides - should log item number only
my $issue = C4::Circulation::AddIssue( $patron, $item->barcode );
ok( $issue, 'Item checked out successfully' );
my $logs = Koha::ActionLogs->search(
{
module => 'CIRCULATION',
action => 'ISSUE',
object => $patron->borrowernumber
},
{ order_by => { -desc => 'timestamp' } }
);
is( $logs->count, 1, 'One log entry created for normal checkout' );
my $log = $logs->next;
is( $log->info, $item->itemnumber, 'Normal checkout logs item number only' );
# Return the item for next test
C4::Circulation::AddReturn( $item->barcode, $library->branchcode );
# Test 2: Checkout with confirmations - should log JSON with confirmations
my $item2 = $builder->build_sample_item( { library => $library->branchcode } );
my $issue2 = C4::Circulation::AddIssue(
$patron,
$item2->barcode,
undef, undef, undef, undef,
{
confirmations => [ 'DEBT', 'AGE_RESTRICTION' ],
forced => []
}
);
ok( $issue2, 'Item with confirmations checked out successfully' );
$logs = Koha::ActionLogs->search(
{
module => 'CIRCULATION',
action => 'ISSUE',
object => $patron->borrowernumber,
info => { '!=', $item->itemnumber } # Exclude the first test log
},
{ order_by => { -desc => 'timestamp' } }
);
is( $logs->count, 1, 'One log entry created for override checkout' );
$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}, [ 'DEBT', 'AGE_RESTRICTION' ], 'Confirmations logged correctly' );
};
subtest 'Bug 9762: AddRenewal override JSON logging' => sub {
plan tests => 11;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item( { library => $library->branchcode } );
t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } );
t::lib::Mocks::mock_preference( 'RenewalLog', 1 );
# Issue the item first
my $issue = C4::Circulation::AddIssue( $patron, $item->barcode );
# Clear any existing renewal logs
Koha::ActionLogs->search( { module => 'CIRCULATION', action => 'RENEWAL' } )->delete;
# Test 1: Normal renewal without overrides - should log item number only
C4::Circulation::AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => $library->branchcode
}
);
my $logs = Koha::ActionLogs->search(
{
module => 'CIRCULATION',
action => 'RENEWAL',
object => $patron->borrowernumber
},
{ order_by => { -desc => 'timestamp' } }
);
is( $logs->count, 1, 'One log entry created for normal renewal' );
my $log = $logs->next;
my $log_data = eval { from_json( $log->info ) };
ok( !$@, 'Normal renewal log info is valid JSON' );
is( $log_data->{itemnumber}, $item->itemnumber, 'Normal renewal JSON contains itemnumber' );
is_deeply( $log_data->{confirmations}, [], 'Normal renewal has empty confirmations array' );
is_deeply( $log_data->{forced}, [], 'Normal renewal has empty forced array' );
# Test 2: Renewal with overrides - should log JSON with confirmations
C4::Circulation::AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => $library->branchcode,
confirmations => [ 'ON_RESERVE', 'RENEWAL_LIMIT' ],
forced => ['TOO_MUCH']
}
);
# Get only the most recent renewal log (the one with overrides)
my $override_log = Koha::ActionLogs->search(
{
module => 'CIRCULATION',
action => 'RENEWAL',
object => $patron->borrowernumber,
info => { 'like' => '%ON_RESERVE%' } # Only get the log with our test overrides
},
{ order_by => { -desc => 'timestamp' }, rows => 1 }
)->next;
ok( $override_log, 'Found override renewal log entry' );
$log_data = eval { from_json( $override_log->info ) };
ok( !$@, 'Override renewal log info is valid JSON' );
ok( exists $log_data->{confirmations}, 'JSON contains confirmations array' );
ok( exists $log_data->{forced}, 'JSON contains forced array' );
is_deeply( $log_data->{confirmations}, [ 'ON_RESERVE', 'RENEWAL_LIMIT' ], 'Confirmations logged correctly' );
is_deeply( $log_data->{forced}, ['TOO_MUCH'], 'Forced overrides logged correctly' );
};
$schema->storage->txn_rollback;
C4::Context->clear_syspref_cache();
$branches = Koha::Libraries->search();