From 7a444666a38dcc15b1e7ba52a46c0028c064403d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 20 Jun 2023 19:23:11 +0100 Subject: [PATCH] Bug 15504: (follow-up) Migrate to one clear method We were using a series of similarly named methods spread in distinct places around the codebase. This combines the logic of C4::Auth::track_login_daily and Koha::Patron->track_login into a new Koha::Patron->update_lastseen method. Signed-off-by: Martin Renvoize Signed-off-by: David Nind --- C4/Auth.pm | 34 +++------------------------------- C4/Circulation.pm | 6 +++--- C4/ILSDI/Services.pm | 4 ++-- C4/SIP/Sip/MsgType.pm | 6 +++++- Koha/Patron.pm | 37 ++++++++++++++++++++++++------------- 5 files changed, 37 insertions(+), 50 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 9f19ebabc18..4e5c567b985 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -74,7 +74,7 @@ BEGIN { @EXPORT_OK = qw( checkauth check_api_auth get_session check_cookie_auth checkpw checkpw_internal checkpw_hash - get_all_subpermissions get_cataloguing_page_permissions get_user_subpermissions track_login_daily in_iprange + get_all_subpermissions get_cataloguing_page_permissions get_user_subpermissions in_iprange get_template_and_user haspermission create_basic_session ); @@ -1319,7 +1319,8 @@ sub checkauth { )); } - track_login_daily( $userid, 'login' ); + my $patron = Koha::Patrons->find({ userid => $userid }); + $patron->update_lastseen('login'); # In case, that this request was a login attempt, we want to prevent that users can repost the opac login # request. We therefore redirect the user to the requested page again without the login parameters. @@ -2345,35 +2346,6 @@ sub getborrowernumber { return 0; } -=head2 track_login_daily - - track_login_daily( $userid ); - -Wraps the call to $patron->track_login, the method used to update borrowers.lastseen. We only call track_login once a day. - -=cut - -sub track_login_daily { - my $userid = shift; - my $activity = shift; - return if !$userid || !$activity || !C4::Context->preference('TrackLastPatronActivity'); - - my $cache = Koha::Caches->get_instance(); - my $cache_key = "track_login_" . $userid; - my $cached = $cache->get_from_cache($cache_key); - my $today = dt_from_string()->ymd; - return if $cached && $cached eq $today; - - my $patron = Koha::Patrons->find({ userid => $userid }); - return unless $patron; - - my $tracked_activities = { map { (lc $_, 1); } split /\s*\,\s*/, C4::Context->preference('TrackLastPatronActivityTriggers') }; - return unless $tracked_activities->{$activity}; - - $patron->track_login; - $cache->set_in_cache( $cache_key, $today ); -} - END { } # module clean-up code here (global destructor) 1; __END__ diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2842021a4f0..2c4a1f36d63 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1677,7 +1677,7 @@ sub AddIssue { )->store; } $issue->discard_changes; - C4::Auth::track_login_daily( $patron->userid, 'check_out' ); + $patron->update_lastseen('check_out'); if ( $item_object->location && $item_object->location eq 'CART' && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) { ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart. @@ -2496,7 +2496,7 @@ sub AddReturn { if C4::Context->preference("ReturnLog"); #Update borrowers.lastseen - C4::Auth::track_login_daily( $patron->userid, 'check_in' ); + $patron->update_lastseen('check_in'); } # Check if this item belongs to a biblio record that is attached to an @@ -3347,7 +3347,7 @@ sub AddRenewal { } ); #Update borrowers.lastseen - C4::Auth::track_login_daily( $patron_unblessed->{userid}, 'renewal' ); + $patron->update_lastseen('renewal'); #Log the renewal logaction("CIRCULATION", "RENEWAL", $borrowernumber, $itemnumber) if C4::Context->preference("RenewalLog"); diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 6cb6ff5d896..62d4242f5dc 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -396,10 +396,10 @@ sub AuthenticatePatron { my $password = $cgi->param('password'); my ($status, $cardnumber, $userid) = C4::Auth::checkpw( $username, $password ); if ( $status == 1 ) { - # Track the login - C4::Auth::track_login_daily( $userid, 'connection' ); # Get the borrower my $patron = Koha::Patrons->find( { userid => $userid } ); + # Track the login + $patron->update_lastseen('connection'); return { id => $patron->borrowernumber }; } elsif ( $status == -2 ){ diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index b166551e450..30c35949a71 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -16,6 +16,7 @@ use C4::SIP::Sip::Checksum qw(verify_cksum); use Data::Dumper; use CGI qw ( -utf8 ); +use C4::Context; use C4::Auth qw(&check_api_auth); use C4::Items qw(ModDateLastSeen); @@ -995,7 +996,10 @@ sub handle_patron_info { $resp = (PATRON_INFO_RESP); if ($patron) { - C4::Auth::track_login_daily( $patron->userid, 'connection' ); + if ( C4::Context->preference('TrackLastPatronActivity') ) { + my $koha_patron = Koha::Patrons->find($patron->userid); + $koha_patron->update_lastseen('connection'); + } $resp .= patron_status_string( $patron, $server ); $resp .= ( defined($lang) and length($lang) == 3 ) ? $lang : $patron->language; $resp .= timestamp(); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 73212772340..574a107a6cd 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -33,6 +33,7 @@ use C4::Log qw( logaction ); use Koha::Account; use Koha::ArticleRequests; use Koha::AuthUtils; +use Koha::Caches; use Koha::Checkouts; use Koha::CirculationRules; use Koha::Club::Enrollments; @@ -1165,24 +1166,34 @@ sub _get_overdue_debarred_delay { } } +=head3 update_lastseen -=head3 track_login + $patron->update_lastseen('activity'); - $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. +Tracks a (successful) login attempt when the TrackLastPatronActivity preference is enabled +and the activity passed is in the TrackLastPatronActivityTriggers list. =cut -sub track_login { - my ( $self, $params ) = @_; - return if - !$params->{force} && - !C4::Context->preference('TrackLastPatronActivity'); - $self->lastseen( dt_from_string() )->store; +sub update_lastseen { + my ( $self, $activity ) = @_; + return $self if !C4::Context->preference('TrackLastPatronActivity'); + + my $tracked_activities = { + map { ( lc $_, 1 ); } split /\s*\,\s*/, + C4::Context->preference('TrackLastPatronActivityTriggers') + }; + return $self unless $tracked_activities->{$activity}; + + my $cache = Koha::Caches->get_instance(); + my $cache_key = "track_login_" . $self->userid; + my $cached = $cache->get_from_cache($cache_key); + my $now = dt_from_string(); + return if $cached && $cached eq $now->ymd; + + $self->lastseen($now)->store; + $cache->set_in_cache( $cache_key, $now->ymd ); + return $self; } =head3 move_to_deleted -- 2.41.0