Attachment 54618 Details for Bug 16276 – Bug 16276: [QA Follow-up] Only track when pref is enabled

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

Bug-16276-QA-Follow-up-Only-track-when-pref-is-ena.patch (text/plain), 3.54 KB, created by Marcel de Rooy on 2016-08-19 11:05:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2016-08-19 11:05:21 UTC

Size: 3.54 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 16276: 50615 | 50616 | 50617 | 50618 | 50619 | 50620 | 51191 | 51407 | 51408 | 51409 | 51410 | 51411 | 51412 | 54588 | 54589 | 54590 | 54591 | 54592 | 54612 | 54613 | 54614 | 54615 | 54616 | 54617 | 54618 | 54619 | 54620 | 54621 | 54622 | 54623 | 54624 | 54625 | 55495 | 55496 | 55497 | 55498 | 55499 | 55500 | 55501