From 16417b64e793077ea0a93dd052045b1e0d470816 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Mon, 19 Jun 2017 09:49:00 -0400
Subject: [PATCH] Bug 18821 - TrackLastPatronActivity is a performance killer

Enabling TrackLastPatronActivity can cause an extreme performance hit, especially on the staff side during checkin and checkout. It makes more sense to only update the last activity timestamp on the first access of a given session. This will greatly improve performance while retaining the same basic information.

Test Plan:
1) Apply this patch
2) Start a new session ( a private browser window works well )
3) Note the lastseen column in the borrowers table is updated
4) Browse a few pages, not the lastseen column is not updated again
5) Close the browser window and repeat steps 2-4
6) prove t/db_dependent/Members.t
---
 C4/Auth.pm               |  6 ++++--
 Koha/Patron.pm           | 12 +++++++++---
 t/db_dependent/Members.t | 20 ++++++++++++++++++--
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index c3ad351aa1..1c5f8476d7 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -774,6 +774,8 @@ sub checkauth {
     my $casparam = $query->param('cas');
     my $q_userid = $query->param('userid') // '';
 
+    my $session;
+
     # Basic authentication is incompatible with the use of Shibboleth,
     # as Shibboleth may return REMOTE_USER as a Shibboleth attribute,
     # and it may not be the attribute we want to use to match the koha login.
@@ -799,7 +801,7 @@ sub checkauth {
     }
     elsif ( $sessionID = $query->cookie("CGISESSID") )
     {    # assignment, not comparison
-        my $session = get_session($sessionID);
+        $session = get_session($sessionID);
         C4::Context->_new_userenv($sessionID);
         my ( $ip, $lasttime, $sessiontype );
         my $s_userid = '';
@@ -1193,7 +1195,7 @@ sub checkauth {
         if ( $userid ) {
             # track_login also depends on pref TrackLastPatronActivity
             my $patron = Koha::Patrons->find({ userid => $userid });
-            $patron->track_login if $patron;
+            $patron->track_login( { session => $session } ) if $patron;
         }
 
         return ( $userid, $cookie, $sessionID, $flags );
diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index d0be998c88..c9353442ab 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -404,9 +404,15 @@ sub has_overdues {
 
 sub track_login {
     my ( $self, $params ) = @_;
-    return if
-        !$params->{force} &&
-        !C4::Context->preference('TrackLastPatronActivity');
+    my $session = $params->{session};
+
+    return unless C4::Context->preference('TrackLastPatronActivity') || $params->{force};
+
+    if ( $session ) {
+        return if $session->param('tracked_for_session') && !$params->{force};
+        $session->param('tracked_for_session', 1);
+    }
+
     $self->lastseen( dt_from_string() )->store;
 }
 
diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index c7bc6b30ee..aada28bb89 100755
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 60;
+use Test::More tests => 62;
 use Test::MockModule;
 use Test::Exception;
 
@@ -350,12 +350,28 @@ $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' );
+
+
+# Test method last_seen / TrackLastPatronActivity
 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' );
+my $last_seen = Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen;
+isnt( $last_seen, undef, 'Lastseen should be changed now' );
+
+# Last seen shouldn't be updated a second time for this session
+t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login();
+is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, $last_seen, 'Lastseen should not be changed' );
+
+# If it's forced, it should still be updated
+sleep(1); # We need to wait a tiny bit to make sure the timestamp will be different
+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login({ force => 1 });
+isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, $last_seen, 'Lastseen should be changed if forced' );
+
 
 # Regression tests for BZ13502
 ## Remove all entries with userid='' (should be only 1 max)
-- 
2.14.3 (Apple Git-98)