From 539521544bd0dfc5d89ab4aacf4cea3b6d8f2dc7 Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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 --- Koha/Patron.pm | 8 +++++--- t/db_dependent/Members.t | 20 ++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index f7970ea..d5af875 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -401,9 +401,11 @@ sub has_overdues { sub track_login { my ( $self, $params ) = @_; - return if - !$params->{force} && - !C4::Context->preference('TrackLastPatronActivity'); + + return unless C4::Context->preference('TrackLastPatronActivity') || $params->{force}; + return if C4::Context->userenv->{'tracked_for_session'} && !$params->{force}; + + C4::Context->userenv->{'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 f75652c..7e74223 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 63; +use Test::More tests => 65; use Test::MockModule; use Data::Dumper qw/Dumper/; use C4::Context; @@ -357,12 +357,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.10.2