-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Fix data#403: return date, time and datetime as scalar value #18
base: develop
Are you sure you want to change the base?
Conversation
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.
Hi, |
Now this is a sensible first proposal. I wonder about two things:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apologies for delay in approval.
$allowed_fields = $this->getAllowedFields($model, 'read'); | ||
$data = []; | ||
// get all field-elements | ||
foreach ($model->elements as $field => $f) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this is wrong now. Should use $model->getFields()
and that way get rid of instanceof \atk4\data\Field
condition and as result this change can be made simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also tests fail.
This is supposed as proposal to fix atk4/data#403 going with the solution @DarkSide666 proposed.