Bug 17621 - API: Should the 'updated_on' Patron field really be a datetime?
Summary: API: Should the 'updated_on' Patron field really be a datetime?
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 17607
Blocks: 17620
  Show dependency treegraph
 
Reported: 2016-11-14 06:38 UTC by Martin Renvoize
Modified: 2017-01-09 07:00 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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.