From 1a1a6ac817030be1ba6bd802a5dc517cb75d645f Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Thu, 12 Oct 2023 12:53:34 +0100
Subject: [PATCH] Bug 35030: Add 'article' to patron activity triggers
Content-Type: text/plain; charset=utf-8

This patch adds 'article' to the list of triggers available for
tracking patron activity.

Test plan
1) Select 'Placing an article request
   TrackPatronLastActivityTriggers system preference
2) As a staff member, place a hold on any item for a test user
3) Confirm that the borrowers.lastseen field is updated for that
   test borrower

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 Koha/ArticleRequest.pm                        | 14 +++++++++++++
 .../en/modules/admin/preferences/patrons.pref |  1 +
 t/db_dependent/Koha/ArticleRequest.t          | 10 +++++++--
 t/db_dependent/Koha/Patron.t                  | 21 ++++++++++++++++---
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm
index 42d2d011f1..1bfbda3484 100644
--- a/Koha/ArticleRequest.pm
+++ b/Koha/ArticleRequest.pm
@@ -64,6 +64,7 @@ sub request {
         if $debit;
 
     $self->store();
+    $self->patron->update_lastseen('article');
     $self->notify();
     return $self;
 }
@@ -226,6 +227,19 @@ sub borrower {
     return Koha::Patron->_new_from_dbic($rs);
 }
 
+=head3 patron
+
+Returns the Koha::Patron object for this article request
+
+=cut
+
+sub patron {
+    my ($self) = @_;
+    my $rs = $self->_result->borrowernumber;
+    return unless $rs;
+    return Koha::Patron->_new_from_dbic($rs);
+}
+
 =head3 branch
 
 Returns the Koha::Library object for this article request
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
index 2f416cb2e9..bb3a6adc30 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
@@ -102,6 +102,7 @@ Patrons:
                renewal: "Renewing an item"
                check_in: "Returning an item"
                hold: "Placing a hold on an item"
+               article: "Places an article request"
      -
          - pref: AutoApprovePatronProfileSettings
            choices:
diff --git a/t/db_dependent/Koha/ArticleRequest.t b/t/db_dependent/Koha/ArticleRequest.t
index 9a5501bff2..2d6819e763 100755
--- a/t/db_dependent/Koha/ArticleRequest.t
+++ b/t/db_dependent/Koha/ArticleRequest.t
@@ -30,7 +30,7 @@ my $builder = t::lib::TestBuilder->new;
 
 subtest 'request() tests' => sub {
 
-    plan tests => 11;
+    plan tests => 13;
 
     $schema->storage->txn_begin;
 
@@ -39,7 +39,7 @@ subtest 'request() tests' => sub {
     my $patron_mock = Test::MockModule->new('Koha::Patron');
     $patron_mock->mock( 'article_request_fee', sub { return $amount; } );
 
-    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { lastseen => undef } } );
     my $item   = $builder->build_sample_item;
 
     my $ar_mock = Test::MockModule->new('Koha::ArticleRequest');
@@ -52,6 +52,7 @@ subtest 'request() tests' => sub {
         }
     );
 
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' );
     $ar->request()->discard_changes;
 
     is( $ar->status, Koha::ArticleRequest::Status::Requested );
@@ -59,6 +60,8 @@ subtest 'request() tests' => sub {
 
     is( $ar->debit_id, undef, 'No fee linked' );
     is( $patron->account->balance, 0, 'No outstanding fees' );
+    $patron->discard_changes;
+    is( $patron->lastseen, undef, 'Patron activity not tracked when article is not a valid trigger' );
 
     # set a fee amount
     $amount = 10;
@@ -71,6 +74,7 @@ subtest 'request() tests' => sub {
         }
     );
 
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'article' );
     $ar->request()->discard_changes;
 
     is( $ar->status, Koha::ArticleRequest::Status::Requested );
@@ -79,6 +83,8 @@ subtest 'request() tests' => sub {
 
     ok( defined $ar->debit_id, 'Fee linked' );
     is( $patron->account->balance, $amount, 'Outstanding fees with the right value' );
+    $patron->discard_changes;
+    isnt( $patron->lastseen, undef, 'Patron activity tracked when article is a valid trigger' );
 
     $schema->storage->txn_rollback;
 };
diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
index ba4e30179b..e5b835553b 100755
--- a/t/db_dependent/Koha/Patron.t
+++ b/t/db_dependent/Koha/Patron.t
@@ -1954,7 +1954,7 @@ subtest 'alert_subscriptions tests' => sub {
 
 subtest 'update_lastseen tests' => sub {
 
-    plan tests => 21;
+    plan tests => 24;
     $schema->storage->txn_begin;
 
     my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
@@ -1969,7 +1969,7 @@ subtest 'update_lastseen tests' => sub {
 
     t::lib::Mocks::mock_preference(
         'TrackLastPatronActivityTriggers',
-        'login,connection,check_in,check_out,renewal,hold'
+        'login,connection,check_in,check_out,renewal,hold,article'
     );
 
     is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' );
@@ -2003,6 +2003,9 @@ subtest 'update_lastseen tests' => sub {
     $patron->update_lastseen('hold');
     $patron->_result()->discard_changes();
     is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a hold' );
+    $patron->update_lastseen('article');
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a article' );
 
     # Check that tracking is disabled when the activity isn't listed
     t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' );
@@ -2048,11 +2051,17 @@ subtest 'update_lastseen tests' => sub {
         $patron->lastseen, $last_seen,
         'Patron last seen should be unchanged after a hold if hold is not selected as an option and the cache has been cleared'
     );
+    $patron->update_lastseen('article');
+    $patron->_result()->discard_changes();
+    is(
+        $patron->lastseen, $last_seen,
+        'Patron last seen should be unchanged after an article request if article is not selected as an option and the cache has been cleared'
+    );
 
     # Check tracking for each activity
     t::lib::Mocks::mock_preference(
         'TrackLastPatronActivityTriggers',
-        'login,connection,check_in,check_out,renewal,hold'
+        'login,connection,check_in,check_out,renewal,hold,article'
     );
 
     $cache->clear_from_cache($cache_key);
@@ -2091,6 +2100,12 @@ subtest 'update_lastseen tests' => sub {
         $patron->lastseen, $last_seen,
         'Patron last seen should be changed after a hold if we cleared the cache'
     );
+    $patron->update_lastseen('article');
+    $patron->_result()->discard_changes();
+    isnt(
+        $patron->lastseen, $last_seen,
+        'Patron last seen should be changed after a article if we cleared the cache'
+    );
 
     $cache->clear_from_cache($cache_key);
     $patron->update_lastseen('renewal');
-- 
2.30.2