Bug 28630 - ILSDI::AuthenticatePatron should set borrowers.lastseen
Summary: ILSDI::AuthenticatePatron should set borrowers.lastseen
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 15504
  Show dependency treegraph
 
Reported: 2021-06-25 08:47 UTC by Martin Renvoize
Modified: 2021-08-16 21:06 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.03,20.11.09


Attachments
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron (1.10 KB, patch)
2021-06-25 08:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Add unit tests (2.92 KB, patch)
2021-06-25 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron (1.10 KB, patch)
2021-06-28 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Add unit tests (2.92 KB, patch)
2021-06-28 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: (follow-up) Update wording of syspref (1.22 KB, patch)
2021-06-28 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron (1.17 KB, patch)
2021-06-29 13:07 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28630: Add unit tests (2.99 KB, patch)
2021-06-29 13:07 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28630: (follow-up) Update wording of syspref (1.28 KB, patch)
2021-06-29 13:07 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron (1.26 KB, patch)
2021-07-02 12:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28630: Add unit tests (3.08 KB, patch)
2021-07-02 12:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28630: (follow-up) Update wording of syspref (1.38 KB, patch)
2021-07-02 12:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron (1.10 KB, patch)
2021-08-04 08:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Add unit tests (3.03 KB, patch)
2021-08-04 08:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: (follow-up) Update wording of syspref (1.26 KB, patch)
2021-08-04 08:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28630: Improve date comparisons in tests (2.62 KB, patch)
2021-08-04 09:13 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-06-25 08:47:54 UTC
Currently, the ILSDI authentication route does not set the lastseen date for a borrower record. It should.
Comment 1 Martin Renvoize 2021-06-25 08:52:36 UTC
Created attachment 122421 [details] [review]
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron

This patch adds a call to track_login_daily for the AuthenticatePatron
method in ILSDI.

Many libraries use this ILSDI endpoint for electron book borrowing and
need to maintain the lastseen based on this type of lending.
Comment 2 Magnus Enger 2021-06-25 09:00:06 UTC
See also bug 15504, comment 28 and comment 29.
Comment 3 Martin Renvoize 2021-06-25 09:35:08 UTC
Created attachment 122422 [details] [review]
Bug 28630: Add unit tests
Comment 4 Andrew Fuerste-Henry 2021-06-25 11:26:58 UTC
In bug 28467 I just updated the working on TrackLastPatronActivity to accurately describe the current behavior. Please include an update there to tell people it now includes ILS-DI.

The comment Magnus links off to also mentions API actions. I agree we should also cover that with lastseen. I don't see a separate bug for that. Martin, are you up for folding that in here?
Comment 5 Martin Renvoize 2021-06-28 13:42:50 UTC
Created attachment 122476 [details] [review]
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron

This patch adds a call to track_login_daily for the AuthenticatePatron
method in ILSDI.

Many libraries use this ILSDI endpoint for electron book borrowing and
need to maintain the lastseen based on this type of lending.
Comment 6 Martin Renvoize 2021-06-28 13:42:54 UTC
Created attachment 122477 [details] [review]
Bug 28630: Add unit tests
Comment 7 Martin Renvoize 2021-06-28 13:42:57 UTC
Created attachment 122478 [details] [review]
Bug 28630: (follow-up) Update wording of syspref
Comment 8 Martin Renvoize 2021-06-28 13:46:49 UTC
(In reply to Andrew Fuerste-Henry from comment #4)
> In bug 28467 I just updated the working on TrackLastPatronActivity to
> accurately describe the current behavior. Please include an update there to
> tell people it now includes ILS-DI.

I've added a follow-up with the wording change, thanks for pointing it out :)

> The comment Magnus links off to also mentions API actions. I agree we should
> also cover that with lastseen. I don't see a separate bug for that. Martin,
> are you up for folding that in here?

Whilst I sort of agree with Magnus, there isn't really an 'IdentifyUser/AuthenticateUser' route in the API as yet and as such not a clear location to tie this too.. the REST API is still more closely tied to third part integrators acting on someone's behalf and doesn't really denote a user interaction which I feel lastseen is for.  So yeah.. I'd wait until such a time that OAuth code grant or similar is introduced which would more clearly denote an actual user login.
Comment 9 Andrew Fuerste-Henry 2021-06-29 13:07:52 UTC
Created attachment 122490 [details] [review]
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron

This patch adds a call to track_login_daily for the AuthenticatePatron
method in ILSDI.

Many libraries use this ILSDI endpoint for electron book borrowing and
need to maintain the lastseen based on this type of lending.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 10 Andrew Fuerste-Henry 2021-06-29 13:07:55 UTC
Created attachment 122491 [details] [review]
Bug 28630: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 11 Andrew Fuerste-Henry 2021-06-29 13:07:58 UTC
Created attachment 122492 [details] [review]
Bug 28630: (follow-up) Update wording of syspref

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 12 Andrew Fuerste-Henry 2021-06-29 13:09:04 UTC
Tested and signed off, but I should have thought sooner about how this was going to run into bug 28455, also signed off.
Comment 13 Marcel de Rooy 2021-07-02 12:08:14 UTC
Created attachment 122544 [details] [review]
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron

This patch adds a call to track_login_daily for the AuthenticatePatron
method in ILSDI.

Many libraries use this ILSDI endpoint for electron book borrowing and
need to maintain the lastseen based on this type of lending.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2021-07-02 12:08:18 UTC
Created attachment 122545 [details] [review]
Bug 28630: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2021-07-02 12:08:22 UTC
Created attachment 122546 [details] [review]
Bug 28630: (follow-up) Update wording of syspref

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Jonathan Druart 2021-08-04 07:57:02 UTC
There is a conflict on the last patch, Martin can you have a look please?
Should be easy for a native English speaker :)
Comment 17 Martin Renvoize 2021-08-04 08:09:57 UTC
Created attachment 123448 [details] [review]
Bug 28630: Add track_login_daily to ILSDI::AuthenticatePatron

This patch adds a call to track_login_daily for the AuthenticatePatron
method in ILSDI.

Many libraries use this ILSDI endpoint for electron book borrowing and
need to maintain the lastseen based on this type of lending.
Comment 18 Martin Renvoize 2021-08-04 08:10:01 UTC
Created attachment 123449 [details] [review]
Bug 28630: Add unit tests
Comment 19 Martin Renvoize 2021-08-04 08:10:05 UTC
Created attachment 123450 [details] [review]
Bug 28630: (follow-up) Update wording of syspref
Comment 20 Jonathan Druart 2021-08-04 09:13:35 UTC
Created attachment 123454 [details] [review]
Bug 28630: Improve date comparisons in tests
Comment 21 Jonathan Druart 2021-08-04 12:06:19 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 22 Kyle M Hall 2021-08-06 18:33:11 UTC
Pushed to 21.05.x for 21.05.03 by request.
Comment 23 Fridolin Somers 2021-08-11 20:13:48 UTC
Since Bug 28455 is in 20.11.x, I choose to backport this one also.

I skipped "Update wording of syspref", I does not apply because text is different. I this it is not very important.

Pushed to 20.11.x for 20.11.09
Comment 24 Victor Grousset/tuxayo 2021-08-16 21:06:59 UTC
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.