From fec140c569608866164dd88da7caa40ac224bc08 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 --- C4/Auth.pm | 34 +++------------------------------- C4/Circulation.pm | 6 +++--- C4/ILSDI/Services.pm | 4 ++-- C4/SIP/Sip/MsgType.pm | 6 +++++- Koha/Patron.pm | 36 ++++++++++++++++++++++++------------ 5 files changed, 37 insertions(+), 49 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 041779854f..2c0dca8451 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -73,7 +73,7 @@ BEGIN { @EXPORT_OK = qw( checkauth check_api_auth get_session check_cookie_auth checkpw checkpw_internal checkpw_hash - get_all_subpermissions get_user_subpermissions track_login_daily in_iprange + get_all_subpermissions get_user_subpermissions in_iprange get_template_and_user haspermission create_basic_session ); @@ -1306,7 +1306,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. @@ -2297,35 +2298,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 114ec32e7c..65bf5f21fb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1676,7 +1676,7 @@ sub AddIssue { )->store; } $issue->discard_changes; - C4::Auth::track_login_daily( $borrower->{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. @@ -2471,7 +2471,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 @@ -3311,7 +3311,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 36cad1269f..a15e694be2 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 6645fcf7ef..0539d287d9 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); @@ -993,7 +994,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 e20eb6f966..3839ea18df 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -32,6 +32,7 @@ use Koha::Account; use Koha::ArticleRequests; use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages ); use Koha::AuthUtils; +use Koha::Caches; use Koha::Checkouts; use Koha::CirculationRules; use Koha::Club::Enrollments; @@ -998,24 +999,35 @@ sub has_overdues { return $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime( dt_from_string() ) } })->count; } -=head3 track_login +=head3 update_lastseen - $patron->track_login; - $patron->track_login({ force => 1 }); + $patron->update_lastseen('activity'); - 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'); +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->make_column_dirty('updated_on'); - $self->lastseen( dt_from_string() )->store; + $self->lastseen($now)->store; + $cache->set_in_cache( $cache_key, $now->ymd ); + return $self; } =head3 move_to_deleted -- 2.41.0