diff --git a/Koha/ActionLog.pm b/Koha/ActionLog.pm index 410a46e09e..faf4f9fc95 100644 --- a/Koha/ActionLog.pm +++ b/Koha/ActionLog.pm @@ -19,12 +19,14 @@ package Koha::ActionLog; use Modern::Perl; +use JSON qw( decode_json ); + use C4::Context; # Avoid `use Koha::Patrons` here: Koha::ActionLog is loaded transitively by # C4::Log very early in the dependency chain, before Koha::Patron has been -# fully defined. The accessors below reach for Koha::Patron at call time, -# by which point it is reliably available. +# fully defined. The accessors below reach for Koha::Patron (and Koha::Items) +# at call time, by which point they are reliably available. use base qw(Koha::Object); @@ -70,6 +72,42 @@ sub patron { return Koha::Patron->_new_from_dbic($rs); } +=head3 item + + my $item = $log->item; + +Returns the related I for a CIRCULATION log entry, or I. + +The itemnumber is not stored in a dedicated column; it lives inside the +C field, which for circulation is either a JSON payload carrying an +C key (ISSUE, RENEWAL) or a bare itemnumber string (RETURN). +Returns I for non-circulation rows, when no itemnumber can be +derived, or when the item has since been deleted. + +=cut + +sub item { + my ($self) = @_; + + return unless ( $self->module // q{} ) eq 'CIRCULATION'; + + my $info = $self->info; + return unless defined $info; + + my $itemnumber; + my $decoded = eval { decode_json($info) }; + if ( ref($decoded) eq 'HASH' && $decoded->{itemnumber} ) { + $itemnumber = $decoded->{itemnumber}; + } elsif ( $info =~ /^\s*(\d+)\s*$/ ) { + $itemnumber = $1; + } + + return unless $itemnumber; + + require Koha::Items; + return Koha::Items->find($itemnumber); +} + =head2 Internal methods =head3 _type diff --git a/api/v1/swagger/definitions/action_log.yaml b/api/v1/swagger/definitions/action_log.yaml index 719a2622d1..ac0c5eb8fc 100644 --- a/api/v1/swagger/definitions/action_log.yaml +++ b/api/v1/swagger/definitions/action_log.yaml @@ -67,4 +67,12 @@ properties: Patron object joined on `object` (only meaningful for MEMBERS, CIRCULATION, FINES and APIKEYS rows; only present if requested via x-koha-embed) + item: + type: + - object + - "null" + description: | + Item object derived from the itemnumber carried in `info` (only + meaningful for CIRCULATION rows; only present if requested via + x-koha-embed) additionalProperties: false diff --git a/api/v1/swagger/paths/action_logs.yaml b/api/v1/swagger/paths/action_logs.yaml index abbe580916..15606a8308 100644 --- a/api/v1/swagger/paths/action_logs.yaml +++ b/api/v1/swagger/paths/action_logs.yaml @@ -67,6 +67,7 @@ enum: - librarian - patron + - item collectionFormat: csv responses: "200": diff --git a/koha-tmpl/intranet-tmpl/prog/js/viewlog.js b/koha-tmpl/intranet-tmpl/prog/js/viewlog.js index 4fb0c964e6..522a9e34aa 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/viewlog.js +++ b/koha-tmpl/intranet-tmpl/prog/js/viewlog.js @@ -124,6 +124,109 @@ function renderCircInfo(raw) { return html; } +/** + * Parse a log `info` payload as a JSON object, returning the parsed object + * or null when it isn't a JSON object. + */ +function parseInfoObject(raw) { + if (!raw) return null; + const trimmed = raw.replace(/^\s+|\s+$/g, ""); + if (trimmed.charAt(0) !== "{") return null; + try { + const parsed = JSON.parse(trimmed); + return parsed && typeof parsed === "object" ? parsed : null; + } catch (e) { + return null; + } +} + +/** + * Build the "Item " link for a CIRCULATION log entry. The item is + * embedded on the row based on the itemnumber carried in `info`. For JSON + * payloads (ISSUE/RENEWAL) the item's branchcode is appended after the barcode, + * Returns the anchor markup wrapped in a
, or null when no item is + * embedded (e.g. the item has since been deleted). + */ +function renderCircItemLink(row) { + const item = row.item; + if (!item) return null; + + const itemnumber = item.item_id; + const barcode = item.external_id != null ? item.external_id : itemnumber; + const biblionumber = item.biblio_id; + + const parsed = parseInfoObject(row.info); + const branchcode = parsed && parsed.branchcode ? parsed.branchcode : null; + + const encodedItem = encodeURIComponent(itemnumber); + const encodedBiblio = encodeURIComponent(biblionumber); + const label = branchcode + ? __("Item %s %s").format(escapeHtml(barcode), escapeHtml(branchcode)) + : __("Item %s").format(escapeHtml(barcode)); + + return ( + "
" + + '' + + label + + "" + + "
" + ); +} + +/** + * Build the hold link (and optional item link) for a HOLDS log entry whose + * `info` payload is a JSON object carrying `hold` and `biblionumber`. + * Returns the markup wrapped in a
, or null. + */ +function renderHoldsInfoLink(row) { + const parsed = parseInfoObject(row.info); + if (!parsed || parsed.hold == null || parsed.biblionumber == null) { + return null; + } + + const encodedBiblio = encodeURIComponent(parsed.biblionumber); + let html = + '' + + __("Hold %s on biblio %s").format( + escapeHtml(parsed.hold), + escapeHtml(parsed.biblionumber) + ) + + ""; + + if (parsed.itemnumber != null) { + const encodedItem = encodeURIComponent(parsed.itemnumber); + html += + " - " + + '' + + __("Item %s").format(escapeHtml(parsed.itemnumber)) + + ""; + } + + return "
" + html + "
"; +} + /** * Render Struct::Diff JSON as a human-readable before/after table. * Falls back to a
 block for non-Struct::Diff JSON or plain text.
@@ -393,21 +496,30 @@ function renderInfo(data, type, row) {
         );
     }
 
-    var body = null;
-    if (mod == "CIRCULATION" || mod == "HOLDS") {
+    let prefix = "";
+    let body = null;
+    if (mod == "CIRCULATION") {
+        prefix = renderCircItemLink(row) || "";
+        body = renderCircInfo(info);
+    } else if (mod == "HOLDS") {
+        prefix = renderHoldsInfoLink(row) || "";
         body = renderCircInfo(info);
     }
-    if (body === null) {
+    // Only fall back to the record/plain-text renderers when we produced
+    // neither a link nor any override text, so we never dump the raw JSON
+    // payload alongside a rendered item/hold link.
+    if (body === null && prefix === "") {
         body = renderRecordInfo(info);
     }
-    if (body === null) {
+    if (body === null && prefix === "") {
         body = escapeHtml(info);
     }
     return (
         '
' + - body + + prefix + + (body || "") + "
" ); } @@ -567,7 +679,7 @@ $(document).ready(function () { ajax: { url: "/api/v1/action_logs", }, - embed: ["librarian", "patron"], + embed: ["librarian", "patron", "item"], order: [[0, "desc"]], pagingType: "full", autoWidth: false, diff --git a/t/db_dependent/api/v1/action_logs.t b/t/db_dependent/api/v1/action_logs.t index 7bb73ba4fe..f4a1316638 100755 --- a/t/db_dependent/api/v1/action_logs.t +++ b/t/db_dependent/api/v1/action_logs.t @@ -18,7 +18,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 6; +use Test::More tests => 7; use Test::Mojo; use Mojo::JSON qw( encode_json ); @@ -193,6 +193,63 @@ subtest 'embeds' => sub { $schema->storage->txn_rollback; }; +subtest 'item embed (CIRCULATION)' => sub { + plan tests => 14; + $schema->storage->txn_begin; + + my ( $librarian, $patron, $password ) = build_fixtures(); + my $userid = $librarian->userid; + my $auth = { Authorization => _basic( $userid, $password ) }; + + my $item = $builder->build_sample_item; + + # ISSUE/RENEWAL log a JSON payload carrying the itemnumber; RETURN logs a + # bare itemnumber string. Both must resolve to the same embedded item. + my $json_info = encode_json( + { + issue => 1, + branchcode => $item->homebranch, + itemnumber => $item->itemnumber, + confirmations => [], + forced => [], + } + ); + logaction( 'CIRCULATION', 'ISSUE', $patron->borrowernumber, $json_info, 'INTRANET' ); + logaction( 'CIRCULATION', 'RETURN', $patron->borrowernumber, $item->itemnumber, 'INTRANET' ); + logaction( 'MEMBERS', 'MODIFY', $patron->borrowernumber, 'no item here', 'INTRANET' ); + + # JSON payload (ISSUE): item resolved from info.itemnumber + $t->get_ok( "/api/v1/action_logs?module=CIRCULATION&action=ISSUE" => { 'x-koha-embed' => 'item', %$auth } ) + ->status_is(200); + my $issue = $t->tx->res->json->[0]; + is( $issue->{item}->{item_id}, $item->itemnumber, 'ISSUE: embedded item_id from JSON info' ); + is( $issue->{item}->{external_id}, $item->barcode, 'ISSUE: embedded barcode (external_id)' ); + is( $issue->{item}->{biblio_id}, $item->biblionumber, 'ISSUE: embedded biblio_id' ); + + # Bare itemnumber payload (RETURN) + $t->get_ok( "/api/v1/action_logs?module=CIRCULATION&action=RETURN" => { 'x-koha-embed' => 'item', %$auth } ) + ->status_is(200); + is( $t->tx->res->json->[0]->{item}->{external_id}, $item->barcode, 'RETURN: item resolved from bare itemnumber' ); + + # Non-CIRCULATION row: item is null even when the embed is requested + $t->get_ok( "/api/v1/action_logs?module=MEMBERS" => { 'x-koha-embed' => 'item', %$auth } )->status_is(200); + is( $t->tx->res->json->[0]->{item}, undef, 'non-CIRCULATION row has null item' ); + + # Deleted item: CIRCULATION row whose itemnumber no longer exists -> null + my $gone = $builder->build_sample_item; + my $gone_id = $gone->itemnumber; + $gone->delete; + logaction( + 'CIRCULATION', 'RENEWAL', $patron->borrowernumber, encode_json( { itemnumber => $gone_id } ), + 'INTRANET' + ); + $t->get_ok( "/api/v1/action_logs?module=CIRCULATION&action=RENEWAL" => { 'x-koha-embed' => 'item', %$auth } ) + ->status_is(200); + is( $t->tx->res->json->[0]->{item}, undef, 'deleted item resolves to null' ); + + $schema->storage->txn_rollback; +}; + subtest 'pagination and sorting' => sub { plan tests => 6; $schema->storage->txn_begin;