From cfad8be84829ffbd07b6f74f37fcfab26412b567 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 12 Oct 2023 12:53:34 +0100 Subject: [PATCH] Bug 35030: Add 'article' to patron activity triggers This patch adds 'article' to the list of triggers available for tracking patron activity. Test plan 1) Select 'Placing an article request TrackPatronLastActivityTriggers system preference 2) As a staff member, place a hold on any item for a test user 3) Confirm that the borrowers.lastseen field is updated for that test borrower --- Koha/ArticleRequest.pm | 14 +++++++++ .../en/modules/admin/preferences/patrons.pref | 1 + t/db_dependent/Koha/ArticleRequest.t | 10 +++++-- t/db_dependent/Koha/Patron.t | 29 +++++++++++++++++-- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm index 42d2d011f1d..1bfbda34847 100644 --- a/Koha/ArticleRequest.pm +++ b/Koha/ArticleRequest.pm @@ -64,6 +64,7 @@ sub request { if $debit; $self->store(); + $self->patron->update_lastseen('article'); $self->notify(); return $self; } @@ -226,6 +227,19 @@ sub borrower { return Koha::Patron->_new_from_dbic($rs); } +=head3 patron + +Returns the Koha::Patron object for this article request + +=cut + +sub patron { + my ($self) = @_; + my $rs = $self->_result->borrowernumber; + return unless $rs; + return Koha::Patron->_new_from_dbic($rs); +} + =head3 branch Returns the Koha::Library object for this article request diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 9e80c862652..32d37aa5451 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -101,6 +101,7 @@ Patrons: check_out: "Checking out an item" renewal: "Renewing an item" check_in: "Returning an item" + article: "Places an article request" - - pref: AutoApprovePatronProfileSettings choices: diff --git a/t/db_dependent/Koha/ArticleRequest.t b/t/db_dependent/Koha/ArticleRequest.t index 9a5501bff29..2d6819e763e 100755 --- a/t/db_dependent/Koha/ArticleRequest.t +++ b/t/db_dependent/Koha/ArticleRequest.t @@ -30,7 +30,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'request() tests' => sub { - plan tests => 11; + plan tests => 13; $schema->storage->txn_begin; @@ -39,7 +39,7 @@ subtest 'request() tests' => sub { my $patron_mock = Test::MockModule->new('Koha::Patron'); $patron_mock->mock( 'article_request_fee', sub { return $amount; } ); - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { lastseen => undef } } ); my $item = $builder->build_sample_item; my $ar_mock = Test::MockModule->new('Koha::ArticleRequest'); @@ -52,6 +52,7 @@ subtest 'request() tests' => sub { } ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); $ar->request()->discard_changes; is( $ar->status, Koha::ArticleRequest::Status::Requested ); @@ -59,6 +60,8 @@ subtest 'request() tests' => sub { is( $ar->debit_id, undef, 'No fee linked' ); is( $patron->account->balance, 0, 'No outstanding fees' ); + $patron->discard_changes; + is( $patron->lastseen, undef, 'Patron activity not tracked when article is not a valid trigger' ); # set a fee amount $amount = 10; @@ -71,6 +74,7 @@ subtest 'request() tests' => sub { } ); + t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'article' ); $ar->request()->discard_changes; is( $ar->status, Koha::ArticleRequest::Status::Requested ); @@ -79,6 +83,8 @@ subtest 'request() tests' => sub { ok( defined $ar->debit_id, 'Fee linked' ); is( $patron->account->balance, $amount, 'Outstanding fees with the right value' ); + $patron->discard_changes; + isnt( $patron->lastseen, undef, 'Patron activity tracked when article is a valid trigger' ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 02a06df75f9..281327fb14d 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -1954,7 +1954,7 @@ subtest 'alert_subscriptions tests' => sub { subtest 'update_lastseen tests' => sub { - plan tests => 18; + plan tests => 21; $schema->storage->txn_begin; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); @@ -1967,7 +1967,10 @@ subtest 'update_lastseen tests' => sub { my $cache_key = "track_activity_" . $patron->borrowernumber; $cache->clear_from_cache($cache_key); - t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' ); + t::lib::Mocks::mock_preference( + 'TrackLastPatronActivityTriggers', + 'login,connection,check_in,check_out,renewal,article' + ); is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' ); @@ -1997,6 +2000,9 @@ subtest 'update_lastseen tests' => sub { $patron->update_lastseen('renewal'); $patron->_result()->discard_changes(); is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a renewal' ); + $patron->update_lastseen('article'); + $patron->_result()->discard_changes(); + is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged after a article' ); # Check that tracking is disabled when the activity isn't listed t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', '' ); @@ -2036,9 +2042,18 @@ subtest 'update_lastseen tests' => sub { $patron->lastseen, $last_seen, 'Patron last seen should be unchanged after a renewal if renewal is not selected as an option and the cache has been cleared' ); + $patron->update_lastseen('article'); + $patron->_result()->discard_changes(); + is( + $patron->lastseen, $last_seen, + 'Patron last seen should be unchanged after an article request if article is not selected as an option and the cache has been cleared' + ); # Check tracking for each activity - t::lib::Mocks::mock_preference( 'TrackLastPatronActivityTriggers', 'login,connection,check_in,check_out,renewal' ); + t::lib::Mocks::mock_preference( + 'TrackLastPatronActivityTriggers', + 'login,connection,check_in,check_out,renewal,article' + ); $cache->clear_from_cache($cache_key); $patron->update_lastseen('login'); @@ -2069,6 +2084,14 @@ subtest 'update_lastseen tests' => sub { 'Patron last seen should be changed after a check_in if we cleared the cache' ); + $cache->clear_from_cache($cache_key); + $patron->update_lastseen('article'); + $patron->_result()->discard_changes(); + isnt( + $patron->lastseen, $last_seen, + 'Patron last seen should be changed after a article if we cleared the cache' + ); + $cache->clear_from_cache($cache_key); $patron->update_lastseen('renewal'); $patron->_result()->discard_changes(); -- 2.41.0