Bug 16276: [QA Follow-up] Only track when pref is enabled

Do not track when the pref has not been enabled.
This patch moves the conditional update in Auth.pm to Koha::Patron.
And adds a test for the new track_login method.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 7171c2b..6b7dc66 100644 (file)
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -35,6 +35,7 @@ use Koha::Caches;
 use Koha::AuthUtils qw(get_script_name hash_password);
 use Koha::Libraries;
 use Koha::LibraryCategories;
+use Koha::Patrons;
 use POSIX qw/strftime/;
 use List::MoreUtils qw/ any /;
 use Encode qw( encode is_utf8);
@@ -1183,11 +1184,9 @@ sub checkauth {
         }
 
         if ( $userid ) {
-            $dbh->do(q|
-                UPDATE borrowers
-                SET lastseen = NOW()
-                WHERE userid = ?
-            |, undef, $userid);
+            # track_login also depends on pref TrackLastPatronActivity
+            my $patron = Koha::Patrons->search({ userid => $userid })->next;
+            $patron->track_login if $patron;
         }
 
         return ( $userid, $cookie, $sessionID, $flags );

diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index d9e2585..b5449a3 100644 (file)
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -248,6 +248,25 @@ sub has_overdues {
     return $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime( dt_from_string() ) } })->count;
 }
 
+=head2 track_login
+
+    $patron->track_login;
+    $patron->track_login({ force => 1 });
+
+    Tracks a (successful) login attempt.
+    The preference TrackLastPatronActivity must be enabled. Or you
+    should pass the force parameter.
+
+=cut
+
+sub track_login {
+    my ( $self, $params ) = @_;
+    return if
+        !$params->{force} &&
+        !C4::Context->preference('TrackLastPatronActivity');
+    $self->lastseen( dt_from_string() )->store;
+}
+
 =head3 type
 
 =cut

diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index f5b6b4d..8ab9069 100755 (executable)
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -17,14 +17,14 @@
 
 use Modern::Perl;
 
-use Test::More tests => 82;
+use Test::More tests => 84;
 use Test::MockModule;
 use Data::Dumper;
 use C4::Context;
 use Koha::Database;
 use Koha::Holds;
 use Koha::List::Patron;
-
+use Koha::Patrons;
 
 use t::lib::Mocks;
 use t::lib::TestBuilder;
@@ -387,6 +387,12 @@ $patstodel = GetBorrowersToExpunge( { last_seen => '2016-02-15' });
 is( scalar @$patstodel, 2, 'TrackLastPatronActivity - 2 patrons must be deleted' );
 $patstodel = GetBorrowersToExpunge( { last_seen => '2016-04-04' });
 is( scalar @$patstodel, 3, 'TrackLastPatronActivity - 3 patrons must be deleted' );
+my $patron2 = $builder->build({ source => 'Borrower', value => { lastseen => undef } });
+t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login;
+is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should not be changed' );
+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login({ force => 1 });
+isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should be changed now' );
 
 # Regression tests for BZ13502
 ## Remove all entries with userid='' (should be only 1 max)