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

Bug 29016: (QA follow-up) Expand action_logs API and test coverage 

Build on the new ActionLog DBIC relations to expose them as opt-in
embeds, and shore up gaps in the test coverage that QA flagged.

Swagger
  * paths/action_logs.yaml declares an x-koha-embed header parameter
    constrained to the supported relations (librarian, patron).
  * definitions/action_log.yaml adds the corresponding optional
    `librarian` and `patron` object fields.

Tests
  * Convert the single 24-test subtest into focused subtests:
    - authorization (403 vs 200)
    - simple typed filters
    - q-DSL: multi-value module, info LIKE, combined filter
    - embeds: librarian and patron, with and without x-koha-embed
    - pagination + X-Total-Count and order_by
  * Mock AuthSuccessLog/AuthFailureLog/BorrowersLog to 0 to remove the
    flakiness where the API GET itself appended an AUTH log row between
    delete and assertion.
  * Build the test librarian with both `tools` and `borrowers` flags so
    embedded patron fields are not redacted by Koha::Patron->is_accessible.
  * Use a small build_fixtures() helper to avoid copy-pasting setup
    across subtests.

Test plan:
  1. yarn build && restart_all  (refresh swagger bundle)
  2. prove t/db_dependent/api/v1/action_logs.t  (all subtests green)
  3. curl -H 'x-koha-embed: librarian,patron' \
       /api/v1/action_logs?module=MEMBERS  -- patron and librarian
       objects appear in the response.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-04-29 09:59:08 +01:00 committed by Lucas Gass
parent c963591c06
commit e7eb27d48e
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
3 changed files with 173 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
api/v1/swagger/definitions/action_log.yaml
View File

@ -52,4 +52,19 @@ properties:
- string
- "null"
description: A diff of the changed object
librarian:
type:
- object
- "null"
description: |
The patron object for the librarian who performed the action
(only present if requested via x-koha-embed)
patron:
type:
- object
- "null"
description: |
Patron object joined on `object` (only meaningful for MEMBERS,
CIRCULATION, FINES and APIKEYS rows; only present if requested
via x-koha-embed)
additionalProperties: false

11
api/v1/swagger/paths/action_logs.yaml
View File

@ -57,6 +57,17 @@
- $ref: "../swagger.yaml#/parameters/q_param"
- $ref: "../swagger.yaml#/parameters/q_body"
- $ref: "../swagger.yaml#/parameters/request_id_header"
- name: x-koha-embed
in: header
required: false
description: Embed list sent as a request header
type: array
items:
type: string
enum:
- librarian
- patron
collectionFormat: csv
responses:
"200":
description: A list of action logs

191
t/db_dependent/api/v1/action_logs.t
View File

