Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Datetime format on export/API #403

Closed
PhilippGrashoff opened this issue Mar 28, 2019 · 5 comments · May be fixed by atk4/api#18
Closed

Datetime format on export/API #403

PhilippGrashoff opened this issue Mar 28, 2019 · 5 comments · May be fixed by atk4/api#18
Assignees
@DarkSide666
Labels
waitingForReaction

Comments

@PhilippGrashoff
Copy link
Contributor

Hi,
as just discussed in the weekly meeting, the ATK API shouldn't export Date, DateTime and Time fields as serialized \DateTime objects, but as strings like (php spreaking):
Datetime: ISO 8601
Date: YYYY-mm-dd
Time: HH:ii:ss

The question open is: Should this be the case on API level only, or deeper in ATK like in export() function itself?
@PhilippGrashoff
Copy link
Contributor Author

Related: #394

@PhilippGrashoff
Copy link
Contributor Author

Regarding API, changing Model->export() wouldnt be enough, as GETting a single record, POST and PATCH use Model->get(), (without param), which creates a one-dimensional array.

@DarkSide666 DarkSide666 self-assigned this Apr 10, 2019
@PhilippGrashoff
Copy link
Contributor Author

HI there,
as there is the new Persistence ArrayOfStrings now, maybe the simplest way for API is to run all values through ArrayOfStrings typecast, which should take good care of at least all date related fields. What do you think?

@DarkSide666
Copy link
Member

DarkSide666 commented May 20, 2019 •
edited
Loading

Yes probably, but I think that's a bit to much work to involve another persistence.

I think it's not yet documented, but now we have this:

  • Model->getField() - useful method to return Field objects. It's better to use this instead of getElement(). It's not related to this issue, but I just wanted to mention that :)
  • Field->toString() - returns field value as string no matter what type field is. You can also pass value parameter to this method and get it typecasted to string.
  • Model->export($fields, $key_field, FALSE) - set 3rd parameter to false and export will not typecast values. This is useful to see what values you're actually storing in database for example. So API repo just need to pass this 3rd parameter === false to export method and job's done i hope.

PhilippGrashoff added a commit to PhilippGrashoff/api that referenced this issue May 20, 2019
@PhilippGrashoff
Fix data#403
2d85f3e 
This is supposed as proposal to fix atk4/data#403 going with the solution @DarkSide666 proposed.

This change routs all GET requests (single record and multiple record as well) through exportModel() function, with uses Model->export() while typecasting set to false. This way, date, time and datetime fields are not cast into PHP \DateTime objects, but the values are returned as stored in Persistence.

The solution posted here works for the mentioned date/time fields, however I see some problems:
1) what about fields that should get casting? In the end, API should return usable values. E.g about type money? API should return them as formatted to display 2 digits after the dot, shouldnt it? Is this still done?
2) getting a single record should throw an Exception when the requested record is not found. For that, it has to be loaded(). export() in exportModel() loads again, causing an unneccessary DB request.
@PhilippGrashoff PhilippGrashoff mentioned this issue May 20, 2019
Open
@mvorisek
Copy link
Member

@PhilippGrashoff please check if fixed, I belive if you export as DB/unmanaged values, all values are casted correctly

@mvorisek mvorisek changed the title Date, DateTime and Time format on export/API Datetime format on export/API Aug 25, 2021
@mvorisek mvorisek closed this as completed Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DarkSide666 DarkSide666

Labels
waitingForReaction
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix data#403: return date, time and datetime as scalar value PhilippGrashoff/api
3 participants
@DarkSide666 @mvorisek @PhilippGrashoff