@@ -, +, @@ --- C4/SIP/Sip/MsgType.pm | 2 +- Koha/Patron.pm | 10 +++++----- ...bug_15504-add-tracklastpatronactivityoptions.pl | 6 ++++++ installer/data/mysql/mandatory/sysprefs.sql | 1 - .../prog/en/modules/admin/preferences/patrons.pref | 8 +------- .../prog/en/modules/tools/cleanborrowers.tt | 6 +++--- misc/cronjobs/delete_patrons.pl | 7 +++---- t/db_dependent/ILSDI_Services.t | 5 ++--- t/db_dependent/Koha/Patron.t | 11 +++++------ t/db_dependent/Koha/Patrons.t | 3 +-- t/db_dependent/Members.t | 14 +++++--------- t/db_dependent/SIP/Message.t | 5 ++--- 12 files changed, 34 insertions(+), 44 deletions(-) --- a/C4/SIP/Sip/MsgType.pm +++ a/C4/SIP/Sip/MsgType.pm @@ -996,7 +996,7 @@ sub handle_patron_info { $resp = (PATRON_INFO_RESP); if ($patron) { - if ( C4::Context->preference('TrackLastPatronActivity') ) { + if ( C4::Context->preference('TrackLastPatronActivityTriggers') ) { my $koha_patron = Koha::Patrons->find($patron->{borrowernumber}); $koha_patron->update_lastseen('connection'); } --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -843,7 +843,7 @@ sub is_active { return 1 if DateTime->compare( dt_from_string( $self->dateenrolled ), $dt ) > -1; # Last seen? Updated each login when you track patron activity - if ( C4::Context->preference('TrackLastPatronActivity') ) { + if ( C4::Context->preference('TrackLastPatronActivityTriggers') ) { return 1 if DateTime->compare( dt_from_string( $self->lastseen ), $dt ) > -1; } @@ -1170,15 +1170,15 @@ sub _get_overdue_debarred_delay { $patron->update_lastseen('activity'); -Tracks a (successful) login attempt when the TrackLastPatronActivity preference is enabled -and the activity passed is in the TrackLastPatronActivityTriggers list. +Updates the lastseen field, limited to one update per day, whenever the activity passed is +listed in TrackLastPatronActivityTriggers. + +The method should be called upon successful completion of the activity. =cut 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') --- a/installer/data/mysql/atomicupdate/bug_15504-add-tracklastpatronactivityoptions.pl +++ a/installer/data/mysql/atomicupdate/bug_15504-add-tracklastpatronactivityoptions.pl @@ -18,5 +18,11 @@ return { ); say $out "Added system preference 'TrackLastPatronActivityTriggers'"; + + $dbh->do(q{ + DELETE FROM systempreferences WHERE variable='TrackLastPatronActivity' + }); + + say $out "Removed system preference 'TrackLastPatronActivity'"; }, }; --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/installer/data/mysql/mandatory/sysprefs.sql @@ -747,7 +747,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('TraceCompleteSubfields','0','0','Force subject tracings to only match complete subfields.','YesNo'), ('TraceSubjectSubdivisions','0','1','Create searches on all subdivisions for subject tracings.','YesNo'), ('TrackClicks','0',NULL,'Track links clicked','Integer'), -('TrackLastPatronActivity','0',NULL,'If set, the field borrowers.lastseen will be updated everytime a patron is seen','YesNo'), ('TrackLastPatronActivityTriggers','',NULL,'If set, the field borrowers.lastseen will be updated everytime a patron is does a selected option','multiple'), ('TransfersBlockCirc','1',NULL,'Should the transfer modal block circulation staff from continuing scanning items','YesNo'), ('TransfersMaxDaysWarning','3',NULL,'Define the days before a transfer is suspected of having a problem','Integer'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -93,13 +93,7 @@ Patrons: 0: "Don't" - charge a fee when a patron changes to a category with an enrollment fee. - - - pref: TrackLastPatronActivity - choices: - 1: Do - 0: "Don't" - - track last patron activity. Use TrackLastPatronActivityTriggers to select which activities should be tracked. - - - - Select which activities should be tracked. The first time each day that one of these activities occurs, borrowers.lastseen will update with the current date and time. + - Select which patron activities should be tracked to signify patron activity. The first time each day that one of these activities occurs, borrowers.lastseen will update with the current date and time. - pref: TrackLastPatronActivityTriggers multiple: login: "Login" --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt @@ -116,7 +116,7 @@ [% INCLUDE 'date-format.inc' %] - [% IF Koha.Preference('TrackLastPatronActivity') %] + [% IF Koha.Preference('TrackLastPatronActivityTriggers') %]
  • @@ -222,7 +222,7 @@ - [% IF Koha.Preference('TrackLastPatronActivity') %] + [% IF Koha.Preference('TrackLastPatronActivityTriggers') %] [% END %] @@ -299,7 +299,7 @@ */ function checkForm(form) { if((form.checkbox[0].checked)){ - if ( (!form.date1.value) && (!form.borrower_dateexpiry.value) [% IF Koha.Preference('TrackLastPatronActivity') %]&& (!form.borrower_lastseen.value) [% END %]&& (!form.borrower_categorycode.value) && (!form.patron_list_id.value)){ + if ( (!form.date1.value) && (!form.borrower_dateexpiry.value) [% IF Koha.Preference('TrackLastPatronActivityTriggers') %]&& (!form.borrower_lastseen.value) [% END %]&& (!form.borrower_categorycode.value) && (!form.patron_list_id.value)){ alert(_("Please enter at least one criterion for deletion!")); return false; } --- a/misc/cronjobs/delete_patrons.pl +++ a/misc/cronjobs/delete_patrons.pl @@ -38,8 +38,8 @@ $not_borrowed_since = dt_from_string( $not_borrowed_since, 'iso' ) $expired_before = dt_from_string( $expired_before, 'iso' ) if $expired_before; -if ( $last_seen and not C4::Context->preference('TrackLastPatronActivity') ) { - pod2usage(q{The --last_seen will be ineffective if TrackLastPatronActivity turned off or there are not TrackLastPatronActivityTriggers defined}); +if ( $last_seen and not C4::Context->preference('TrackLastPatronActivityTriggers') ) { + pod2usage(q{The --last_seen will be ineffective if there are not TrackLastPatronActivityTriggers enabled}); } unless ( $not_borrowed_since or $expired_before or $last_seen or @category_code or $branchcode or $file ) { @@ -185,8 +185,7 @@ Delete patrons with an account expired before this date. Delete patrons who have not been active since this date. -The system preference TrackLastPatronActivity must be enabled and at least one TrackLastPatronActivityTriggers -must be set to use this option. +The system preference TrackLastPatronActivityTriggers must be contain least trigger to use this option. =item B<--category_code> --- a/t/db_dependent/ILSDI_Services.t +++ a/t/db_dependent/ILSDI_Services.t @@ -47,7 +47,6 @@ subtest 'AuthenticatePatron test' => sub { $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); my $plain_password = 'tomasito'; @@ -71,7 +70,7 @@ subtest 'AuthenticatePatron test' => sub { $query->param( 'username', $borrower->{userid}); $query->param( 'password', $plain_password); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '' ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); my $reply = C4::ILSDI::Services::AuthenticatePatron( $query ); is( $reply->{id}, $borrower->{borrowernumber}, "userid and password - Patron authenticated" ); is( $reply->{code}, undef, "Error code undef"); @@ -89,7 +88,7 @@ subtest 'AuthenticatePatron test' => sub { is( $reply->{code}, 'PatronNotFound', "non-existing userid - PatronNotFound" ); is( $reply->{id}, undef, "id undef"); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); $query->param( 'username', uc( $borrower->{userid} )); $reply = C4::ILSDI::Services::AuthenticatePatron( $query ); my $now = dt_from_string; --- a/t/db_dependent/Koha/Patron.t +++ a/t/db_dependent/Koha/Patron.t @@ -338,14 +338,14 @@ subtest 'is_active' => sub { $patron->dateenrolled('2020-01-01')->store; # Check lastseen, test days parameter - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', 1 ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login' ); $patron->update_lastseen('login'); is( $patron->is_active( { days => 1 } ), 1, 'Just logged in' ); my $ago = dt_from_string->subtract( days => 2 ); $patron->lastseen($ago)->store; is( $patron->is_active( { days => 1 } ), 0, 'Not active since yesterday' ); is( $patron->is_active( { days => 3 } ), 1, 'Active within last 3 days' ); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', 0 ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); is( $patron->is_active( { days => 3 } ), 0, 'Pref disabled' ); # Look at holds, test with weeks @@ -1967,7 +1967,6 @@ subtest 'update_lastseen tests' => sub { my $cache_key = "track_activity_" . $patron->borrowernumber; $cache->clear_from_cache($cache_key); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' ); t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' ); is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' ); @@ -1977,7 +1976,7 @@ subtest 'update_lastseen tests' => sub { $patron->update_lastseen('login'); $patron->_result()->discard_changes(); - isnt( $patron->lastseen, undef, 'Patron should have last seen set when TrackLastPatronActivity = 1' ); + isnt( $patron->lastseen, undef, 'Patron should have last seen set when TrackLastPatronActivityTriggers contains values' ); my $last_seen = $patron->lastseen; Time::Fake->offset( $now->epoch + 5 ); @@ -2076,12 +2075,12 @@ subtest 'update_lastseen tests' => sub { isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed after a renewal if we cleared the cache' ); # Check that the preference takes effect - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); $patron->lastseen(undef)->store; $cache->clear_from_cache($cache_key); $patron->update_lastseen('login'); $patron->_result()->discard_changes(); - is( $patron->lastseen, undef, 'Patron should still have last seen unchanged when TrackLastPatronActivity = 0' ); + is( $patron->lastseen, undef, 'Patron should still have last seen unchanged when TrackLastPatronActivityTriggers is unset' ); Time::Fake->reset; $schema->storage->txn_rollback; --- a/t/db_dependent/Koha/Patrons.t +++ a/t/db_dependent/Koha/Patrons.t @@ -1619,10 +1619,9 @@ subtest 'BorrowersLog tests' => sub { is( scalar @logs, 1, 'With BorrowerLogs, one detailed MODIFY action should be logged for the modification.' ); t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', 1 ); $patron->update_lastseen('connection'); @logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); - is( scalar @logs, 1, 'With BorrowerLogs and TrackLastPatronActivity we should not spam the logs'); + is( scalar @logs, 1, 'With BorrowerLogs and TrackLastPatronActivityTriggers we should not spam the logs'); }; $schema->storage->txn_rollback; --- a/t/db_dependent/Members.t +++ a/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 50; +use Test::More tests => 49; use Test::MockModule; use Test::Exception; @@ -280,7 +280,7 @@ is( scalar(@$patstodel),2,'Borrowers without issues deleted by expiration_date a $patstodel = GetBorrowersToExpunge( {not_borrowed_since => '2016-01-02', patron_list_id => $list1->patron_list_id() } ); is( scalar(@$patstodel),2,'Borrowers without issues deleted by last issue date'); -# Test GetBorrowersToExpunge and TrackLastPatronActivity +# Test GetBorrowersToExpunge and TrackLastPatronActivityTriggers $dbh->do(q|UPDATE borrowers SET lastseen=NULL|); $builder->build({ source => 'Borrower', @@ -307,11 +307,11 @@ $builder->build({ } }); $patstodel = GetBorrowersToExpunge( { last_seen => '1999-12-12' }); -is( scalar @$patstodel, 0, 'TrackLastPatronActivity - 0 patrons must be deleted' ); +is( scalar @$patstodel, 0, 'TrackLastPatronActivityTriggers - 0 patrons must be deleted' ); $patstodel = GetBorrowersToExpunge( { last_seen => '2016-02-15' }); -is( scalar @$patstodel, 2, 'TrackLastPatronActivity - 2 patrons must be deleted' ); +is( scalar @$patstodel, 2, 'TrackLastPatronActivityTriggers - 2 patrons must be deleted' ); $patstodel = GetBorrowersToExpunge( { last_seen => '2016-04-04' }); -is( scalar @$patstodel, 3, 'TrackLastPatronActivity - 3 patrons must be deleted' ); +is( scalar @$patstodel, 3, 'TrackLastPatronActivityTriggers - 3 patrons must be deleted' ); my $patron2 = $builder->build({ source => 'Borrower', value => { @@ -320,10 +320,6 @@ my $patron2 = $builder->build({ } }); t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); -t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' ); -Koha::Patrons->find( $patron2->{borrowernumber} )->update_lastseen('connection'); -is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should not be changed' ); -t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' ); Koha::Patrons->find( $patron2->{borrowernumber} )->update_lastseen('connection'); isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should be changed now' ); --- a/t/db_dependent/SIP/Message.t +++ a/t/db_dependent/SIP/Message.t @@ -194,7 +194,6 @@ subtest 'Lastseen response' => sub { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); my $builder = t::lib::TestBuilder->new(); my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; @@ -220,7 +219,7 @@ subtest 'Lastseen response' => sub { my $server = { ils => $mocks->{ils} }; undef $response; - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '' ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); $msg->handle_patron_info( $server ); isnt( $response, undef, 'At least we got a response.' ); @@ -229,7 +228,7 @@ subtest 'Lastseen response' => sub { $seen_patron = Koha::Patrons->find({ cardnumber => $seen_patron->{cardnumber} }); is( output_pref({str => $seen_patron->lastseen(), dateonly => 1}), output_pref({str => '2001-01-01', dateonly => 1}),'Last seen not updated if not tracking patrons'); undef $response; - t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'connection' ); $msg->handle_patron_info( $server ); isnt( $response, undef, 'At least we got a response.' ); --