@ -18,9 +18,11 @@
use Modern::Perl;
use Test::NoWarnings;
use Test::More tests => 2;
use Test::More tests => 6;
use Test::Mojo;
use Mojo::JSON qw( encode_json );
use t::lib::TestBuilder;
use t::lib::Mocks;
@ -31,25 +33,27 @@ use Koha::Database;
my $schema = Koha::Database->new->schema;
my $builder = t::lib::TestBuilder->new;
t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
t::lib::Mocks::mock_preference( 'AuthSuccessLog', 0 );
t::lib::Mocks::mock_preference( 'AuthFailureLog', 0 );
t::lib::Mocks::mock_preference( 'BorrowersLog', 0 );
my $t = Test::Mojo->new('Koha::REST::V1');
subtest 'list() tests' => sub {
plan tests => 24;
$schema->storage->txn_begin;
# Build credentials and an isolated set of seed log entries used by every
# subtest below.
sub build_fixtures {
# Need both `tools` (view_system_logs) and `borrowers` so embedded patron
# fields don't get redacted by Koha::Patron->is_accessible.
my $librarian = $builder->build_object(
{
class => 'Koha::Patrons',
value => { flags => 2**13 } # tools flag = 13
value => { flags => 2**13 + 2**4 } # tools + borrowers
}
);
my $password = 'thePassword123';
$librarian->set_password( { password => $password, skip_validation => 1 } );
my $userid = $librarian->userid;
my $patron = $builder->build_object(
{
@ -58,58 +62,157 @@ subtest 'list() tests' => sub {
}
);
$patron->set_password( { password => $password, skip_validation => 1 } );
Koha::ActionLogs->search->delete;
t::lib::Mocks::mock_userenv( { borrowernumber => $librarian->borrowernumber } );
return ( $librarian, $patron, $password );
}
subtest 'authorization' => sub {
plan tests => 4;
$schema->storage->txn_begin;
my ( $librarian, $patron, $password ) = build_fixtures();
my $userid = $librarian->userid;
my $unauth_userid = $patron->userid;
# Delete all logs after patron creation (building patrons can generate log entries)
Koha::ActionLogs->search->delete;
$t->get_ok("//$unauth_userid:$password@/api/v1/action_logs")
->status_is( 403, 'forbidden without view_system_logs' );
$t->get_ok("//$userid:$password@/api/v1/action_logs")->status_is( 200, 'allowed with tools/view_system_logs' );
## Unauthorized access
$t->get_ok("//$unauth_userid:$password@/api/v1/action_logs")->status_is(403);
$schema->storage->txn_rollback;
};
# Delete again after auth attempts (login can generate AUTH logs)
Koha::ActionLogs->search->delete;
subtest 'simple typed filters' => sub {
plan tests => 13;
$schema->storage->txn_begin;
## Authorized user tests
# No logs, so empty array should be returned
$t->get_ok("//$userid:$password@/api/v1/action_logs")->status_is(200)->json_is( [] );
my ( $librarian, $patron, $password ) = build_fixtures();
my $userid = $librarian->userid;
# Delete any logs created by the successful auth above
Koha::ActionLogs->search->delete;
# Create some log entries using a test-specific module to avoid collisions
t::lib::Mocks::mock_userenv( { borrowernumber => $librarian->borrowernumber } );
logaction( 'MEMBERS', 'MODIFY', $patron->borrowernumber, 'test info', 'INTRANET' );
logaction( 'CIRCULATION', 'ISSUE', $patron->borrowernumber, 'circ info', 'INTRANET' );
logaction( 'CATALOGUING', 'ADD', 42, 'biblio added', 'COMMANDLINE' );
# Should return all three logs
$t->get_ok("//$userid:$password@/api/v1/action_logs")->status_is(200);
my $logs = $t->tx->res->json;
is( scalar @$logs, 3, 'Three log entries returned' );
# Filter by module
$t->get_ok("//$userid:$password@/api/v1/action_logs?module=MEMBERS")->status_is(200);
$logs = $t->tx->res->json;
is( scalar @$logs, 1, 'One MEMBERS log entry returned' );
is( $logs->[0]->{module}, 'MEMBERS', 'Correct module returned' );
is( $logs->[0]->{info}, 'test info', 'Correct info returned' );
my $logs = $t->tx->res->json;
is( scalar @$logs, 1, 'one MEMBERS row' );
is( $logs->[0]->{module}, 'MEMBERS', 'correct module returned' );
is( $logs->[0]->{info}, 'test info', 'correct info returned' );
# Filter by action
$t->get_ok("//$userid:$password@/api/v1/action_logs?action=ADD")->status_is(200);
$logs = $t->tx->res->json;
is( scalar @$logs, 1, 'One ADD action log entry returned' );
is( $logs->[0]->{action}, 'ADD', 'Correct action returned' );
is( $t->tx->res->json->[0]->{action}, 'ADD', 'action filter works' );
# Filter by interface
$t->get_ok("//$userid:$password@/api/v1/action_logs?interface=COMMANDLINE")->status_is(200);
$logs = $t->tx->res->json;
is( scalar @$logs, 1, 'One COMMANDLINE log entry returned' );
is( $logs->[0]->{interface}, 'COMMANDLINE', 'Correct interface returned' );
is( $t->tx->res->json->[0]->{interface}, 'COMMANDLINE', 'interface filter works' );
# Warn on unsupported query parameter
$t->get_ok("//$userid:$password@/api/v1/action_logs?bork=borkbork")
->status_is(400)
->json_is( [ { path => '/query/bork', message => 'Malformed query string' } ] );
->status_is( 400, 'unsupported query param rejected' );
$schema->storage->txn_rollback;
};
subtest 'q DSL: multi-value, like, range' => sub {
plan tests => 13;
$schema->storage->txn_begin;
my ( $librarian, $patron, $password ) = build_fixtures();
my $userid = $librarian->userid;
logaction( 'MEMBERS', 'MODIFY', $patron->borrowernumber, 'flag toggled', 'INTRANET' );
logaction( 'CIRCULATION', 'ISSUE', $patron->borrowernumber, 'item issued', 'INTRANET' );
logaction( 'CATALOGUING', 'ADD', 42, 'biblio added', 'COMMANDLINE' );
logaction( 'CATALOGUING', 'MODIFY', 42, 'biblio edited', 'COMMANDLINE' );
# Multi-value module via q
my $q_multi = encode_json( { module => [ 'MEMBERS', 'CIRCULATION' ] } );
$t->get_ok("//$userid:$password@/api/v1/action_logs?q=$q_multi")->status_is(200);
is( scalar @{ $t->tx->res->json }, 2, 'multi-value module filter returns union' );
# Substring (LIKE) on info
my $q_like = encode_json( { info => { -like => '%biblio%' } } );
$t->get_ok("//$userid:$password@/api/v1/action_logs?q=$q_like")->status_is(200);
is( scalar @{ $t->tx->res->json }, 2, 'like operator on info matches both biblio rows' );
# Object filter via typed param
$t->get_ok("//$userid:$password@/api/v1/action_logs?object=42")->status_is(200);
is( scalar @{ $t->tx->res->json }, 2, 'object filter returns rows for object 42' );
# Combined filter
my $q_combo = encode_json(
{
module => 'CATALOGUING',
action => 'MODIFY'
}
);
$t->get_ok("//$userid:$password@/api/v1/action_logs?q=$q_combo")->status_is(200);
my $combo = $t->tx->res->json;
is( scalar @$combo, 1, 'combined filter narrows to one row' );
is( $combo->[0]->{action}, 'MODIFY', 'combined filter returned MODIFY' );
$schema->storage->txn_rollback;
};
subtest 'embeds' => sub {
plan tests => 10;
$schema->storage->txn_begin;
my ( $librarian, $patron, $password ) = build_fixtures();
my $userid = $librarian->userid;
logaction( 'MEMBERS', 'MODIFY', $patron->borrowernumber, 'embed test', 'INTRANET' );
logaction( 'CATALOGUING', 'ADD', 42, 'biblio', 'COMMANDLINE' );
# Without embed, no librarian/patron objects in payload
$t->get_ok("//$userid:$password@/api/v1/action_logs?module=MEMBERS")->status_is(200);
my $bare = $t->tx->res->json->[0];
ok( !exists $bare->{librarian}, 'no librarian key without embed' );
ok( !exists $bare->{patron}, 'no patron key without embed' );
# With embed=librarian, librarian object populated
$t->get_ok( "/api/v1/action_logs?module=MEMBERS" =>
{ 'x-koha-embed' => 'librarian', Authorization => _basic( $userid, $password ) } )->status_is(200);
my $embed_lib = $t->tx->res->json->[0];
is(
$embed_lib->{librarian}->{patron_id},
$librarian->borrowernumber,
'embedded librarian.patron_id matches the actor'
);
# With embed=patron, patron object populated for MEMBERS row
$t->get_ok( "/api/v1/action_logs?module=MEMBERS" =>
{ 'x-koha-embed' => 'patron', Authorization => _basic( $userid, $password ) } )->status_is(200);
my $embed_pat = $t->tx->res->json->[0];
is(
$embed_pat->{patron}->{patron_id},
$patron->borrowernumber,
'embedded patron.patron_id matches the row object'
);
$schema->storage->txn_rollback;
};
subtest 'pagination and sorting' => sub {
plan tests => 6;
$schema->storage->txn_begin;
my ( $librarian, $patron, $password ) = build_fixtures();
my $userid = $librarian->userid;
logaction( 'MEMBERS', 'MODIFY', $patron->borrowernumber, "row $_", 'INTRANET' ) for 1 .. 5;
$t->get_ok("//$userid:$password@/api/v1/action_logs?_per_page=2&_page=1&module=MEMBERS")->status_is(200);
is( scalar @{ $t->tx->res->json }, 2, 'pagination limits results' );
is( $t->tx->res->headers->header('x-total-count'), 5, 'X-Total-Count reflects unfiltered total' );
$t->get_ok("//$userid:$password@/api/v1/action_logs?_order_by=-action_id&module=MEMBERS")->status_is(200);
$schema->storage->txn_rollback;
};
sub _basic {
my ( $u, $p ) = @_;
require MIME::Base64;
return 'Basic ' . MIME::Base64::encode_base64( "$u:$p", '' );
}