Attachment 156903 Details for Bug 15504 – Bug 15504: (follow-up) Unit tests

[patch] Bug 15504: (follow-up) Unit tests

Bug-15504-follow-up-Unit-tests.patch (text/plain), 15.57 KB, created by Martin Renvoize (ashimema) on 2023-10-12 06:18:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-10-12 06:18:30 UTC

Size: 15.57 KB

patch

obsolete

>From 97d4216aa4fb5a468c461764e974c8a1f6a36fe9 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>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> t/db_dependent/Auth.t         | 104 +------------------------------
> t/db_dependent/Koha/Patron.t  | 113 +++++++++++++++++++++++++++++++++-
> t/db_dependent/Koha/Patrons.t |   4 +-
> t/db_dependent/Members.t      |   6 +-
> 4 files changed, 120 insertions(+), 107 deletions(-)
>
>diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t
>index 055dfd26869..46b500c013f 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 )
>     );
> }
> 
>@@ -415,106 +415,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 44c1726a5da..0bc047371b4 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 => 28;
>+use Test::More tests => 29;
> use Test::Exception;
> use Test::Warn;
>+use Time::Fake;
> 
> use Koha::CirculationRules;
> use Koha::Database;
>@@ -1950,3 +1951,113 @@ subtest 'alert_subscriptions tests' => sub {
> 
>     $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;
>+};
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index 24d4f7a811f..23a9a2730c3 100755
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -1618,12 +1618,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 7dceafd1bba..9c65ccb8f80 100755
>--- a/t/db_dependent/Members.t
>+++ b/t/db_dependent/Members.t
>@@ -319,10 +319,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

Actions: View | Diff | Splinter Review

Attachments on bug 15504: 152332 | 152333 | 152334 | 152335 | 152336 | 152500 | 152501 | 152502 | 152503 | 152504 | 152505 | 152506 | 155290 | 155291 | 155292 | 155293 | 155294 | 155295 | 155296 | 155534 | 155535 | 155536 | 155537 | 155538 | 155539 | 155540 | 156598 | 156599 | 156600 | 156601 | 156602 | 156603 | 156604 | 156605 | 156606 | 156607 | 156608 | 156742 | 156855 | 156856 | 156857 | 156858 | 156859 | 156860 | 156861 | 156862 | 156863 | 156864 | 156865 | 156866 | 156867 | 156868 | 156869 | 156870 | 156875 | 156879 | 156898 | 156899 | 156900 | 156901 | 156902 | 156903 | 156904 | 156905 | 156906 | 156907 | 156908 | 156909 | 156910 | 156911 | 156912 | 156913 | 156914 | 156915 | 156917 | 156918 | 156919 | 156920 | 156921 | 156922 | 156923 | 156924 | 156925 | 156926 | 156927 | 156928 | 156929 | 156930 | 156931 | 156932 | 156933 | 156934 | 156935 | 157602 | 157603 | 157604 | 157605 | 157606 | 157607 | 157608 | 157609 | 157610 | 157611 | 157612 | 157613 | 157614 | 157615 | 157616 | 157617 | 157618 | 157619 | 157620 | 157624