From 20089f67d1d212de6479b330002d550d85f027de Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 15 Oct 2025 15:31:02 +0300 Subject: [PATCH] Bug 25673: Log patron view actions in staff client and REST API This patch adds a crude logging of patron view events. It adds logging to the following REST API endpoints - /api/v1/patrons - /api/v1/patrons/{patron_id} In staff client, instead of defining the log event on each page individually, we log every authorized staff client page request that contains a borrowernumber as a request parameter. To test staff client: 1. Enable system preference BorrowersViewLog 2. Navigate to patrons 3. Without specifying any filters, click "Search" 4. Click on one of the borrowers 5. Navigate to "Modification log" of this patron 6. Observe two rows Module Action Info Interface "Patrons" "VIEW" "members/moremember" "Staff interface" "Patrons" "VIEW" "list" "REST API" 7. Disable system preference BorrowersViewLog 8. Repeat steps 2-5 9. Observe no new rows To test REST API: 1. prove t/db_dependent/api/v1/patrons.t --- C4/Auth.pm | 16 +++++++- Koha/REST/V1/Patrons.pm | 10 +++++ t/db_dependent/api/v1/patrons.t | 66 ++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 20242735e1a..f028323ca50 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -437,8 +437,20 @@ sub get_template_and_user { } C4::Search::History::set_to_session( { cgi => $in->{'query'}, search_history => [] } ); - } elsif ( $in->{type} eq 'intranet' and C4::Context->preference('EnableSearchHistory') ) { - $template->param( EnableSearchHistory => 1 ); + } elsif ( $in->{type} eq 'intranet' ) { + if ( C4::Context->preference('EnableSearchHistory') ) { + $template->param( EnableSearchHistory => 1 ); + } + if ( C4::Context->preference('BorrowersViewLog') ) { + if ( defined $in->{'query'}->param('borrowernumber') ) { + my $log_info = $in->{'template_name'}; + $log_info =~ s/\.tt$//; + logaction( + 'MEMBERS', 'VIEW', $in->{'query'}->param('borrowernumber'), + $log_info, $in->{'type'} + ); + } + } } } else { # if this is an anonymous session, setup to display public lists... diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 7d733c10dc1..3e2b559f96c 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -56,6 +56,12 @@ sub list { my $patrons_rs = Koha::Patrons->search($query); my $patrons = $c->objects->search($patrons_rs); + if ( C4::Context->preference('BorrowersViewLog') && $patrons ) { + foreach my $patron (@$patrons) { + C4::Log::logaction( 'MEMBERS', 'VIEW', $patron->{patron_id}, 'list' ); + } + } + return $c->render( status => 200, openapi => $patrons @@ -81,6 +87,10 @@ sub get { return $c->render_resource_not_found("Patron") unless $patron; + if ( C4::Context->preference('BorrowersViewLog') ) { + C4::Log::logaction( 'MEMBERS', 'VIEW', $patron->{patron_id}, 'get' ); + } + return $c->render( status => 200, openapi => $patron diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index aff93e092b0..a06eb342dbc 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -83,7 +83,7 @@ subtest 'list() tests' => sub { subtest 'librarian access tests' => sub { - plan tests => 15; + plan tests => 16; $schema->storage->txn_begin; @@ -210,6 +210,36 @@ subtest 'list() tests' => sub { ->json_is( '/0/patron_id' => $patron->id, 'Filtering by date-time works' ); }; + subtest 'BorrowersViewLog' => sub { + plan tests => 6; + + t::lib::Mocks::mock_preference( 'BorrowersViewLog', 1 ); + + $t->get_ok("//$userid:$password@/api/v1/patrons")->status_is(200); + + my $count = Koha::ActionLogs->search({ + module => 'MEMBERS', + action => 'VIEW', + user => $librarian->borrowernumber, + interface => 'api' + })->count; + + ok($count>0, "BorrowersViewLog generated some view log entries"); + + t::lib::Mocks::mock_preference( 'BorrowersViewLog', 0 ); + + $t->get_ok("//$userid:$password@/api/v1/patrons")->status_is(200); + + my $off_count = Koha::ActionLogs->search({ + module => 'MEMBERS', + action => 'VIEW', + user => $librarian->borrowernumber, + interface => 'api' + })->count; + + is($off_count, $count, "BorrowersViewLog does not generate log entries when disabled"); + }; + $schema->storage->txn_rollback; }; @@ -277,7 +307,7 @@ subtest 'get() tests' => sub { subtest 'librarian access tests' => sub { - plan tests => 8; + plan tests => 9; $schema->storage->txn_begin; @@ -325,6 +355,38 @@ subtest 'get() tests' => sub { ->json_is( '/patron_id' => $patron->id )->json_is( '/category_id' => $patron->categorycode ) ->json_is( '/surname' => $patron->surname )->json_is( '/patron_card_lost' => Mojo::JSON->false ); + subtest 'BorrowersViewLog' => sub { + plan tests => 6; + + t::lib::Mocks::mock_preference( 'BorrowersViewLog', 1 ); + + $t->get_ok("//$userid:$password@/api/v1/patrons/" . $patron->id)->status_is(200); + + my $count = Koha::ActionLogs->search({ + module => 'MEMBERS', + action => 'VIEW', + user => $librarian->borrowernumber, + object => $patron->id, + interface => 'api' + })->count; + + ok($count>0, "BorrowersViewLog generated some view log entries for this patron"); + + t::lib::Mocks::mock_preference( 'BorrowersViewLog', 0 ); + + $t->get_ok("//$userid:$password@/api/v1/patrons/" . $patron->id)->status_is(200); + + my $off_count = Koha::ActionLogs->search({ + module => 'MEMBERS', + action => 'VIEW', + user => $librarian->borrowernumber, + object => $patron->id, + interface => 'api' + })->count; + + is($off_count, $count, "BorrowersViewLog does not generate log entries when disabled"); + }; + $schema->storage->txn_rollback; }; -- 2.34.1