Bug 24650 - API returned timestamps are incompatible with existing helpers
Summary: API returned timestamps are incompatible with existing helpers
Status: RESOLVED DUPLICATE of bug 24455
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-13 09:13 UTC by Andrew Isherwood
Modified: 2022-11-07 10:41 UTC (History)
3 users (show)

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


Attachments
Bug 24650: Convert ISO datetimes to syspref dates (1.99 KB, patch)
2020-02-13 12:34 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 24650: Convert ISO datetimes to syspref dates (2.04 KB, patch)
2020-02-15 10:58 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2020-02-13 09:13:05 UTC
API endpoints that return system maintained timestamps (specified as datetime in the DB schema and specified as 'date-time' in the endpoint definition) return the timestamps as a form of RFC3339 timestamps. For example, from the GET /patrons/{patron_id} response:

"last_seen": "2020-02-13T08:47:29+00:00"

This is good as it's in a standard form. However, all the existing handlers in koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc that are used to transform a received timestamp into the instance's defined format are expecting timestamps in the format: (see DateTime_from_syspref)

[datestring] [hours:minutes]

Thereby making it impossible to display a returned timestamp to the user in their chosen format.

We either need to create additional helpers or modify the API returning of ISO formatted timestamps to something the helpers can cope with.
Comment 1 Andrew Isherwood 2020-02-13 09:14:36 UTC
Actually, it's not exclusively "system maintained" timestamps, it's anything defined as datetime in the DB
Comment 2 Andrew Isherwood 2020-02-13 11:55:18 UTC
Looking into this further it seems I have *completely* misunderstood the purpose of the *_from_syspref functions in calendar.inc. It would appear they accept a date / datetime in the format specified in the syspref, and return a Datetime. Much the same as Date.parse() will do.

In which case, it would seem we're missing functions to take an ISO formatted datetime and return it in the format specified in the syspref.
Comment 3 Andrew Isherwood 2020-02-13 12:34:45 UTC
Created attachment 98818 [details] [review]
Bug 24650: Convert ISO datetimes to syspref dates

This patch adds a function 'ISO_to_syspref' that accepts an ISO8601 / RFC3339 formatted timestamp, as returned
from the API for datetime DB columns, and returns a string formatted
according to the 'dateformat' syspref, optionally including the time

Test plan:
- Apply the patch
- Load a page that uses calendar.inc, for example a patron's loan list
in the staff interface
- In the JS console, type the following and notice the printed date is
in the format according to the syspref:

var dt = new Date;
console.log(ISO_to_syspref(dt.toISOString()));

- In the JS console, type the following and notice the printed date is
in the format according to the syspref, and includes the time:

console.log(ISO_to_syspref(dt.toISOString(), true));
Comment 4 David Nind 2020-02-15 10:58:39 UTC
Created attachment 99061 [details] [review]
Bug 24650: Convert ISO datetimes to syspref dates

This patch adds a function 'ISO_to_syspref' that accepts an ISO8601 / RFC3339 formatted timestamp, as returned
from the API for datetime DB columns, and returns a string formatted
according to the 'dateformat' syspref, optionally including the time

Test plan:
- Apply the patch
- Load a page that uses calendar.inc, for example a patron's loan list
in the staff interface
- In the JS console, type the following and notice the printed date is
in the format according to the syspref:

var dt = new Date;
console.log(ISO_to_syspref(dt.toISOString()));

- In the JS console, type the following and notice the printed date is
in the format according to the syspref, and includes the time:

console.log(ISO_to_syspref(dt.toISOString(), true));

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Katrin Fischer 2020-02-17 21:25:22 UTC
Andrew, please don't forget to fill out the Assignee when you start working on a bug!
Comment 6 Jonathan Druart 2020-02-18 15:53:58 UTC
This is blocked until I understand the need and the link with bug 24455.

Andrew, what is your real-life use case?
Comment 7 Andrew Isherwood 2020-02-19 08:33:25 UTC
Receiving ISO timestamps back from the API and needing to display them in the UI according to the format specified in the syspref
Comment 8 Andrew Isherwood 2020-02-19 08:34:51 UTC
Looking at bug 24455, it looks like it may make this redundant
Comment 9 Andrew Isherwood 2020-03-02 09:00:39 UTC
Setting this to "In discussion" because bug 24608 depends on it and having it "Blocked" prevents the QA of 24608. Although this bug is likely to be made redundant by bug 24455, that bug is still awaiting sign off, so in the meantime 24608 cannot be QA'ed.
Comment 10 Andrew Isherwood 2020-03-02 09:02:59 UTC
Actually, that doesn't seem to help as git bz still doesn't pick it up as a dependency. I don't want to set it back to "Needs signoff", so I'll add a comment to bug 24608 noting that this bug needs to be manually applied.
Comment 11 Katrin Fischer 2020-03-02 10:05:05 UTC
The best here would be to add to the discussion to push forward a solution (unblocking the depenencies).
Comment 12 Andrew Isherwood 2020-03-02 10:50:00 UTC
Done. I have tested and signed off bug 24455, I've also switched my dependent bug 24608 to use that. I'll mark this as a duplicate.

*** This bug has been marked as a duplicate of bug 24455 ***