Description
Colin Campbell
2017-05-18 13:39:20 UTC
Created attachment 63533 [details] [review] proposed patch 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. is the patch only adding a field in preferences or is it using it somewhere? (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 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. 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> No idea if the QAer will request an addition test in SIP/Patron.t or Message.t ;) addition -> additional 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. 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> Created attachment 68652 [details] [review] Bug 18625: QA followup - Unit tests (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. FAIL C4/SIP/ILS/Patron.pm FAIL pod coverage POD is missing for update_lastseen Created attachment 72557 [details] [review] Added pod to update_lastseen method Add a pod entry to update lastseen Created attachment 72558 [details] [review] correct typo in commit message s/Simply/Simplify/ Amended patch to add the required pod reset status to resubmit for QA Created attachment 73920 [details] [review] Bug 18625: QA followup - Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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> 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> Pushed to master for 18.05, thanks to everybody involved! Enhancement, not backported for 17.11.x Not sure if this should not be a bug fix instead of an enhancement? |