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>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Martin Renvoize 2023-06-21 08:21:59 +01:00 committed by Tomas Cohen Arazi
parent dff2fc82d7
commit b30d32a571
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
4 changed files with 120 additions and 107 deletions

View file

@ -7,7 +7,7 @@ use CGI qw ( -utf8 );
use Test::MockObject; use Test::MockObject;
use Test::MockModule; use Test::MockModule;
use List::MoreUtils qw/all any none/; use List::MoreUtils qw/all any none/;
use Test::More tests => 19; use Test::More tests => 18;
use Test::Warn; use Test::Warn;
use t::lib::Mocks; use t::lib::Mocks;
use t::lib::TestBuilder; use t::lib::TestBuilder;
@ -22,7 +22,7 @@ use Koha::Auth::TwoFactorAuth;
BEGIN { BEGIN {
use_ok( use_ok(
'C4::Auth', '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 )
); );
} }
@ -415,106 +415,6 @@ subtest 'checkauth() tests' => sub {
C4::Context->_new_userenv; # For next tests 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 { subtest 'no_set_userenv parameter tests' => sub {
plan tests => 7; plan tests => 7;

View file

@ -19,9 +19,10 @@
use Modern::Perl; use Modern::Perl;
use Test::More tests => 28; use Test::More tests => 29;
use Test::Exception; use Test::Exception;
use Test::Warn; use Test::Warn;
use Time::Fake;
use Koha::CirculationRules; use Koha::CirculationRules;
use Koha::Database; use Koha::Database;
@ -1950,3 +1951,113 @@ subtest 'alert_subscriptions tests' => sub {
$patron->discard_changes; $patron->discard_changes;
}; };
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;
};

View file

@ -1618,12 +1618,12 @@ subtest 'BorrowersLog tests' => sub {
is( $log_info->{cardnumber}->{before}, $cardnumber, 'Got correct old cardnumber' ); 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.' ); 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 ); 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 } ); @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'); is( scalar @logs, 1, 'With BorrowerLogs and TrackLastPatronActivity we should not spam the logs');
}; };
$schema->storage->txn_rollback; $schema->storage->txn_rollback;
subtest 'Test Koha::Patrons::merge' => sub { subtest 'Test Koha::Patrons::merge' => sub {

View file

@ -324,10 +324,12 @@ my $patron2 = $builder->build({
flags => undef, flags => undef,
} }
}); });
t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' );
t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' ); 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' ); 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' ); isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should be changed now' );
# Test GetBorrowersToExpunge and regular patron with permission # Test GetBorrowersToExpunge and regular patron with permission