Bugzilla – Attachment 190624 Details for
Bug 41358
action logs info column should always store JSON
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41358: Always store circulation logs in consistent JSON format
18f82bf.patch (text/plain), 5.78 KB, created by
Martin Renvoize (ashimema)
on 2025-12-18 15:58:23 UTC
(
hide
)
Description:
Bug 41358: Always store circulation logs in consistent JSON format
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-12-18 15:58:23 UTC
Size:
5.78 KB
patch
obsolete
>From 18f82bf60fc4aec3091768ed3e4a090991b24e0e Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 18 Dec 2025 14:47:39 +0000 >Subject: [PATCH] Bug 41358: Always store circulation logs in consistent JSON > format > >Previously, CIRCULATION ISSUE logs would store just the itemnumber when >there were no confirmations or forced overrides, but would store a full >JSON structure when overrides were present. This inconsistency makes >reporting difficult as queries need to handle both formats. > >This patch changes AddIssue to always store logs in JSON format, >matching the pattern already used in AddRenewal. The JSON structure >now consistently includes: >- issue: issue_id >- branchcode: branchcode >- itemnumber: itemnumber >- confirmations: array of confirmation codes (empty array if none) >- forced: array of forced override codes (empty array if none) > >Benefits: >- Consistent data format simplifies reporting and analysis >- Matches existing RENEWAL logging pattern >- Display code (tools/viewlog.pl) already handles both formats, > so backward compatibility is maintained > >Test plan: >1. prove t/db_dependent/Circulation.t >2. Perform a normal checkout - verify action log contains JSON >3. Perform a checkout with overrides - verify confirmations/forced > are populated in the JSON >4. Check Tools > Log viewer - both types should display correctly > >Sponsored-by: OpenFifth <https://openfifth.co.uk/> >--- > C4/Circulation.pm | 23 ++++++++++------------- > t/db_dependent/Circulation.t | 16 ++++++++++------ > 2 files changed, 20 insertions(+), 19 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 0b2b57e3754..4a7f8b762aa 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1951,19 +1951,16 @@ sub AddIssue { > > # Log the checkout > if ( C4::Context->preference('IssueLog') ) { >- my $info = $item_object->itemnumber; >- if ( defined($confirmations) || defined($forced) ) { >- $info = to_json( >- { >- issue => $issue->issue_id, >- branchcode => $issue->branchcode, >- itemnumber => $item_object->itemnumber, >- confirmations => $confirmations, >- forced => $forced >- }, >- { pretty => 1, canonical => 1 } >- ); >- } >+ my $info = to_json( >+ { >+ issue => $issue->issue_id, >+ branchcode => $issue->branchcode, >+ itemnumber => $item_object->itemnumber, >+ confirmations => $confirmations || [], >+ forced => $forced || [] >+ }, >+ { pretty => 1, canonical => 1 } >+ ); > logaction( > "CIRCULATION", "ISSUE", > $patron->borrowernumber, >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index af0a61c5324..3e3d0d4ec41 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -7871,7 +7871,7 @@ subtest 'ChildNeedsGuarantor' => sub { > }; > > subtest 'Bug 9762: AddIssue override JSON logging' => sub { >- plan tests => 8; >+ plan tests => 11; > > my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >@@ -7883,7 +7883,7 @@ subtest 'Bug 9762: AddIssue override JSON logging' => sub { > # Clear any existing logs > Koha::ActionLogs->search( { module => 'CIRCULATION', action => 'ISSUE' } )->delete; > >- # Test 1: Normal checkout without overrides - should log item number only >+ # Test 1: Normal checkout without overrides - should log JSON with empty arrays > my $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); > ok( $issue, 'Item checked out successfully' ); > >@@ -7896,8 +7896,12 @@ subtest 'Bug 9762: AddIssue override JSON logging' => sub { > { 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' ); >+ my $log = $logs->next; >+ my $log_data = eval { from_json( $log->info ) }; >+ ok( !$@, 'Normal checkout log info is valid JSON' ); >+ is( $log_data->{itemnumber}, $item->itemnumber, 'JSON contains correct itemnumber' ); >+ is_deeply( $log_data->{confirmations}, [], 'Confirmations is empty array for normal checkout' ); >+ is_deeply( $log_data->{forced}, [], 'Forced is empty array for normal checkout' ); > > # Return the item for next test > C4::Circulation::AddReturn( $item->barcode, $library->branchcode ); >@@ -7921,14 +7925,14 @@ subtest 'Bug 9762: AddIssue override JSON logging' => sub { > module => 'CIRCULATION', > action => 'ISSUE', > object => $patron->borrowernumber, >- info => { '!=', $item->itemnumber } # Exclude the first test log >+ info => { 'LIKE', '%' . $item2->itemnumber . '%' } # Find the second test log by itemnumber > }, > { 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 ) }; >+ $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' ); >-- >2.52.0 >
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 41358
: 190624 |
190625
|
190626
|
190627