From c963591c0634a014e8c114bdb343fe3f40265956 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Wed, 29 Apr 2026 09:58:41 +0100
Subject: [PATCH] Bug 29016: (QA follow-up) Add librarian/patron relations to
 ActionLog

QA review noted that the new /api/v1/action_logs endpoint exposed only
column data, so the log viewer ended up showing bare borrowernumbers in
the Librarian and Object columns where the old server-side rendering
showed firstname/surname (borrowernumber). Add the DBIC plumbing needed
to support x-koha-embed for those relationships.

Two belongs_to relationships are declared below the auto-generated
marker on Koha::Schema::Result::ActionLog:

  * librarian -> borrowers.borrowernumber via action_logs.user
  * patron    -> borrowers.borrowernumber via action_logs.object

The patron relation is intentionally polymorphic; the action_logs.object
column holds different identifiers per module (biblionumber for
CATALOGUING, subscriptionid for SERIAL, ...) so callers must filter by
module before trusting the joined patron - this is documented in both
schema and Koha::ActionLog comments.

Koha::ActionLog gains explicit librarian() and patron() accessor methods
following the Koha::Hold pattern, returning Koha::Patron objects (or
undef when the relation does not match). The accessors deliberately
avoid `use Koha::Patrons` at compile time: Koha::ActionLog is loaded
very early via C4::Log and Koha::Patron is not yet defined at that
point - the accessors reach for it lazily at call time.

koha_object_class / koha_objects_class overrides are also added to bring
the result class in line with the rest of the codebase.

Test plan:
  1. prove t/db_dependent/api/v1/action_logs.t  (still green)
  2. perl -MKoha::ActionLog -e 'print "ok"' (no circular import)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 Koha/ActionLog.pm               | 37 +++++++++++++++++++++++++++++++++
 Koha/Schema/Result/ActionLog.pm | 37 ++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Koha/ActionLog.pm b/Koha/ActionLog.pm
index 6a175bb6dd..410a46e09e 100644
--- a/Koha/ActionLog.pm
+++ b/Koha/ActionLog.pm
@@ -21,6 +21,11 @@ use Modern::Perl;
 
 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.
+
 use base qw(Koha::Object);
 
 =head1 NAME
@@ -31,8 +36,40 @@ Koha::ActionLog - Koha ActionLog Object class
 
 =head2 Class methods
 
+=head3 librarian
+
+    my $librarian = $log->librarian;
+
+Returns the related I<Koha::Patron> object for the librarian who performed
+the action, or I<undef> if the C<user> column was not set (e.g. cron jobs).
+
 =cut
 
+sub librarian {
+    my ($self) = @_;
+    my $rs = $self->_result->librarian;
+    return unless $rs;
+    return Koha::Patron->_new_from_dbic($rs);
+}
+
+=head3 patron
+
+    my $patron = $log->patron;
+
+Returns the related I<Koha::Patron> object joined on the C<object> column.
+Only meaningful for rows where C<object> is a borrowernumber (MEMBERS,
+CIRCULATION, FINES and APIKEYS modules); for other modules the join may
+match an unrelated patron and the caller must filter by C<module>.
+
+=cut
+
+sub patron {
+    my ($self) = @_;
+    my $rs = $self->_result->patron;
+    return unless $rs;
+    return Koha::Patron->_new_from_dbic($rs);
+}
+
 =head2 Internal methods
 
 =head3 _type
diff --git a/Koha/Schema/Result/ActionLog.pm b/Koha/Schema/Result/ActionLog.pm
index c97ffdcf21..2fa580cb2b 100644
--- a/Koha/Schema/Result/ActionLog.pm
+++ b/Koha/Schema/Result/ActionLog.pm
@@ -154,6 +154,41 @@ __PACKAGE__->set_primary_key("action_id");
 # Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-05-02 14:28:43
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:8ZJaG4iJAbay93LAg5xPsQ
 
+# Both relations below are pseudo-relations: action_logs.user and
+# action_logs.object have no SQL foreign-key constraint on borrowers,
+# so we set is_foreign_key_constraint => 0 to keep TestBuilder from
+# trying to materialise a Borrower row when building action_logs
+# fixtures.
+__PACKAGE__->belongs_to(
+    "librarian",
+    "Koha::Schema::Result::Borrower",
+    { borrowernumber => "user" },
+    {
+        is_foreign_key_constraint => 0,
+        join_type                 => "LEFT",
+    },
+);
+
+# `object` is polymorphic across modules (borrowernumber, biblionumber,
+# subscriptionid, ...). The relation is only meaningful for MEMBERS,
+# CIRCULATION, FINES and APIKEYS rows; other modules may produce a
+# coincidentally-matching borrowernumber row that callers must ignore.
+__PACKAGE__->belongs_to(
+    "patron",
+    "Koha::Schema::Result::Borrower",
+    { borrowernumber => "object" },
+    {
+        is_foreign_key_constraint => 0,
+        join_type                 => "LEFT",
+    },
+);
+
+sub koha_object_class {
+    'Koha::ActionLog';
+}
+
+sub koha_objects_class {
+    'Koha::ActionLogs';
+}
 
-# You can replace this text with custom code or comments, and it will be preserved on regeneration
 1;