Bug 18625

Summary: Update borrower last seen from SIP
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: SIP2Assignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andreas.jonsson, benjamin.rokseth, colin.campbell, dominic.pichette, jdemuth, jonathan.druart, jonathan.field, jzairo, katrin.fischer, lisettepalouse+koha, m.de.rooy, margaret, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16276    
Bug Blocks: 15504    
Attachments: proposed patch
Bug 18625: Update lastseen in patron info request
Bug 18625: (QA follow-up) Simply update_lastseen
Bug 18625: Update lastseen in patron info request
Bug 18625: (QA follow-up) Simply update_lastseen
Bug 18625: QA followup - Unit tests
Added pod to update_lastseen method
correct typo in commit message
Bug 18625: QA followup - Unit tests
Bug 18625: Update lastseen in patron info request
Bug 18625: (QA follow-up) Simplify update_lastseen

Description Colin Campbell 2017-05-18 13:39:20 UTC
Bug 16276 added a syspref and field for last seen. In addition to the places there it should also be updated from SIP. The rationale is this in addition to the self check functions handled by sip an increasing number of packages are available which use sip to authenticate users as being valid library patrons to access various resources e.g. computer equipment, ebooks or services via a proxy. Some library patrons may only be using the library to access the internet (validated by sip) and never borrower items or sign in via the opac. The proposal is to update last seen if the borrower is a subject of a 63 (Patron Information request) from sip. We have a number of sites currently maintaining this in a patron attribute.
Comment 1 Colin Campbell 2017-05-18 15:28:31 UTC
Created attachment 63533 [details] [review]
proposed patch
Comment 2 Benjamin Rokseth 2017-05-26 08:12:14 UTC
Very nice. I totally agree with the proposal, as our patrons mainly operate self-serviced, and we currently have no option to register who's active or not.
Comment 3 Dominic Pichette 2017-09-29 20:02:07 UTC
is the patch only adding a field in preferences or is it using it somewhere?
Comment 4 Colin Campbell 2017-10-09 09:55:25 UTC
(In reply to Dominic Pichette from comment #3)
> is the patch only adding a field in preferences or is it using it somewhere?

This patch is not adding the field - See bug 16276 it is adding the recording of that info in the sip patron info request which was not included in that change
Comment 5 Marcel de Rooy 2017-10-11 12:50:29 UTC
Created attachment 67953 [details] [review]
Bug 18625: Update lastseen in patron info request

Many services use SIP patron req info to validate a patron against
the koha database. If recording lastseen sites will
also want to record that the user has accessed these
library facilities although they have not logged into koha

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that Patron Info request (63) updates lastseen when the pref has
been enabled.
Comment 6 Marcel de Rooy 2017-10-11 12:50:33 UTC
Created attachment 67954 [details] [review]
Bug 18625: (QA follow-up) Simply update_lastseen

No need to check the pref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2017-10-11 12:52:22 UTC
No idea if the QAer will request an addition test in SIP/Patron.t or Message.t ;)
Comment 8 Marcel de Rooy 2017-10-11 12:52:39 UTC
addition -> additional
Comment 9 Nick Clemens 2017-10-27 02:25:57 UTC
Created attachment 68650 [details] [review]
Bug 18625: Update lastseen in patron info request

Many services use SIP patron req info to validate a patron against
the koha database. If recording lastseen sites will
also want to record that the user has accessed these
library facilities although they have not logged into koha

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that Patron Info request (63) updates lastseen when the pref has
been enabled.
Comment 10 Nick Clemens 2017-10-27 02:26:01 UTC
Created attachment 68651 [details] [review]
Bug 18625: (QA follow-up) Simply update_lastseen

No need to check the pref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Nick Clemens 2017-10-27 02:26:04 UTC
Created attachment 68652 [details] [review]
Bug 18625: QA followup - Unit tests
Comment 12 Nick Clemens 2017-10-27 02:27:20 UTC
(In reply to Marcel de Rooy from comment #7)
> No idea if the QAer will request an addition test in SIP/Patron.t or
> Message.t ;)

Either that or he might write them :-)

Excellent idea, very useful for patrons using external services.
Comment 13 Jonathan Druart 2017-10-27 14:30:41 UTC
FAIL   C4/SIP/ILS/Patron.pm
  FAIL   pod coverage
    POD is missing for update_lastseen
Comment 14 Colin Campbell 2018-03-09 09:19:03 UTC
Created attachment 72557 [details] [review]
Added pod to update_lastseen method

Add a pod entry to update lastseen
Comment 15 Colin Campbell 2018-03-09 09:20:56 UTC
Created attachment 72558 [details] [review]
correct typo in commit message

s/Simply/Simplify/
Comment 16 Colin Campbell 2018-03-09 09:22:50 UTC
Amended patch to add the required pod reset status to resubmit for QA
Comment 17 Kyle M Hall 2018-04-10 11:21:35 UTC
Created attachment 73920 [details] [review]
Bug 18625: QA followup - Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2018-04-10 11:21:42 UTC
Created attachment 73921 [details] [review]
Bug 18625: Update lastseen in patron info request

Many services use SIP patron req info to validate a patron against
the koha database. If recording lastseen sites will
also want to record that the user has accessed these
library facilities although they have not logged into koha

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that Patron Info request (63) updates lastseen when the pref has
been enabled.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2018-04-10 11:21:45 UTC
Created attachment 73922 [details] [review]
Bug 18625: (QA follow-up) Simplify update_lastseen

No need to check the pref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Jonathan Druart 2018-04-11 19:54:05 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 21 Nick Clemens 2018-05-14 19:32:15 UTC
Enhancement, not backported for 17.11.x
Comment 22 Katrin Fischer 2018-05-15 09:49:33 UTC
Not sure if this should not be a bug fix instead of an enhancement?