From c1e0e75883a25ce4ee5ca4d0022e7caa415e8f15 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 --- 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