From c1e0e75883a25ce4ee5ca4d0022e7caa415e8f15 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Wed, 21 Jun 2023 08:21:59 +0100
Subject: [PATCH] Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 t/db_dependent/Auth.t         | 104 +-----------------------------
 t/db_dependent/Koha/Patron.t  | 115 +++++++++++++++++++++++++++++++++-
 t/db_dependent/Koha/Patrons.t |   4 +-
 t/db_dependent/Members.t      |   6 +-
 4 files changed, 122 insertions(+), 107 deletions(-)

diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t
index a67ae3f695..a3a2a11217 100755
--- a/t/db_dependent/Auth.t
+++ b/t/db_dependent/Auth.t
@@ -7,7 +7,7 @@ use CGI qw ( -utf8 );
 use Test::MockObject;
 use Test::MockModule;
 use List::MoreUtils qw/all any none/;
-use Test::More tests => 19;
+use Test::More tests => 18;
 use Test::Warn;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
@@ -22,7 +22,7 @@ use Koha::Auth::TwoFactorAuth;
 BEGIN {
     use_ok(
         'C4::Auth',
-        qw( checkauth haspermission track_login_daily checkpw get_template_and_user checkpw_hash get_cataloguing_page_permissions )
+        qw( checkauth haspermission checkpw get_template_and_user checkpw_hash get_cataloguing_page_permissions )
     );
 }
 
@@ -371,106 +371,6 @@ subtest 'checkauth() tests' => sub {
     C4::Context->_new_userenv; # For next tests
 };
 
