Bug 43119: Restore item and hold links to the log viewer

INSERT INTO action_logs (timestamp, user, module, action, object, info, interface) VALUES
    (NOW(), 5, 'CIRCULATION', 'ISSUE', 5,
     '{"issue":1,"branchcode":"CPL","itemnumber":1,"confirmations":["TOO_MANY"],"forced":["DEBT"]}', 'intranet'),
    (NOW(), 5, 'CIRCULATION', 'RETURN', 5,
     '1', 'intranet'),
    (NOW(), 5, 'HOLDS', 'CREATE', 7,
     '{"hold":7,"biblionumber":1,"itemnumber":1,"confirmations":["HIGHHOLDS"],"forced":["HOLD_POLICY_OVERRIDE"]}', 'intranet');

Test plan:
1) Without the patch applied, run the populate SQL above against the kohadev database.
2) Go to Tools > Log viewer
3) Under Modules, select only Circulation - leave the date range empty and click Submit.
4) Observe the ISSUE row's Info column shows the raw JSON payload as plain text, and the RETURN row shows the bare itemnumber "1" as plain text - neither is a clickable link to the item.
5) Go back, select only the Holds module, click Submit, and observe the CREATE row's Info column likewise shows raw JSON with no hold/item links.
6) Apply the patch.
7) Run yarn build && restart_all, then hard refresh the browser.
8) Repeat step 3.
9) Confirm the ISSUE row's Info column now shows a link reading "Item <barcode> CPL" (e.g. Item 3999900000001 CPL) - the branchcode comes from the JSON payload - and clicking it opens the item page. Override information is also displayed correctly
10) Confirm the RETURN row's Info column now shows a link reading "Item <barcode>" (no branchcode) opening the same item's detail page - verifying the bare-itemnumber payload path.
11) Repeat step 5. Confirm the CREATE row now shows "Hold 7 on biblio 1" linking to reserve/request.pl?biblionumber=1, followed by " - Item 1" linking to the item detail page. Override information is also displayed correctly

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

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<Koha::Item> for a CIRCULATION log entry, or I<undef>.
+
+The itemnumber is not stored in a dedicated column; it lives inside the
+C<info> field, which for circulation is either a JSON payload carrying an
+C<itemnumber> key (ISSUE, RENEWAL) or a bare itemnumber string (RETURN).
+Returns I<undef> 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

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

api/v1/swagger/paths/action_logs.yaml

@@ -67,6 +67,7 @@
           enum:
             - librarian
             - patron
+            - item
         collectionFormat: csv
     responses:
       "200":

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 <barcode>" 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 <div>, 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 (
+        "<div>" +
+        '<a href="/cgi-bin/koha/catalogue/moredetail.pl?itemnumber=' +
+        encodedItem +
+        "&biblionumber=" +
+        encodedBiblio +
+        "&bi=" +
+        encodedBiblio +
+        "#item" +
+        encodedItem +
+        '" title="' +
+        __("Display detail for this item") +
+        '">' +
+        label +
+        "</a>" +
+        "</div>"
+    );
+}
+
+/**
+ * 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 <div>, 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 =
+        '<a href="/cgi-bin/koha/reserve/request.pl?biblionumber=' +
+        encodedBiblio +
+        '" title="' +
+        __("Display holds for this record") +
+        '">' +
+        __("Hold %s on biblio %s").format(
+            escapeHtml(parsed.hold),
+            escapeHtml(parsed.biblionumber)
+        ) +
+        "</a>";
+
+    if (parsed.itemnumber != null) {
+        const encodedItem = encodeURIComponent(parsed.itemnumber);
+        html +=
+            " - " +
+            '<a href="/cgi-bin/koha/catalogue/moredetail.pl?itemnumber=' +
+            encodedItem +
+            "&biblionumber=" +
+            encodedBiblio +
+            "#item" +
+            encodedItem +
+            '" title="' +
+            __("Display detail for this item") +
+            '">' +
+            __("Item %s").format(escapeHtml(parsed.itemnumber)) +
+            "</a>";
+    }
+
+    return "<div>" + html + "</div>";
+}
+
 /**
  * Render Struct::Diff JSON as a human-readable before/after table.
  * Falls back to a <pre> 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 (
         '<div class="loginfo" id="loginfo' +
         escapeHtml(action_id) +
         '">' +
-        body +
+        prefix +
+        (body || "") +
         "</div>"
     );
 }
@@ -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,

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;