Bug 15504 - Track Patron's Last Activity
Summary: Track Patron's Last Activity
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P1 - high enhancement with 30 votes (vote)
Assignee: Matt Blenkinsop
QA Contact: Marcel de Rooy
URL: http://devs.bywatersolutions.com/proj...
Keywords: rel_23_11_candidate
: 14513 21007 27243 (view as bug list)
Depends on: 18625 28455 28630 33245
Blocks: 35027 35030 35221 35231
  Show dependency treegraph
 
Reported: 2016-01-06 16:48 UTC by Nicole C. Engard
Modified: 2023-11-02 13:31 UTC (History)
30 users (show)

See Also:
Change sponsored?: Seeking cosponsors
Patch complexity: Medium patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/745
Text to go in the release notes:
This enhancement changes how tracking patron activities (and updating the borrowers.lastseen field) works - you can now select what patron activities to track. Previously, with TrackLastPatronActivity enabled, the borrowers.lastseen field was updated when one of the "hard-coded" activities occurred - you could not select what patron activities to track With this enhancement, you can now individually select the patron activities to track. The current trackable activities are: - All activities - Checking in an item - Checking out an item - Connecting to Koha using SIP and ILS-DI - Logging in (for both the OPAC and the staff interface) - Placing a hold on an item (added by bug 35027) - Placing an article request (added by bug 35030) - Renewing an item Notes: - If no activities are selected, then patron activity is not tracked and the borrowers.lastseen field is not updated. - The system preference TrackLastPatronActivity was renamed to TrackLastPatronActivityTriggers.
Version(s) released in:
23.11.00


