Bug 17621

Summary: API: Should the 'updated_on' Patron field really be a datetime?
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Web servicesAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, lari.taskula, m.de.rooy, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17651
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 17607    
Bug Blocks: 17620    

Description Martin Renvoize (ashimema) 2016-11-14 06:38:15 UTC
Bug #17607 highlighted that all other date fields in the Patron response are simply 'date', whereas update_on is a date-time (without a timezone).

We should assess whether it's actually necessary for the update_on field to record the timestamp as opposed to just a date, and if it required we should ensure we record and return the timezone detail along with it.
Comment 1 Marcel de Rooy 2016-11-14 07:44:35 UTC
(In reply to Martin Renvoize from comment #0)
> Bug #17607 highlighted that all other date fields in the Patron response are
> simply 'date', whereas update_on is a date-time (without a timezone).
> 
> We should assess whether it's actually necessary for the update_on field to
> record the timestamp as opposed to just a date, and if it required we should
> ensure we record and return the timezone detail along with it.

We already have quite a number of datetime fields in Koha, but until now we apparently did not care much about the timezones. What would make this a different one?
Comment 2 Martin Renvoize (ashimema) 2016-11-14 09:04:03 UTC
The simple fact that it's a public facing api makes it different. We handle timezone internally on the server, but for api consumers if they're not made aware of the timezone then they cannot do such handling themselves ;)

It's also pretty standard practice to format a datetime to conform to the RFC's (and swagger allows for this). The RFC's include timezone as part of the string.

I am aware there's precedent for datetime and date level specificity in different fields.  I'm just not aware why this field in the Patrons table is different to the other date fields in the patrons object.  Namely, lastseen, dateenrolled, dateexpired, debarred.
Comment 3 Jonathan Druart 2016-11-17 09:25:06 UTC
It makes sense to me to have a datetime for this kind of info. You can easily imagine a script processing the patron modified the last X hours, instead of days.
Comment 4 Katrin Fischer 2017-01-07 23:44:11 UTC
(In reply to Jonathan Druart from comment #3)
> It makes sense to me to have a datetime for this kind of info. You can
> easily imagine a script processing the patron modified the last X hours,
> instead of days.

+1, I think it's also consistent with similar fields on other tables (items, issues, currency) and helps when trying to diagnose problems, say with a patron import script or similar.
Comment 5 Martin Renvoize (ashimema) 2017-01-09 06:59:17 UTC
OK, I think my point here was more about the api returnter_capitadiscoverytnr field than the database field itself.

a) If we're wanting to keep this as a datetime as opposed to a date without time then we really need to ensure we return a timezone along with it.
b) The field name is misleading and non-standard for this sort of thing, 'updated_on' suggests a 'date' not a datetime.. 'updated' is a less specific term which commonly denotes a timestamp as opposed to a date.

My point really is that the API should NOT simply reflect the database.. an API is meant to hide the data storage details and rather expose the 'objects' and 'methods' an api consumer can work with.  Understanding of implementation details should not be required or indeed cared about by an api consumer.
Comment 6 Martin Renvoize (ashimema) 2017-01-09 07:00:52 UTC
Man I hate not being able to edit comments in bugzilla!

s/returnter_capitadiscoverytnr/return/g

No idea how that crap got into that string.