Bug 16276

Summary: When automatically deleting expired borrowers, make sure they didn't log in recently
Product: Koha Reporter: Nicolas Legrand <nicolas.legrand>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: colin.campbell, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, kyle, lisettepalouse+koha, lucas, m.de.rooy, martin.renvoize, mtj, nengard, nick
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15504
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
RMNOTE - Remove from release notes, Correction to update statement, feature actually added in 16.11.
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17607, 17713, 17820, 18625, 18821    
Attachments: Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: [DO_NOT_PUSH ] schema changes
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: [DO_NOT_PUSH ] schema changes
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: [QA Follow-up] Number of tests corrected
Bug 16276: [QA Follow-up] Only track when pref is enabled
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: [QA Follow-up] Number of tests corrected
Bug 16276: [QA Follow-up] Only track when pref is enabled
Bug 16276: Update borrowers.lastseen when a patron is active
Bug 16276: Make delete_patrons.pl deal with --last_seen
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account
Bug 16276: Make the batch patron deletion tool deal with last_seen
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen
Bug 16276: [QA Follow-up] Number of tests corrected
Bug 16276: [QA Follow-up] Only track when pref is enabled

Description Nicolas Legrand 2016-04-15 13:32:35 UTC
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.

Whith this parameter we would like to be able to use it from misc/cronjobs/delete_patrons.pl and also with the "Clean Patron Records" intranet page.
Comment 1 Jonathan Druart 2016-04-25 09:10:14 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-04-25 09:10:19 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-04-25 09:10:22 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-04-25 09:10:26 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-04-25 09:10:30 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-04-25 09:10:34 UTC Comment hidden (obsolete)
Comment 7 Nicolas Legrand 2016-05-04 10:12:08 UTC
Does not apply, there's a conflict on koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref. I'm not sure what's the righte move to do? Fix the conflict and if everything's OK sign?
Comment 8 Jonathan Druart 2016-05-04 14:51:45 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-05-04 14:53:18 UTC
(In reply to Nicolas Legrand from comment #7)
> Does not apply, there's a conflict on
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref. I'm
> not sure what's the righte move to do? Fix the conflict and if everything's
> OK sign?

The right way is to change the status to 'does not apply' and let the developer rebase the patches. However, if the fix is easy, you can fix it and sign it off.
Comment 10 Nicolas Legrand 2016-05-11 10:27:25 UTC Comment hidden (obsolete)
Comment 11 Nicolas Legrand 2016-05-11 10:27:30 UTC Comment hidden (obsolete)
Comment 12 Nicolas Legrand 2016-05-11 10:27:34 UTC Comment hidden (obsolete)
Comment 13 Nicolas Legrand 2016-05-11 10:27:38 UTC Comment hidden (obsolete)
Comment 14 Nicolas Legrand 2016-05-11 10:27:43 UTC Comment hidden (obsolete)
Comment 15 Nicolas Legrand 2016-05-11 10:27:47 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2016-08-19 07:39:05 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-08-19 07:39:11 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2016-08-19 07:39:17 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-08-19 07:39:20 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2016-08-19 07:39:26 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2016-08-19 07:40:25 UTC
Last patch set does not include the DBIC schema changes.
Comment 22 Marcel de Rooy 2016-08-19 11:04:52 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2016-08-19 11:04:57 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2016-08-19 11:05:01 UTC Comment hidden (obsolete)
Comment 25 Marcel de Rooy 2016-08-19 11:05:06 UTC Comment hidden (obsolete)
Comment 26 Marcel de Rooy 2016-08-19 11:05:13 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2016-08-19 11:05:17 UTC Comment hidden (obsolete)
Comment 28 Marcel de Rooy 2016-08-19 11:05:21 UTC Comment hidden (obsolete)
Comment 29 Marcel de Rooy 2016-08-19 11:10:41 UTC
Oops forgot some signoff lines..Hang on
Comment 30 Marcel de Rooy 2016-08-19 11:11:45 UTC Comment hidden (obsolete)
Comment 31 Marcel de Rooy 2016-08-19 11:11:49 UTC Comment hidden (obsolete)
Comment 32 Marcel de Rooy 2016-08-19 11:11:53 UTC Comment hidden (obsolete)
Comment 33 Marcel de Rooy 2016-08-19 11:11:57 UTC Comment hidden (obsolete)
Comment 34 Marcel de Rooy 2016-08-19 11:12:01 UTC Comment hidden (obsolete)
Comment 35 Marcel de Rooy 2016-08-19 11:12:05 UTC Comment hidden (obsolete)
Comment 36 Marcel de Rooy 2016-08-19 11:12:09 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2016-09-12 15:07:20 UTC
Created attachment 55495 [details] [review]
Bug 16276: Update borrowers.lastseen when a patron is active

In order to add the ability to delete patrons who have been inactive for
more than a given time, we need to track down the last time they were
active.
To do that, we need a new DB column in the borrowers table (lastseen).
Note that the borrowers.lastseen column will not be initialised for existing
installations (set to NULL) so inactive existing patrons will never be
deleted.
A workaround would be to init them at the date when the new column will
be added.

Test plan:
0/ Set the new pref TrackLastPatronActivity on
1/ Log in as a patron (staff or OPAC)
2/ Check that the borrowers.lastseen field has been updated with the
current time

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

https://bugs.koha-community.org/show_bug.cgi?id=12276

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 38 Jonathan Druart 2016-09-12 15:07:24 UTC
Created attachment 55496 [details] [review]
Bug 16276: Make delete_patrons.pl deal with --last_seen

This patch modify the delete_patrons.pl cronjob to deal with the
last_seen option.

To test it, you just have to use the --last_seen option and pass a valid
date (iso format)

Example:
  perl misc/cronjobs/delete_patrons.pl --last_seen="1984-02-04" --confirm
will delete all the patrons who do not have been active since this date.

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

https://bugs.koha-community.org/show_bug.cgi?id=12276

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 39 Jonathan Druart 2016-09-12 15:07:28 UTC
Created attachment 55497 [details] [review]
Bug 16276: Change to GetBorrowersToExpunge to take last_seen into account

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
With small change: number of tests in t/db_dependent/Members.t

https://bugs.koha-community.org/show_bug.cgi?id=12276

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 40 Jonathan Druart 2016-09-12 15:07:32 UTC
Created attachment 55498 [details] [review]
Bug 16276: Make the batch patron deletion tool deal with last_seen

This patch adds the same change as the previous one to the batch patron
deletion tool.

If the pref TrackLastPatronActivity is enabled, the librarians will be
able to delete patrons who do not have been connected since a given
time.

Test plan:
Define a date for the "who have not been connected since" options and
confirm that it works as expected.

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

https://bugs.koha-community.org/show_bug.cgi?id=12276

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 41 Jonathan Druart 2016-09-12 15:07:36 UTC
Created attachment 55499 [details] [review]
Bug 16276: Add a new pref TrackLastPatronActivity and new column borrowers.lastseen

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

https://bugs.koha-community.org/show_bug.cgi?id=12276

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Jonathan Druart 2016-09-12 15:07:40 UTC
Created attachment 55500 [details] [review]
Bug 16276: [QA Follow-up] Number of tests corrected

Applies to t/db_dependent/Members.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 43 Jonathan Druart 2016-09-12 15:07:44 UTC
Created attachment 55501 [details] [review]
Bug 16276: [QA Follow-up] Only track when pref is enabled

Do not track when the pref has not been enabled.
This patch moves the conditional update in Auth.pm to Koha::Patron.
And adds a test for the new track_login method.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Kyle M Hall 2016-09-13 18:30:30 UTC
Pushed to master for 16.11, thanks Jonathan, Marcel!
Comment 45 Mason James 2016-11-30 11:22:16 UTC
(In reply to Kyle M Hall from comment #44)
> Pushed to master for 16.11, thanks Jonathan, Marcel!

enhancement, passing for 16.05
Comment 46 Colin Campbell 2017-05-18 13:41:48 UTC
New bug 18625 to update the field if 'last seen' is from a sip patron info request (used to validate patron for access to resources)
Comment 47 Jonathan Druart 2019-02-22 12:16:30 UTC
*** Bug 15504 has been marked as a duplicate of this bug. ***
Comment 48 Nick Clemens 2019-02-22 15:02:58 UTC
Followup pushed to fix the update feedback
http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=55332977bf4d70de9d78c324ad88b302d2174c10
Comment 49 Martin Renvoize 2019-02-26 09:37:17 UTC
Followup pushed to 18.11.x series too..
Comment 50 Lucas Gass 2019-03-08 02:22:14 UTC
skipping enhancement for 18.05.x