Attachments
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.13 KB, patch)
2023-06-14 14:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.39 KB, patch)
2023-06-14 14:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.91 KB, patch)
2023-06-14 14:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.69 KB, patch)
2023-06-14 14:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.83 KB, patch)
2023-06-14 14:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.20 KB, patch)
2023-06-21 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.43 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.63 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.74 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.88 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.84 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.71 KB, patch)
2023-06-21 09:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.20 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.43 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.63 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.74 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.88 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.65 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.30 KB, patch)
2023-09-07 11:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.25 KB, patch)
2023-09-12 05:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.48 KB, patch)
2023-09-12 05:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.68 KB, patch)
2023-09-12 05:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.79 KB, patch)
2023-09-12 05:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.92 KB, patch)
2023-09-12 05:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.69 KB, patch)
2023-09-12 05:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.35 KB, patch)
2023-09-12 05:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.26 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.48 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.68 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.79 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.92 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.53 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.22 KB, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find (937 bytes, patch)
2023-10-06 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Add Triggers mock to unit tests (1.46 KB, patch)
2023-10-06 10:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test (8.44 KB, patch)
2023-10-06 10:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Fix is_active test (1.16 KB, patch)
2023-10-06 10:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Add caching to update_lastseen (1.50 KB, patch)
2023-10-09 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.26 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.48 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.68 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.79 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.92 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.57 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.66 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find (937 bytes, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Add Triggers mock to unit tests (1.46 KB, patch)
2023-10-11 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test (8.44 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Fix is_active test (1.01 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity (2.02 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Rename cache key (1.62 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Terminology (1.25 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Update terminology on cleanborrowers (1.39 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons (1.73 KB, patch)
2023-10-11 17:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove PatronLastActivity preference (18.72 KB, patch)
2023-10-11 18:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove PatronLastActivity preference (18.84 KB, patch)
2023-10-11 20:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.26 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.48 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.68 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.79 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.92 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.57 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.66 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find (937 bytes, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Add Triggers mock to unit tests (1.46 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test (8.44 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Fix is_active test (1.01 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity (2.02 KB, patch)
2023-10-12 06:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Rename cache key (1.62 KB, patch)
2023-10-12 06:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Terminology (1.25 KB, patch)
2023-10-12 06:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Update terminology on cleanborrowers (1.39 KB, patch)
2023-10-12 06:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons (1.73 KB, patch)
2023-10-12 06:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: Remove PatronLastActivity preference (18.84 KB, patch)
2023-10-12 06:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Fix dbrev (1.80 KB, patch)
2023-10-12 07:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.35 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.58 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.76 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.88 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Add unit test (7.02 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.66 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.74 KB, patch)
2023-10-12 09:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find (1.01 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Add Triggers mock to unit tests (1.55 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test (8.53 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Fix is_active test (1.10 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity (2.11 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Rename cache key (1.71 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Terminology (1.35 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Update terminology on cleanborrowers (1.48 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons (1.82 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Remove PatronLastActivity preference (18.91 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Fix dbrev (1.80 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Final changes (6.16 KB, patch)
2023-10-12 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15504: Add new syspref TrackLastPatronActivityTriggers (4.31 KB, patch)
2023-10-22 07:22 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: Update track_login_daily to accept triggers (1.54 KB, patch)
2023-10-22 07:22 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: Add triggers to instances of track_login_daily (3.72 KB, patch)
2023-10-22 07:22 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: Remove update_lastseen method (2.85 KB, patch)
2023-10-22 07:22 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: Add unit test (6.98 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Unit tests (15.62 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Migrate to one clear method (7.70 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find (992 bytes, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Add Triggers mock to unit tests (1.51 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test (8.49 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Fix is_active test (1.06 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity (2.07 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Rename cache key (1.68 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Terminology (1.31 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Update terminology on cleanborrowers (1.44 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons (1.78 KB, patch)
2023-10-22 07:23 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: Remove PatronLastActivity preference (19.01 KB, patch)
2023-10-22 07:24 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Fix dbrev (1.76 KB, patch)
2023-10-22 07:24 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (QA follow-up) Final changes (6.12 KB, patch)
2023-10-22 07:24 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 15504: (QA follow-up): more consistent quotes in atomicupdate (1.15 KB, patch)
2023-10-22 20:32 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2016-01-06 16:48:40 UTC
Work to be done:

    Add new timestamp field ‘last_activity’ to the ‘borrowers’ table

    Update the field if one of the following ocurrs:
        Patron checks out item
        Patron renews an item
        Patron returns an item
        Patron is authenticated via SIP2
        Patron logs into OPAC


Seeking Co-sponsors: http://devs.bywatersolutions.com/projects/track-patrons-last-activity/
Comment 1 Christopher Brannon 2016-03-18 22:38:33 UTC
Please consider having another field that would record the type of activity logged, just like you have in your list.

Christopher
Comment 2 David Cook 2019-02-18 04:53:15 UTC
I noticed recently that "lastseen" updates for "Patron logs into OPAC" and "Patron is authenticated via SIP2" and that's it. I've had libraries interested in the field being updated for the other activities Nicole mentioned as well.
Comment 3 Jonathan Druart 2019-02-22 12:16:30 UTC
(In reply to David Cook from comment #2)
> I noticed recently that "lastseen" updates for "Patron logs into OPAC" and
> "Patron is authenticated via SIP2" and that's it. I've had libraries
> interested in the field being updated for the other activities Nicole
> mentioned as well.

Which ones? I do not understand what's missing.

Marked as duplicate of bug 16276 as borrowers.lastseen has been added by this bug.

*** This bug has been marked as a duplicate of bug 16276 ***
Comment 4 David Cook 2019-02-24 23:23:08 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to David Cook from comment #2)
> > I noticed recently that "lastseen" updates for "Patron logs into OPAC" and
> > "Patron is authenticated via SIP2" and that's it. I've had libraries
> > interested in the field being updated for the other activities Nicole
> > mentioned as well.
> 
> Which ones? I do not understand what's missing.
> 
> Marked as duplicate of bug 16276 as borrowers.lastseen has been added by
> this bug.
> 
> *** This bug has been marked as a duplicate of bug 16276 ***

Nicole listed 5 activities:

Patron checks out item
Patron renews an item
Patron returns an item
Patron is authenticated via SIP2
Patron logs into OPAC

Only the last two update "lastseen". 

If you're a borrower who never uses a self-checkout or logs into the OPAC, but you frequently check out materials via a staff member, your "lastseen" value will be really old even though you're a frequent user of the library. 

I've had libraries complain that they're trying to find inactive borrowers using "lastseen" but it's including active borrowers who don't log into Koha.
Comment 5 David Cook 2019-02-24 23:24:03 UTC
So I don't think this is a duplicate at all. I'm going to change the status back.
Comment 6 Jonathan Druart 2019-03-04 18:03:19 UTC
Indeed, you are right.
The description of bug 16276 says:
"""
We would like to automate the delation of expired patrons to comply to french laws regarding privacy. But we don't want to delate an expired patron which is still using the OPAC for bibliographical purpose, like managing lists. We would like to be able to say when the patron logged himself for the last time to permit this.
"""

It could be renamed last_login to avoid ambiguity.
Comment 7 Martha Fuerst 2019-05-17 20:46:55 UTC
Except that calling it last_login doesn't indicate the SIP2 connection, which is important. In our case, a patron may use their card 100% for databases or digital materials, so tracking the SIP2 is key for this.
Comment 8 Katrin Fischer 2020-01-08 22:45:57 UTC
*** Bug 14513 has been marked as a duplicate of this bug. ***
Comment 9 Agnes Rivers-Moore 2020-01-09 02:22:11 UTC
Summary so far - 
A timestamp, updated by checkout / renew / return / SIP2 / OPAC login. (Nicole)
The type of activity needs to be indicated as well as the timestamp. (Christopher)
SIP2 activity needs to be tracked. (Martha)
Noted "Lastseen" seems to capture OPAC login and authenticated by SIP. This would be the "updated" timestamp displayed in the patron summary.

My thoughts -
It would help to know more about how people want to use this timestamp. Is it for reports (inactive patrons for weeding, or activity for statistics), or to be visible to staff in the patron record, like the "Updated" timestamp? 
We would need clearer labels - "updated" implies but does not actually indicate when the borrower record was renewed or edited.

Checkout / renew / return (whether through intranet, OPAC or SIP) are one kind of activity, and are captured already in circulation history. OPAC login and SIP (e.g. for authentication of outside systems like Overdrive) are another, and they are also already captured somewhere but not clear to staff. Should they be handled separately or feed into a single last activity date?

I would also like a way to update that activity timestamp without checking out. Something like the "statistical borrower" but a "statistical item" that could be scanned to record library activity not connected to Koha. For example, people who use computers or sign up for programmes. Maybe there is one I have missed.

I agree with Martha that tracking SIP2 connections is important, and perhaps should be addressed in more detail. We use it for self-checkout, computer booking (in-library activities) and authentication for accessing Overdrive, RBDigital (external resources).

Some label suggestions - last-login (OPAC or self-checkout), updated (patron details changed through OPAC or Intranet), last-activity (activity from any channel including SIP), remote-use (outside resource SIP connections and OPAC login). 
Just suggestions, for wider discussion.
Comment 10 Christopher Brannon 2020-01-09 18:14:26 UTC
(In reply to Agnes Rivers-Moore from comment #9)
> Some label suggestions - last-login (OPAC or self-checkout), updated (patron
> details changed through OPAC or Intranet), last-activity (activity from any
> channel including SIP), remote-use (outside resource SIP connections and
> OPAC login). 
> Just suggestions, for wider discussion.

I'm concerned with over complicating the information.  In my opinion, there are a couple things we want to track here:

* Patron record modification
* Patron activity (login/checkouts through any connection)

Timestamps are generic, and a catch-all.  That is good for logs, but the two above are more specific.  These are two that need to be recorded in the patron record, that we don't have to scrounge for in logs that are not necessarily recorded, or are purged on a regular basis.

I'm not sure about last-activity or remote-use.  Those are a little more granular and bleed over into the other two mentioned above and might make it more confusing.  But this is my opinion.
Comment 11 Agnes Rivers-Moore 2020-01-09 18:39:02 UTC
Agree with Christopher about the library activities - we just want the labels to clearly describe what activity sets the timestamp.
To clarify - Martha and I definitely want the SIP2 activity to be tracked separately from checkouts, because SIP2 includes activities that are outside Koha circulation.
Comment 12 Christopher Brannon 2020-01-09 18:55:00 UTC
So maybe:

* Patron record modification
* Any patron activity (login/checkouts through any connection)
* SIP patron activity (part of previous count)
Comment 13 Christopher Brannon 2020-06-29 22:40:53 UTC
Folks, this is a rather important issue.  Would love to see this progress beyond 4 years of rare discussion.  :D
Comment 14 Christopher Brannon 2020-06-29 22:46:12 UTC
To add to this, it is really holding us back on tracking active patrons.  We cannot clean out our patron databases on inactive accounts, because currently Koha is not tracking activity through 3rd party databases or computer checkouts via SIP authentication.  This really needs some momentum.
Comment 15 David Cook 2020-06-30 00:00:44 UTC
(In reply to Christopher Brannon from comment #13)
> Folks, this is a rather important issue.  Would love to see this progress
> beyond 4 years of rare discussion.  :D

I agree that it's important, but it's not a priority for us at this time.

I notice this issue is marked as "Seeking cosponsors". It might be worthwhile to send an email to the koha-devel list to see if someone wants to work on it, or if you have funds - look to hire someone to do the work.
Comment 16 Victor Grousset/tuxayo 2020-06-30 03:05:24 UTC
(In reply to David Cook from comment #2)
> I noticed recently that "lastseen" updates for "Patron logs into OPAC" and
> "Patron is authenticated via SIP2" and that's it.

IIUC all the data is already there albeit in various tables right?
So, is the functionality achievable via a report? (which could be included by default & upgrades)
Comment 17 Christopher Brannon 2020-06-30 15:11:27 UTC
(In reply to David Cook from comment #15)
> (In reply to Christopher Brannon from comment #13)
> > Folks, this is a rather important issue.  Would love to see this progress
> > beyond 4 years of rare discussion.  :D
> 
> I agree that it's important, but it's not a priority for us at this time.
> 
> I notice this issue is marked as "Seeking cosponsors". It might be
> worthwhile to send an email to the koha-devel list to see if someone wants
> to work on it, or if you have funds - look to hire someone to do the work.

The link to dev list is a dead link.

"Not a priority for us"...who is "US"?
Comment 18 Martha Fuerst 2020-06-30 15:42:51 UTC
Chiming in again.

This is ESPECIALLY important for us now, with COVID-19 and social distancing procedures, we have more people using 100% online resources - and people who have gotten cards using online registration, verified via email, to use those resources.

Tracking last patron activity via SIP2/OPAC login is definitely a priority for US at Huntsville-Madison County.

Best,

-Marti Fuerst
Comment 19 Agnes Rivers-Moore 2020-06-30 15:56:32 UTC
(In reply to Martha Fuerst from comment #18)
> Chiming in again.
> 
> This is ESPECIALLY important for us now, with COVID-19 and social distancing
> procedures, we have more people using 100% online resources - and people who
> have gotten cards using online registration, verified via email, to use
> those resources.
> 
> Tracking last patron activity via SIP2/OPAC login is definitely a priority
> for US at Huntsville-Madison County.
> 
> Best,
> 
> -Marti Fuerst

+1 - COVID-19 has certainly increased the urgency on this!
Comment 20 Victor Grousset/tuxayo 2020-06-30 19:03:42 UTC
(In reply to Christopher Brannon from comment #17)
> The link to dev list is a dead link.

http://devs.bywatersolutions.com/projects/track-patrons-last-activity/ isn't the dev list. It was a page ByWater Solutions had on this topic.


The mailing lists are here:

https://koha-community.org/support/koha-mailing-lists/

«General List» and «Koha Developers List» seems to me the good ones to seek organizations to cosponsor or/and to contract to work on this.

(assuming SQL report are not enough?)
Comment 21 Christopher Brannon 2020-06-30 19:14:29 UTC
(In reply to Victor Grousset/tuxayo from comment #20)
 
> (assuming SQL report are not enough?)

As was mentioned, pulling stuff from activity logs is hit or miss.  Depending on how often information is purged, you won't get very accurate results.  This is data that is best recorded in the patron record or related table.
Comment 22 Victor Grousset/tuxayo 2020-06-30 19:34:10 UTC
(In reply to Christopher Brannon from comment #21)
> (In reply to Victor Grousset/tuxayo from comment #20)
>  
> > (assuming SQL report are not enough?)
> 
> As was mentioned, pulling stuff from activity logs is hit or miss. 

I missed that part.
What do you mean by activity logs? The issues and old_issues tables?

Because if I understand correctly, the following data is in the above tables + borrowers.lastseen

> Patron checks out item
> Patron renews an item
> Patron returns an item
> Patron is authenticated via SIP2
> Patron logs into OPAC
Comment 23 Christopher Brannon 2020-06-30 19:48:01 UTC
I must be missing something here.  Where is the information on patron logins to OPAC or authentication via SIP2 that isn't in a log?  I'm not sure what tables I should be looking at for this information.
Comment 24 Victor Grousset/tuxayo 2020-06-30 21:16:12 UTC
(In reply to Christopher Brannon from comment #23)
> I must be missing something here.  Where is the information on patron logins
> to OPAC or authentication via SIP2 that isn't in a log?

borrowers.lastseen IIUC
Comment 25 Victor Grousset/tuxayo 2020-06-30 21:18:04 UTC
Please double check, as a non-native speaker, misunderstandings are bound to happen someday :)

(In reply to David Cook from comment #2)
> I noticed recently that "lastseen" updates for "Patron logs into OPAC" and
> "Patron is authenticated via SIP2" and that's it. I've had libraries
> interested in the field being updated for the other activities Nicole
> mentioned as well.
Comment 26 David Cook 2020-07-01 02:49:09 UTC
(In reply to Christopher Brannon from comment #17)
> "Not a priority for us"...who is "US"?

Ah, fair question! I  mean the vendor I work for Prosentient Systems and the libraries that it supports. None of our libraries are signalling this as a priority for us, so we're working on other things that are more important for them. 

I should've been more precise with my language :).
Comment 27 Cab Vinton 2020-09-10 18:54:22 UTC
Agreeing w/ Martha & Agnes that lastseen dates should also reflect patron checkout, renewal, & return activity.
Comment 28 Magnus Enger 2020-09-14 13:01:41 UTC
In order to be complete, I think we need to expand this:

  Patron is authenticated via SIP2

into at least this: 

  Patron is authenticated via SIP2, ILS-DI, REST API

We have ebook services that authenticate patrons via ILS-DI, and if that is all they use the library for they will not be counted as active users.
Comment 29 Agnes Rivers-Moore 2020-09-17 22:53:22 UTC
I would like to suggest, respectfully, that the bug title be changed to describe this topic more accurately. As it stands it appears to be about tracking last Koha activity, but we really need tracking activity in outside resources via SIP2, ILS-DI, REST API
Comment 30 Jonathan Druart 2020-12-17 11:09:12 UTC
*** Bug 27243 has been marked as a duplicate of this bug. ***
Comment 31 Magnus Enger 2020-12-28 11:55:23 UTC
Relevat code:

C4/SIP/ILS/Patron.pm:

476 =head2 update_lastseen
477 
478     $patron->update_lastseen();
479 
480     Patron method to update lastseen field in borrower
481     to record that patron has been seen via sip connection
482 
483 =cut
484 
485 sub update_lastseen {
486     my $self = shift;
487     my $kohaobj = Koha::Patrons->find( $self->{borrowernumber} );
488     $kohaobj->track_login if $kohaobj; # track_login checks the pref
489 }

Koha/Patron.pm:

910 =head3 track_login
911 
912     $patron->track_login;
913     $patron->track_login({ force => 1 });
914 
915     Tracks a (successful) login attempt.
916     The preference TrackLastPatronActivity must be enabled. Or you
917     should pass the force parameter.
918 
919 =cut
920 
921 sub track_login {
922     my ( $self, $params ) = @_;
923     return if
924         !$params->{force} &&
925         !C4::Context->preference('TrackLastPatronActivity');
926     $self->lastseen( dt_from_string() )->store;
927 }
Comment 32 Andrew Fuerste-Henry 2021-06-25 11:37:56 UTC
Bug 28455, signed off, adds checkouts to lastseen. So we're kinda working in two different directions here.

I'm in favor of a simple approach here: a single field with a datetime that updates each time a patron does any of the listed actions (log in to OPAC, SIP, ILSDI, API, checkout, checkin).

The batch patron deletion tool already knows how to filter by both lastseen and circ activity. The need I run into here relates to reports rather than the deletion tool itself. We get regular requests for a report that will give a column of the last date on which a patron either logged in to something or checked something in or out. That's much messier SQL than it needs to be with the current data structure.
Comment 33 Christopher Brannon 2021-08-06 19:13:37 UTC
(In reply to Andrew Fuerste-Henry from comment #32)
> Bug 28455, signed off, adds checkouts to lastseen. So we're kinda working in
> two different directions here.
> 
> I'm in favor of a simple approach here: a single field with a datetime that
> updates each time a patron does any of the listed actions (log in to OPAC,
> SIP, ILSDI, API, checkout, checkin).
> 
> The batch patron deletion tool already knows how to filter by both lastseen
> and circ activity. The need I run into here relates to reports rather than
> the deletion tool itself. We get regular requests for a report that will
> give a column of the last date on which a patron either logged in to
> something or checked something in or out. That's much messier SQL than it
> needs to be with the current data structure.

+1.  I would add, if it is a need for anyone to define what triggers this field, a permission with checkboxes of actions that would trigger it would finish this off nicely.
Comment 34 Matt Blenkinsop 2023-06-14 14:21:27 UTC
Created attachment 152332 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen
Comment 35 Matt Blenkinsop 2023-06-14 14:21:30 UTC
Created attachment 152333 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active
Comment 36 Matt Blenkinsop 2023-06-14 14:21:34 UTC
Created attachment 152334 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily
Comment 37 Matt Blenkinsop 2023-06-14 14:21:38 UTC
Created attachment 152335 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily
Comment 38 Matt Blenkinsop 2023-06-14 14:21:42 UTC
Created attachment 152336 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t
Comment 39 Matt Blenkinsop 2023-06-14 14:22:16 UTC
This bug hasn’t been active for a while but we recently had a support ticket from a customer that seemed to fit into this discussion. By comparing that with the discussion on this bug I’ve prepared a patch that will hopefully satisfy the requirement of both. There are now two sysprefs - “TrackLastPatronActivity” and “TrackLastPatronActivityTriggers”. The second of these offers a list of the following:

-                login: "Login to OPAC"
-                connection: "Connection to Koha via SIP or ILSDI"
-                check_out: "Checking out an item"
-                renewal: "Renewing an item"
-                check_in: "Returning an item"

When TrackLastPatronActivity is enabled and one or more of these triggers is selected, borrowers.lastseen will update when that trigger happens. This will only happen once per day as per the current method. If this structure/concept is approved then expanding it with further options should be fairly simple if required.
Comment 40 Matt Blenkinsop 2023-06-14 14:22:44 UTC
Notes on testing - The intent of track_login_daily is to log a patron action/activity once per day and cache this activity. This means that the lastseen field is only updated once per day rather than every time an action is performed. This makes sense in operation but makes testing this patch more complicated. We want to test multiple methods of updating the lastseen field. If we don’t clear that cache before each method then the patch will work as intended but the test plan will fail as only one update will be performed. The test plan therefore includes a “restart_all” step at various points before an activity is performed - this step uses “restart_all” in KTD to clear the cache and must not be skipped otherwise the test plan will fail. Furthermore, the “Login to OPAC” action, called in checkauth is triggered very often throughout Koha through various route guards etc. The test plan begins by testing this activity first and then de-selecting this option. If this option is left selected, then it will trigger before the other actions (check in, check out etc) can be triggered and the test plan will fail due to the login action already being cached again.


Test plan:
1. Apply patch
2. reset_all
3. Navigate to Administration and search for “TrackLastPatronActivity”
4. Observe that there are two system preferences - “TrackLastPatronActivity” and “TrackLastPatronActivityTriggers”
5. Change TrackLastPatronActivity to “Do” and save
6. Choose a patron to use as a test subject
7. In the database, observe that the “lastseen” property for that patron is still NULL
8. In TrackLastPatronActivityTriggers, select “Login to OPAC” and save
9. In the OPAC, log in as your selected patron
10. In the database, observe that the “lastseen” property for that patron has been updated
11. In TrackLastPatronActivityTriggers, unselect “Login to OPAC” and instead select “Checking out an item” and save
12. restart_all
13. Check out an item to your selected patron
14. In the database, observe that the “lastseen” property for that patron has been updated
15. In TrackLastPatronActivityTriggers,  select “Renewing an item” and save
16. restart_all
17. In either the OPAC or the staff client - renew the item that you have just checked out to that patron
18. In the database, observe that the “lastseen” property for that patron has been updated
19. In TrackLastPatronActivityTriggers,  select “Returning an item” and save
20. restart_all
21. Return the item that you have checked out to the patron
22. In the database, observe that the “lastseen” property for that patron has been updated
23. In TrackLastPatronActivityTriggers,  select “Connection to Koha via SIP or ILSDI” and save
24. In system preferences, search for “ILS-DI” and enable this preference
25. restart_all
26. In the browser, navigate to http://DOMAIN/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=USERNAME&password=PASSWORD, swapping out DOMAIN, USERNAME and PASSWORD to match your koha instance and patron details
27. In the database, observe that the “lastseen” property for that patron has been updated
28. Run “prove -v t/db_dependent/Auth.t”
29. Observe that all tests pass
30. Sign off!
Comment 41 Martin Renvoize 2023-06-21 09:36:59 UTC
Created attachment 152500 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2023-06-21 09:37:04 UTC
Created attachment 152501 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize 2023-06-21 09:37:09 UTC
Created attachment 152502 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2023-06-21 09:37:13 UTC
Created attachment 152503 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2023-06-21 09:37:17 UTC
Created attachment 152504 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2023-06-21 09:37:21 UTC
Created attachment 152505 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2023-06-21 09:37:26 UTC
Created attachment 152506 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2023-06-21 09:38:50 UTC
Internal code review done, and a couple of follow-ups added as I felt we could clean up some of the code (not related to Matts work directly, but whilst I was in here I wanted to do the follow-ups)

Nice work Matt, all working as designed in my testing.
Comment 49 Martin Renvoize 2023-09-07 11:52:15 UTC
Created attachment 155290 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2023-09-07 11:52:19 UTC
Created attachment 155291 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2023-09-07 11:52:23 UTC
Created attachment 155292 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2023-09-07 11:52:27 UTC
Created attachment 155293 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2023-09-07 11:52:31 UTC
Created attachment 155294 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2023-09-07 11:52:35 UTC
Created attachment 155295 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2023-09-07 11:52:39 UTC
Created attachment 155296 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2023-09-07 11:53:30 UTC
Rebased to drop dependency on bug 33997
Comment 57 David Nind 2023-09-12 05:55:25 UTC
Created attachment 155534 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 58 David Nind 2023-09-12 05:55:45 UTC
Created attachment 155535 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 59 David Nind 2023-09-12 05:56:03 UTC
Created attachment 155536 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 60 David Nind 2023-09-12 05:56:24 UTC
Created attachment 155537 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 61 David Nind 2023-09-12 05:56:46 UTC
Created attachment 155538 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 62 David Nind 2023-09-12 05:57:12 UTC
Created attachment 155539 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 63 David Nind 2023-09-12 05:57:32 UTC
Created attachment 155540 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 64 David Nind 2023-09-12 06:14:25 UTC
I've added my sign-off as well. 8-)

Testing notes (using KTD):

1. Followed test plan in comment #40.

2. For step 7, I:
   2.1 used patron 49 (Mary), updated card number and password for testing
   2.2 database checking (lastseen is near the end in the field before 'default' (lang column)):
        koha-mysql kohadev
        select * from borrowers where borrowernumber=49;
Comment 65 Andrii Nugged 2023-09-25 17:20:50 UTC
(In reply to David Nind from comment #63)
> Created attachment 155540 [details] [review] [review]
> Bug 15504: (follow-up) Migrate to one clear method

    my $patron = Koha::Patrons->find({ userid => $userid });
    $patron->update_lastseen('login');

I've got one case when it returned empty $patron, so that led to:

    [ERROR] Can't call method "update_lastseen" on an undefined value
    at /usr/share/koha/lib/C4/Auth.pm line xxx.

Yet it was when I tried to religion from an older session, can't reproduce 1:1 now, but giving a warning now for team + researching myself further.
Comment 66 Andrii Nugged 2023-09-25 18:34:40 UTC
(In reply to Andrii Nugged from comment #65)
> Yet it was when I tried to religion from an older session, can't reproduce
> 1:1 now, but giving a warning now for team + researching myself further.

Own update: it's because $userid also comes empty to that code lines (in cases when old session? thinking aloud)
Comment 67 Katrin Fischer 2023-10-02 20:32:33 UTC
*** Bug 21007 has been marked as a duplicate of this bug. ***
Comment 68 Andrii Nugged 2023-10-04 17:13:54 UTC
One bug in SIP here:

File: C4/SIP/Sip/MsgType.pm
0999:         if ( C4::Context->preference('TrackLastPatronActivity') ) {
1000:             my $koha_patron = Koha::Patrons->find($patron->userid);
1001:             $koha_patron->update_lastseen('connection');
1002:         }
1003:         $resp .= patron_status_string( $patron, $server );

ERROR: Cannot access 'userid' field of class 'C4::SIP::ILS::Patron' at /usr/share/koha/lib/C4/SIP/Sip/MsgType.pm

That:

            my $koha_patron = Koha::Patrons->find($patron->userid);

should be, probably, this way:

            my $koha_patron = Koha::Patrons->find( { cardnumber => $patron->id } );
Comment 69 Katrin Fischer 2023-10-04 19:22:40 UTC
Please switch to failed QA if there is a bug in testing.
Comment 70 Martin Renvoize 2023-10-06 10:36:05 UTC
Rebasing now bug 33245 is in.  I think bug 33245 could be significantly simplified once this is in personally..
Comment 71 Martin Renvoize 2023-10-06 10:47:29 UTC
Created attachment 156598 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 72 Martin Renvoize 2023-10-06 10:47:33 UTC
Created attachment 156599 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 73 Martin Renvoize 2023-10-06 10:47:38 UTC
Created attachment 156600 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 74 Martin Renvoize 2023-10-06 10:47:42 UTC
Created attachment 156601 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 75 Martin Renvoize 2023-10-06 10:47:46 UTC
Created attachment 156602 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 76 Martin Renvoize 2023-10-06 10:47:51 UTC
Created attachment 156603 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 77 Martin Renvoize 2023-10-06 10:47:55 UTC
Created attachment 156604 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 78 Martin Renvoize 2023-10-06 10:47:59 UTC
Created attachment 156605 [details] [review]
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find
Comment 79 Martin Renvoize 2023-10-06 10:48:03 UTC
Created attachment 156606 [details] [review]
Bug 15504: (follow-up) Add Triggers mock to unit tests
Comment 80 Martin Renvoize 2023-10-06 10:48:07 UTC
Created attachment 156607 [details] [review]
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test
Comment 81 Martin Renvoize 2023-10-06 10:48:12 UTC
Created attachment 156608 [details] [review]
Bug 15504: (follow-up) Fix is_active test

Bug 33245 introduced a new is_active patron method and the corresponding
tests included calls to the method we depreacte in this patchset
Comment 82 Andrii Nugged 2023-10-09 13:24:44 UTC
QA-question:

This sub:

    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};

        ...

is called very often (on each Auth-request, for each borrower, so each page load or whatever happens, then for every other event).

Is that "split" of static variable from preference('TrackLastPatronActivityTriggers') justified to be called that way EVERY time, splitting again and again?
Comment 83 Martin Renvoize 2023-10-09 14:01:06 UTC
Created attachment 156742 [details] [review]
Bug 15504: (QA follow-up) Add caching to update_lastseen

This patch caches the activity triggers hash upon the first call to
update_lastseen.
Comment 84 Martin Renvoize 2023-10-09 14:02:19 UTC
Final follow-up adds caching.. but I've not benchmarked it.. would be interesting to see if the cache fetch + check it's valid is any quicker than doing the split inline though...
Comment 85 Andrii Nugged 2023-10-09 14:49:15 UTC
> check it's valid is any quicker than doing the split inline though...

you said this - it's reasonable. I agree that we need to use our hivemind to choose because it might be "six of one/half a dozen",

if that's Memcached, then it's probably heavier than splitting.
But if to use Perl-code cache, then it might be faster or close to.

Hmmm.
Comment 86 Andrii Nugged 2023-10-09 14:52:07 UTC
Then invalidating the cache is another timely issue, and so on.
Sic :) (now I feel guilty I even asked for that xD ).
Comment 87 Jonathan Druart 2023-10-10 07:54:26 UTC
This is cached in L2 (memcached) but L1 will be populated (memory).
So the retrieval of the syspref should be fast.

But here we are talking about caching the split of a very short string, and we are in perl, I am not sure it makes a lot of sense.
Comment 88 Andrii Nugged 2023-10-10 15:19:15 UTC
Let's leave split by Perl (revert attachment 156742 [details] [review] away) and add an explicit comment to the code why we are not caching for future generations of developers who will face this code?
Comment 89 Andrii Nugged 2023-10-10 15:21:42 UTC
Adn one more thing:

Not covered HOLDS.

When the borrower makes hold on themself (but this is slightly covered with the "login" part) but also when the operator makes hold on the borrower:

should this be +1 place in the code

+ tracking option in the dropdown among "renewal, checkout, check-in"?
Comment 90 Marcel de Rooy 2023-10-10 15:45:29 UTC
(In reply to Andrii Nugged from comment #89)
> Not covered HOLDS.

Recalls, article requests ?
Note that they could be covered in a follow-up. But they seem relevant too.
Comment 91 Martin Renvoize 2023-10-11 12:20:53 UTC
Obsoleted final follow-up..

I'd really like to see this actually move forward rather than get stuck in discussion hell with ever expanding requirements.. 

If you want more options adding, please either actually submit code or create a new bug that hangs off this one.
Comment 92 Marcel de Rooy 2023-10-11 12:34:40 UTC
Will be looking here very SOON
Comment 93 Martin Renvoize 2023-10-11 12:38:08 UTC
(In reply to Marcel de Rooy from comment #90)
> (In reply to Andrii Nugged from comment #89)
> > Not covered HOLDS.
> 
> Recalls, article requests ?
> Note that they could be covered in a follow-up. But they seem relevant too.

Can an article request be triggered by staff on behalf of a user?
Comment 94 Marcel de Rooy 2023-10-11 12:51:24 UTC
(In reply to Martin Renvoize from comment #93)
> (In reply to Marcel de Rooy from comment #90)
> > (In reply to Andrii Nugged from comment #89)
> > > Not covered HOLDS.
> > 
> > Recalls, article requests ?
> > Note that they could be covered in a follow-up. But they seem relevant too.
> 
> Can an article request be triggered by staff on behalf of a user?

Yes.
Comment 95 Marcel de Rooy 2023-10-11 13:02:32 UTC
(In reply to Marcel de Rooy from comment #90)
> (In reply to Andrii Nugged from comment #89)
> > Not covered HOLDS.
> 
> Recalls, article requests ?

Recalls cant be set in staff for another user. So not applicable here.
Comment 96 Marcel de Rooy 2023-10-11 14:27:00 UTC
OK. This looks almost ready to me. No panic please.
Here are some minor comments. The first and last one should get attention here imo.

dbrev
Blocker
The default should probably be LOGIN IF TrackLastLogin was enabled.
Please adjust.

Preferences
Clearing all triggers is the same as turning off existing pref? Could we remove it ? Here or in a follow-up ?

my $cache_key = "track_login_" . $self->userid;
No blocker. Would probably be better to cache with borrowernumber. Userid may change?

+    Time::Fake->offset( $now->epoch + 5 );
No blocker, just teasing :)
You might have trouble in case you run the test close to midnight.
Could we cache the epoch value ?

-sub track_login {
-    my ( $self, $params ) = @_;
-    return if
-        !$params->{force} &&
Didnt we use force somewhere before?

Login from OPAC => Login
=> Staff login counts as well. I think we should just remove OPAC here.

This code still needs attention.
Blocker
koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
                [% IF Koha.Preference('TrackLastPatronActivity') %]
                    <li>
                        <label for="borrower_lastseen">who have not been connected since:</label>
                        <input size="10" id="borrower_lastseen" name="borrower_lastseen" type="text" class="flatpickr" />
                        <span class="hint">[% INCLUDE 'date-format.inc' %]</span>
                    </li>
                [% END %]
AND Similarly:
misc/cronjobs/delete_patrons.pl
=item B<--last_seen>
Delete patrons who have not been connected since this date.
The system preference TrackLastPatronActivity must be enabled to use this option.
Comment 97 Marcel de Rooy 2023-10-11 14:29:00 UTC
Opened bug 35050 to extend with placing hold or article request.
Comment 98 Marcel de Rooy 2023-10-11 14:31:51 UTC
(In reply to Marcel de Rooy from comment #97)
> Opened bug 35050 to extend with placing hold or article request.

bug 35030
Comment 99 Marcel de Rooy 2023-10-11 14:35:54 UTC
Doing 1000 splits in a  for loop:

$VAR1 = '1697034924.81935';
$VAR2 = '1697034924.82318';
$VAR3 = '0.00383496284484863';

Sp no big deal
Comment 100 Marcel de Rooy 2023-10-11 14:43:09 UTC
I did not see 35027. So 35030 should only be article request.
Comment 101 Martin Renvoize 2023-10-11 16:58:44 UTC
(In reply to Marcel de Rooy from comment #96)
> dbrev
> Blocker
> The default should probably be LOGIN IF TrackLastLogin was enabled.
> Please adjust.

Done, will post patch momentarily.. however, it's actually 'connection,login,check_out' to match the existing behaviour.

> Preferences
> Clearing all triggers is the same as turning off existing pref? Could we
> remove it ? Here or in a follow-up ?

I like that, perhaps as a quick follow-up bug.. I'm happy to code that tonight.

> my $cache_key = "track_login_" . $self->userid;
> No blocker. Would probably be better to cache with borrowernumber. Userid
> may change?

Totally agree on this one.. though we just used the existing key.  I've amended it now as another follow-up anyway.. about to upload.

> +    Time::Fake->offset( $now->epoch + 5 );
> No blocker, just teasing :)
> You might have trouble in case you run the test close to midnight.
> Could we cache the epoch value ?

Oh, this could be fun.. one for later though in my opinion.. should it crop up.

> -sub track_login {
> -    my ( $self, $params ) = @_;
> -    return if
> -        !$params->{force} &&
> Didnt we use force somewhere before?

It was supported before, but never actually used. Matt and I discussed it and decided to remove the functionality for now as it was never called that way.

> Login from OPAC => Login
> => Staff login counts as well. I think we should just remove OPAC here.

Fair, I'll do a follow-up for that too before posting up the current rebase.

> This code still needs attention.
> Blocker
> koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
>                 [% IF Koha.Preference('TrackLastPatronActivity') %]
>                     <li>
>                         <label for="borrower_lastseen">who have not been
> connected since:</label>
>                         <input size="10" id="borrower_lastseen"
> name="borrower_lastseen" type="text" class="flatpickr" />
>                         <span class="hint">[% INCLUDE 'date-format.inc'
> %]</span>
>                     </li>
>                 [% END %]
> AND Similarly:
> misc/cronjobs/delete_patrons.pl
> =item B<--last_seen>
> Delete patrons who have not been connected since this date.
> The system preference TrackLastPatronActivity must be enabled to use this
> option.

Looking at these two now.
Comment 102 Martin Renvoize 2023-10-11 17:13:22 UTC
Created attachment 156855 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 103 Martin Renvoize 2023-10-11 17:13:26 UTC
Created attachment 156856 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 104 Martin Renvoize 2023-10-11 17:13:30 UTC
Created attachment 156857 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 105 Martin Renvoize 2023-10-11 17:13:35 UTC
Created attachment 156858 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 106 Martin Renvoize 2023-10-11 17:13:39 UTC
Created attachment 156859 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 107 Martin Renvoize 2023-10-11 17:13:43 UTC
Created attachment 156860 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 108 Martin Renvoize 2023-10-11 17:13:48 UTC
Created attachment 156861 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 109 Martin Renvoize 2023-10-11 17:13:52 UTC
Created attachment 156862 [details] [review]
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find
Comment 110 Martin Renvoize 2023-10-11 17:13:56 UTC
Created attachment 156863 [details] [review]
Bug 15504: (follow-up) Add Triggers mock to unit tests
Comment 111 Martin Renvoize 2023-10-11 17:14:01 UTC
Created attachment 156864 [details] [review]
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test
Comment 112 Martin Renvoize 2023-10-11 17:14:05 UTC
Created attachment 156865 [details] [review]
Bug 15504: (follow-up) Fix is_active test

Bug 33245 introduced a new is_active patron method and the corresponding
tests included calls to the method we depreacte in this patchset
Comment 113 Martin Renvoize 2023-10-11 17:14:10 UTC
Created attachment 156866 [details] [review]
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity

This patch updates the database update to set
'check_out,connection,login' as the activity triggers should we find at
upgrade time that TrackLastPatronActivity is set to true.

This retains the existing functionality.
Comment 114 Martin Renvoize 2023-10-11 17:14:14 UTC
Created attachment 156867 [details] [review]
Bug 15504: (follow-up) Rename cache key

I agree, it's better to use borrowernumber for the cache key and I also
take the oportunity to rename it to more clearly reflect the function
(it's no longer 'login' only, it's now 'activity')
Comment 115 Martin Renvoize 2023-10-11 17:14:18 UTC
Created attachment 156868 [details] [review]
Bug 15504: (QA follow-up) Terminology

This just updates 'Login to OPAC' to 'Login'.. we should squash this
Comment 116 Martin Renvoize 2023-10-11 17:14:22 UTC
Created attachment 156869 [details] [review]
Bug 15504: (follow-up) Update terminology on cleanborrowers

This patch updates the terminology on the cleanborrowers form to reflect
that 'activity' and not just 'connectivity' is tracked by
TrackLastPatronActivity.
Comment 117 Martin Renvoize 2023-10-11 17:14:27 UTC
Created attachment 156870 [details] [review]
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons

This patch adds clarifications to the last_seen option in delete_patrons
to clarify that TrackLastPatronActivity and
TrackLastPatronActivityTriggers system preferences act as a couplet.
Comment 118 Martin Renvoize 2023-10-11 18:38:28 UTC
Created attachment 156875 [details] [review]
Bug 15504: Remove PatronLastActivity preference

This patch removes the older PatronLastActivity preference opting to
replace it by the new PatronLastActivityTriggers preference. The feature
can now be disabled by simply not selecting any triggers
Comment 119 Martin Renvoize 2023-10-11 18:40:17 UTC
I opted to just fold in the removal of TrackLastPatronActivity.. it can be moved to it's own bug if you prefer.. or squashed a bit.. I kept them as distict patches for now just to make it clear where I've responded to each of the feedback points.
Comment 120 Martin Renvoize 2023-10-11 20:22:47 UTC
Created attachment 156879 [details] [review]
Bug 15504: Remove PatronLastActivity preference

This patch removes the older PatronLastActivity preference opting to
replace it by the new PatronLastActivityTriggers preference. The feature
can now be disabled by simply not selecting any triggers
Comment 121 Marcel de Rooy 2023-10-12 05:52:44 UTC
Resuming here now. Please dont touch the patches.
Comment 122 Marcel de Rooy 2023-10-12 06:02:34 UTC
Applying: Bug 15504: (follow-up) Unit tests
error: sha1 information is lacking or useless (t/db_dependent/Koha/Patron.t).
error: could not build fake ancestor
Patch failed at 0001 Bug 15504: (follow-up) Unit tests
Comment 123 Marcel de Rooy 2023-10-12 06:04:13 UTC
I cant continue now. Please fix.
Comment 124 Martin Renvoize 2023-10-12 06:18:07 UTC
Created attachment 156898 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 125 Martin Renvoize 2023-10-12 06:18:12 UTC
Created attachment 156899 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 126 Martin Renvoize 2023-10-12 06:18:16 UTC
Created attachment 156900 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 127 Martin Renvoize 2023-10-12 06:18:21 UTC
Created attachment 156901 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 128 Martin Renvoize 2023-10-12 06:18:26 UTC
Created attachment 156902 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 129 Martin Renvoize 2023-10-12 06:18:30 UTC
Created attachment 156903 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 130 Martin Renvoize 2023-10-12 06:18:35 UTC
Created attachment 156904 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 131 Martin Renvoize 2023-10-12 06:18:40 UTC
Created attachment 156905 [details] [review]
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find
Comment 132 Martin Renvoize 2023-10-12 06:18:45 UTC
Created attachment 156906 [details] [review]
Bug 15504: (follow-up) Add Triggers mock to unit tests
Comment 133 Martin Renvoize 2023-10-12 06:18:49 UTC
Created attachment 156907 [details] [review]
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test
Comment 134 Martin Renvoize 2023-10-12 06:18:54 UTC
Created attachment 156908 [details] [review]
Bug 15504: (follow-up) Fix is_active test

Bug 33245 introduced a new is_active patron method and the corresponding
tests included calls to the method we depreacte in this patchset
Comment 135 Martin Renvoize 2023-10-12 06:18:58 UTC
Created attachment 156909 [details] [review]
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity

This patch updates the database update to set
'check_out,connection,login' as the activity triggers should we find at
upgrade time that TrackLastPatronActivity is set to true.

This retains the existing functionality.
Comment 136 Martin Renvoize 2023-10-12 06:19:03 UTC
Created attachment 156910 [details] [review]
Bug 15504: (follow-up) Rename cache key

I agree, it's better to use borrowernumber for the cache key and I also
take the oportunity to rename it to more clearly reflect the function
(it's no longer 'login' only, it's now 'activity')
Comment 137 Martin Renvoize 2023-10-12 06:19:07 UTC
Created attachment 156911 [details] [review]
Bug 15504: (QA follow-up) Terminology

This just updates 'Login to OPAC' to 'Login'.. we should squash this
Comment 138 Martin Renvoize 2023-10-12 06:19:12 UTC
Created attachment 156912 [details] [review]
Bug 15504: (follow-up) Update terminology on cleanborrowers

This patch updates the terminology on the cleanborrowers form to reflect
that 'activity' and not just 'connectivity' is tracked by
TrackLastPatronActivity.
Comment 139 Martin Renvoize 2023-10-12 06:19:17 UTC
Created attachment 156913 [details] [review]
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons

This patch adds clarifications to the last_seen option in delete_patrons
to clarify that TrackLastPatronActivity and
TrackLastPatronActivityTriggers system preferences act as a couplet.
Comment 140 Martin Renvoize 2023-10-12 06:19:22 UTC
Created attachment 156914 [details] [review]
Bug 15504: Remove PatronLastActivity preference

This patch removes the older PatronLastActivity preference opting to
replace it by the new PatronLastActivityTriggers preference. The feature
can now be disabled by simply not selecting any triggers
Comment 141 Marcel de Rooy 2023-10-12 06:25:08 UTC
Coming back here. Just hang on a bit.
Comment 142 Marcel de Rooy 2023-10-12 07:28:06 UTC
Created attachment 156915 [details] [review]
Bug 15504: (QA follow-up) Fix dbrev

Resolve:
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Unknown column 'check_out' in 'field list' at /usr/share/koha/C4/Installer.pm line 741

Test plan:
Remove new pref from db.
Run dbrev again.
Check results.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 143 Marcel de Rooy 2023-10-12 07:53:52 UTC
We also need 35041 to fix a current failure on Koha/Patron.t. Submitted now.
Comment 144 Marcel de Rooy 2023-10-12 09:09:04 UTC
 FAIL   C4/Auth.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

perl -MPod::Coverage=C4::Auth -e1
C4::Auth has a Pod::Coverage rating of 0.705882352941177
The following are uncovered: checkpw, checkpw_hash, checkpw_internal, getborrowernumber, safe_exit

So no problems from this patch set. Ignoring this warn from qa tools here. We just removed a sub here.
Comment 145 Marcel de Rooy 2023-10-12 09:13:29 UTC
Created attachment 156917 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 146 Marcel de Rooy 2023-10-12 09:13:33 UTC
Created attachment 156918 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 147 Marcel de Rooy 2023-10-12 09:13:38 UTC
Created attachment 156919 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 148 Marcel de Rooy 2023-10-12 09:13:43 UTC
Created attachment 156920 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 149 Marcel de Rooy 2023-10-12 09:13:47 UTC
Created attachment 156921 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 150 Marcel de Rooy 2023-10-12 09:13:52 UTC
Created attachment 156922 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 151 Marcel de Rooy 2023-10-12 09:13:56 UTC
Created attachment 156923 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 152 Marcel de Rooy 2023-10-12 09:14:01 UTC
Created attachment 156924 [details] [review]
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 153 Marcel de Rooy 2023-10-12 09:14:05 UTC
Created attachment 156925 [details] [review]
Bug 15504: (follow-up) Add Triggers mock to unit tests

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 154 Marcel de Rooy 2023-10-12 09:14:10 UTC
Created attachment 156926 [details] [review]
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 155 Marcel de Rooy 2023-10-12 09:14:14 UTC
Created attachment 156927 [details] [review]
Bug 15504: (follow-up) Fix is_active test

Bug 33245 introduced a new is_active patron method and the corresponding
tests included calls to the method we depreacte in this patchset

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 156 Marcel de Rooy 2023-10-12 09:14:19 UTC
Created attachment 156928 [details] [review]
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity

This patch updates the database update to set
'check_out,connection,login' as the activity triggers should we find at
upgrade time that TrackLastPatronActivity is set to true.

This retains the existing functionality.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 157 Marcel de Rooy 2023-10-12 09:14:24 UTC
Created attachment 156929 [details] [review]
Bug 15504: (follow-up) Rename cache key

I agree, it's better to use borrowernumber for the cache key and I also
take the oportunity to rename it to more clearly reflect the function
(it's no longer 'login' only, it's now 'activity')

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 158 Marcel de Rooy 2023-10-12 09:14:28 UTC
Created attachment 156930 [details] [review]
Bug 15504: (QA follow-up) Terminology

This just updates 'Login to OPAC' to 'Login'.. we should squash this

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 159 Marcel de Rooy 2023-10-12 09:14:32 UTC
Created attachment 156931 [details] [review]
Bug 15504: (follow-up) Update terminology on cleanborrowers

This patch updates the terminology on the cleanborrowers form to reflect
that 'activity' and not just 'connectivity' is tracked by
TrackLastPatronActivity.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 160 Marcel de Rooy 2023-10-12 09:14:37 UTC
Created attachment 156932 [details] [review]
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons

This patch adds clarifications to the last_seen option in delete_patrons
to clarify that TrackLastPatronActivity and
TrackLastPatronActivityTriggers system preferences act as a couplet.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 161 Marcel de Rooy 2023-10-12 09:14:41 UTC
Created attachment 156933 [details] [review]
Bug 15504: Remove PatronLastActivity preference

This patch removes the older PatronLastActivity preference opting to
replace it by the new PatronLastActivityTriggers preference. The feature
can now be disabled by simply not selecting any triggers

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 162 Marcel de Rooy 2023-10-12 09:14:46 UTC
Created attachment 156934 [details] [review]
Bug 15504: (QA follow-up) Fix dbrev

Resolve:
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Unknown column 'check_out' in 'field list' at /usr/share/koha/C4/Installer.pm line 741

Test plan:
Remove new pref from db.
Run dbrev again.
Check results.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 163 Marcel de Rooy 2023-10-12 09:14:50 UTC
Created attachment 156935 [details] [review]
Bug 15504: (QA follow-up) Final changes

[1] Comment in Patron->is_active sharpened.
[2] Patron->update_lastseen
      return if $cached && $cached eq $now->ymd;
    Should be
      return $self if $cached && $cached eq $now->ymd;
    Consistent return value.
[3] Typo if there are not TrackLastPatronActivityTriggers enabled
[4] Typo TrackLastPatronActivityTriggers must be contain
[5] Typo patron is does .. option => performs an action
[6] Tidy on dbrev to fix
      The file is less tidy than before (bad/messy lines before: 0, now: 4)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 164 Marcel de Rooy 2023-10-12 09:16:00 UTC
Looks good to me now.

Last commment. Just noticing here. Will open a new bug.
t/db_dependent/Members.t
$dbh->do(q|UPDATE borrowers SET lastseen=NULL|);
Not from this patch set, but this is bad practice. We should change it (elsewhere).
Comment 165 Marcel de Rooy 2023-10-12 09:20:22 UTC
Note for RM: There are follow-up bugs opened to extend the feature. But imo this can go in as-is. There is commitment to work further.
Comment 166 Tomás Cohen Arazi 2023-10-20 20:26:55 UTC
(In reply to Marcel de Rooy from comment #165)
> Note for RM: There are follow-up bugs opened to extend the feature. But imo
> this can go in as-is. There is commitment to work further.

Thanks for this clarification.
Comment 167 Tomás Cohen Arazi 2023-10-20 20:33:08 UTC
Please rebase ASAP. Thanks!
Comment 168 Andrii Nugged 2023-10-22 07:22:41 UTC
Created attachment 157602 [details] [review]
Bug 15504: Add new syspref TrackLastPatronActivityTriggers

This patch adds a new syspref TrackLastPatronActivityTriggers to determine which actions should update borrowers.lastseen

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 169 Andrii Nugged 2023-10-22 07:22:47 UTC
Created attachment 157603 [details] [review]
Bug 15504: Update track_login_daily to accept triggers

This patch adds triggers to track_login_daily so that it only tracks activity when that trigger is active

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 170 Andrii Nugged 2023-10-22 07:22:52 UTC
Created attachment 157604 [details] [review]
Bug 15504: Add triggers to instances of track_login_daily

This patch adds a trigger to every instance of track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 171 Andrii Nugged 2023-10-22 07:22:57 UTC
Created attachment 157605 [details] [review]
Bug 15504: Remove update_lastseen method

This patch removes update_lastseen which has been replaced by a call to track_login_daily

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 172 Andrii Nugged 2023-10-22 07:23:02 UTC
Created attachment 157606 [details] [review]
Bug 15504: Add unit test

prove -v t/db_dependent/Auth.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 173 Andrii Nugged 2023-10-22 07:23:07 UTC
Created attachment 157607 [details] [review]
Bug 15504: (follow-up) Unit tests

This patch removes, moves and updates the existing unit tests for
track_login_daily and track_login to all sit under Koha::Patron and
update_lastseen.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 174 Andrii Nugged 2023-10-22 07:23:12 UTC
Created attachment 157608 [details] [review]
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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 175 Andrii Nugged 2023-10-22 07:23:17 UTC
Created attachment 157609 [details] [review]
Bug 15504: (follow-up) Use borrowernumber in Koha::Patrons->find

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 176 Andrii Nugged 2023-10-22 07:23:21 UTC
Created attachment 157610 [details] [review]
Bug 15504: (follow-up) Add Triggers mock to unit tests

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 177 Andrii Nugged 2023-10-22 07:23:26 UTC
Created attachment 157611 [details] [review]
Bug 15504: (follow-up) Tidy 'lastseen' section of unit test

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 178 Andrii Nugged 2023-10-22 07:23:31 UTC
Created attachment 157612 [details] [review]
Bug 15504: (follow-up) Fix is_active test

Bug 33245 introduced a new is_active patron method and the corresponding
tests included calls to the method we depreacte in this patchset

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 179 Andrii Nugged 2023-10-22 07:23:36 UTC
Created attachment 157613 [details] [review]
Bug 15504: (follow-up) Respect existing TrackLastPatronActivity

This patch updates the database update to set
'check_out,connection,login' as the activity triggers should we find at
upgrade time that TrackLastPatronActivity is set to true.

This retains the existing functionality.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 180 Andrii Nugged 2023-10-22 07:23:41 UTC
Created attachment 157614 [details] [review]
Bug 15504: (follow-up) Rename cache key

I agree, it's better to use borrowernumber for the cache key and I also
take the oportunity to rename it to more clearly reflect the function
(it's no longer 'login' only, it's now 'activity')

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 181 Andrii Nugged 2023-10-22 07:23:46 UTC
Created attachment 157615 [details] [review]
Bug 15504: (QA follow-up) Terminology

This just updates 'Login to OPAC' to 'Login'.. we should squash this

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 182 Andrii Nugged 2023-10-22 07:23:50 UTC
Created attachment 157616 [details] [review]
Bug 15504: (follow-up) Update terminology on cleanborrowers

This patch updates the terminology on the cleanborrowers form to reflect
that 'activity' and not just 'connectivity' is tracked by
TrackLastPatronActivity.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 183 Andrii Nugged 2023-10-22 07:23:55 UTC
Created attachment 157617 [details] [review]
Bug 15504: (follow-up) Clarify --last_seen option in delete_patrons

This patch adds clarifications to the last_seen option in delete_patrons
to clarify that TrackLastPatronActivity and
TrackLastPatronActivityTriggers system preferences act as a couplet.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 184 Andrii Nugged 2023-10-22 07:24:00 UTC
Created attachment 157618 [details] [review]
Bug 15504: Remove PatronLastActivity preference

This patch removes the older PatronLastActivity preference opting to
replace it by the new PatronLastActivityTriggers preference. The feature
can now be disabled by simply not selecting any triggers

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 185 Andrii Nugged 2023-10-22 07:24:05 UTC
Created attachment 157619 [details] [review]
Bug 15504: (QA follow-up) Fix dbrev

Resolve:
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Unknown column 'check_out' in 'field list' at /usr/share/koha/C4/Installer.pm line 741

Test plan:
Remove new pref from db.
Run dbrev again.
Check results.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 186 Andrii Nugged 2023-10-22 07:24:10 UTC
Created attachment 157620 [details] [review]
Bug 15504: (QA follow-up) Final changes

[1] Comment in Patron->is_active sharpened.
[2] Patron->update_lastseen
      return if $cached && $cached eq $now->ymd;
    Should be
      return $self if $cached && $cached eq $now->ymd;
    Consistent return value.
[3] Typo if there are not TrackLastPatronActivityTriggers enabled
[4] Typo TrackLastPatronActivityTriggers must be contain
[5] Typo patron is does .. option => performs an action
[6] Tidy on dbrev to fix
      The file is less tidy than before (bad/messy lines before: 0, now: 4)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 187 Andrii Nugged 2023-10-22 07:24:54 UTC
I did the rebase. But which status we should put for the ticket?
Comment 188 Andrii Nugged 2023-10-22 07:33:46 UTC
btw, that atomic update quoting is very inconsistent:

        my ($tracklastactivity) = $dbh->selectrow_array(
            q|
            SELECT value FROM systempreferences WHERE variable='TrackLastPatronActivity';
        |
        );

vs

        $dbh->do(
            qq{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('TrackLastPatronActivityTriggers',?,NULL,'If set, the field borrowers.lastseen will be updated every time a patron performs a selected action','multiple') },
            undef, $triggers,
        );

vs

        $dbh->do(
            q{
            DELETE FROM systempreferences WHERE variable='TrackLastPatronActivity'
        }
        );

why so?
Comment 189 Victor Grousset/tuxayo 2023-10-22 20:32:30 UTC
Created attachment 157624 [details] [review]
Bug 15504: (QA follow-up): more consistent quotes in atomicupdate
Comment 190 Victor Grousset/tuxayo 2023-10-22 20:34:30 UTC
(In reply to Andrii Nugged from comment #187)
> btw, that atomic update quoting is very inconsistent:

It's just between 1st and 3rd case. The 2nd uses interpolation so needs the double-q IIUC

Is there even a recommendation for a default choice for the others?
The examples at
https://wiki.koha-community.org/wiki/Database_updates#How_to_write_an_atomicupdate_file
use q{ so let's use that.

updatedatabase tested.

----



> I did the rebase. But which status we should put for the ticket?

Thanks for the rebase :) It was passed QA before. So passed QA, unless the rebase showed that other changes could interfere with this and require retesting. **How was it?**
Comment 191 Tomás Cohen Arazi 2023-10-24 13:06:44 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 192 Cab Vinton 2023-10-24 13:09:14 UTC
Super-excited for this. Thanks to everyone for all their contributions & work to get this far!
Comment 193 Andrii Nugged 2023-10-24 19:41:52 UTC
(In reply to Tomás Cohen Arazi from comment #191)
> Pushed to master for 23.11.

Good! /me taking this to our preprod pilot, and soon to pre-release prod pilot. So if something pop-ups, we'll feeback quickly :P

\o]
Comment 194 Fridolin Somers 2023-10-26 06:49:29 UTC
Enhancement not pushed to 23.05.x
Comment 195 David Nind 2023-11-01 20:02:07 UTC
I've updated the release notes, as this how the enhancement worked was changed between sign off and being pushed.

Feel free to correct any mistakes I've made or make it better!
Comment 196 David Nind 2023-11-01 21:44:59 UTC
I renamed the activities in the release notes to match that used on bug 35221, and I also added the bug number for the activities added by other bugs (holds and article requests).
Comment 197 Marcel de Rooy 2023-11-02 13:30:13 UTC
See 35231. OPAC is blocked when logging out.

Did we miss an Auth.pm test here? :)