-subtest 'track_login_daily tests' => sub {
-
-    plan tests => 18;
-
-    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
-    my $userid = $patron->userid;
-
-    $patron->lastseen( undef );
-    $patron->store();
-
-    my $cache     = Koha::Caches->get_instance();
-    my $cache_key = "track_login_" . $patron->userid;
-    $cache->clear_from_cache($cache_key);
-
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' );
-
-    is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' );
-
-    C4::Auth::track_login_daily( $userid, 'login' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, undef, 'Patron should have last seen set when TrackLastPatronActivity = 1' );
-
-    sleep(1); # We need to wait a tiny bit to make sure the timestamp will be different
-    my $last_seen = $patron->lastseen;
-    C4::Auth::track_login_daily( $userid, 'login' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a login' );
-    C4::Auth::track_login_daily( $userid, 'connection' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a SIP/ILSDI connection' );
-    C4::Auth::track_login_daily( $userid, 'check_out' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a check out' );
-    C4::Auth::track_login_daily( $userid, 'check_in' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a check in' );
-    C4::Auth::track_login_daily( $userid, 'renewal' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a renewal' );
-
-    # Check that removing options stops tracking changes
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection,check_in,check_out,renewal' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'login' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a login if login is not selected as an option and the cache has been cleared' );
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'check_in,check_out,renewal' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'connection' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a connection if connection is not selected as an option and the cache has been cleared' );
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'check_out,renewal' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'check_in' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a check_in if check_in is not selected as an option and the cache has been cleared' );
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'renewal' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'check_out' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a check_out if check_out is not selected as an option and the cache has been cleared' );
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'renewal' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a renewal if renewal is not selected as an option and the cache has been cleared' );
-
-    # Restore all options to test changes
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' );
-
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'login' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a login if we cleared the cache' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'connection' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a connection if we cleared the cache' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'check_out' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a check_out if we cleared the cache' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'check_in' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a check_in if we cleared the cache' );
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'renewal' );
-    $patron->_result()->discard_changes();
-    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a renewal if we cleared the cache' );
-
-    t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
-    $patron->lastseen( undef )->store;
-    $cache->clear_from_cache($cache_key);
-    C4::Auth::track_login_daily( $userid, 'login' );
-    $patron->_result()->discard_changes();
-    is( $patron->lastseen, undef, 'Patron should still have last seen unchanged when TrackLastPatronActivity = 0' );
-};
-
 subtest 'no_set_userenv parameter tests' => sub {
 
     plan tests => 7;
diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
index 5d05a16efd..9912b5e326 100755
--- a/t/db_dependent/Koha/Patron.t
+++ b/t/db_dependent/Koha/Patron.t
@@ -19,9 +19,10 @@
 
 use Modern::Perl;
 
-use Test::More tests => 24;
+use Test::More tests => 25;
 use Test::Exception;
 use Test::Warn;
+use Time::Fake;
 
 use Koha::CirculationRules;
 use Koha::Database;
@@ -1498,3 +1499,115 @@ subtest 'update privacy tests' => sub {
     is( $old_checkout->borrowernumber, $anon_patron->id, "Checkout is successfully anonymized");
     is( $patron->privacy(), 2, "Patron privacy is successfully updated");
 };
+
+subtest 'update_lastseen tests' => sub {
+
+    plan tests => 18;
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    my $userid = $patron->userid;
+
+    $patron->lastseen( undef );
+    $patron->store();
+
+    my $cache     = Koha::Caches->get_instance();
+    my $cache_key = "track_login_" . $patron->userid;
+    $cache->clear_from_cache($cache_key);
+
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' );
+
+    is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' );
+
+    my $now = dt_from_string();
+    Time::Fake->offset( $now->epoch );
+
+    $patron->update_lastseen( 'login' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, undef, 'Patron should have last seen set when TrackLastPatronActivity = 1' );
+    my $last_seen = $patron->lastseen;
+
+    Time::Fake->offset( $now->epoch + 5 );
+    # Test that lastseen isn't updated more than once in a day (regardless of activity passed)
+    $patron->update_lastseen( 'login' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a login' );
+    $patron->update_lastseen( 'connection' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a SIP/ILSDI connection' );
+    $patron->update_lastseen( 'check_out' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a check out' );
+    $patron->update_lastseen( 'check_in' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a check in' );
+    $patron->update_lastseen( 'renewal' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a renewal' );
+
+    # Check that tracking is disabled when the activity isn't listed
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' );
+    $cache->clear_from_cache($cache_key); # Rule out the more than once day prevention above
+
+    $patron->update_lastseen( 'login' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a login if login is not selected as an option and the cache has been cleared' );
+
+    $patron->update_lastseen( 'connection' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a connection if connection is not selected as an option and the cache has been cleared' );
+
+    $patron->update_lastseen( 'check_in' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a check_in if check_in is not selected as an option and the cache has been cleared' );
+
+    $patron->update_lastseen( 'check_out' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a check_out if check_out is not selected as an option and the cache has been cleared' );
+
+    $patron->update_lastseen( 'renewal' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a renewal if renewal 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' );
+
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'login' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a login if we cleared the cache' );
+
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'connection' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a connection if we cleared the cache' );
+
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'check_out' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a check_out if we cleared the cache' );
+
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'check_in' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a check_in if we cleared the cache' );
+
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'renewal' );
+    $patron->_result()->discard_changes();
+    isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a renewal if we cleared the cache' );
+
+    # Check that the preference takes effect
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
+    $patron->lastseen( undef )->store;
+    $cache->clear_from_cache($cache_key);
+    $patron->update_lastseen( 'login' );
+    $patron->_result()->discard_changes();
+    is( $patron->lastseen, undef, 'Patron should still have last seen unchanged when TrackLastPatronActivity = 0' );
+
+    Time::Fake->reset;
+    $schema->storage->txn_rollback;
+};
+
+
diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
index 89ce426187..456fb892c3 100755
--- a/t/db_dependent/Koha/Patrons.t
+++ b/t/db_dependent/Koha/Patrons.t
@@ -1605,12 +1605,12 @@ subtest 'BorrowersLog tests' => sub {
     is( $log_info->{cardnumber}->{before}, $cardnumber, 'Got correct old cardnumber' );
     is( scalar @logs, 1, 'With BorrowerLogs, one detailed MODIFY action should be logged for the modification.' );
 
+    t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' );
     t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', 1 );
-    $patron->track_login();
+    $patron->update_lastseen('connection');
     @logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } );
     is( scalar @logs, 1, 'With BorrowerLogs and TrackLastPatronActivity we should not spam the logs');
 };
-
 $schema->storage->txn_rollback;
 
 subtest 'Test Koha::Patrons::merge' => sub {
diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index 7d9852af57..a0d228cbe8 100755
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -338,10 +338,12 @@ my $patron2 = $builder->build({
         flags => undef,
     }
 });
+t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' );
 t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
-Koha::Patrons->find( $patron2->{borrowernumber} )->track_login;
+Koha::Patrons->find( $patron2->{borrowernumber} )->update_lastseen('connection');
 is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should not be changed' );
-Koha::Patrons->find( $patron2->{borrowernumber} )->track_login({ force => 1 });
+t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
+Koha::Patrons->find( $patron2->{borrowernumber} )->update_lastseen('connection');
 isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should be changed now' );
 
 # Test GetBorrowersToExpunge and regular patron with permission
-- 
2.41.0