Summary: | API: Should the 'updated_on' Patron field really be a datetime? | ||
---|---|---|---|
Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
Component: | Web services | Assignee: | 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
(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? 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. 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. (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. 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. 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. |