From 06ac97bd1ef58b7a2a2602fcc326ab1fe71abefa Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Fri, 19 Aug 2016 12:23:01 +0200
Subject: [PATCH] Bug 16276: [QA Follow-up] Only track when pref is enabled
Content-Type: text/plain; charset=utf-8

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>
---
 C4/Auth.pm               |    9 ++++-----
 Koha/Patron.pm           |   19 +++++++++++++++++++
 t/db_dependent/Members.t |   10 ++++++++--
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 8431e2a..52acf24 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -35,6 +35,7 @@ use Koha;
 use Koha::AuthUtils qw(get_script_name hash_password);
 use Koha::LibraryCategories;
 use Koha::Libraries;
+use Koha::Patrons;
 use POSIX qw/strftime/;
 use List::MoreUtils qw/ any /;
 use Encode qw( encode is_utf8);
@@ -1184,11 +1185,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 42b066b..95cfafb 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -208,6 +208,25 @@ sub update_password {
     return 1;
 }
 
+=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
--- 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)
-- 
1.7.10.4