Rework based on #18 Normalize fields #25
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.
Codecov Report
@@ Coverage Diff @@
## develop #25 +/- ##
=============================================
+ Coverage 81.16% 82.63% +1.46%
- Complexity 76 81 +5
=============================================
Files 1 1
Lines 154 167 +13
=============================================
+ Hits 125 138 +13
Misses 29 29
Continue to review full report at Codecov.
|
| $data = []; | ||
| foreach ($allowed_fields as $fieldName) { | ||
| $field = $m->getField($fieldName); | ||
| $data[$field->actual ?? $fieldName] = $field->toString(); |
There was a problem hiding this comment.
I dont think using actual is sensible. Think of this example Model:
$this->addField('name');
$join = $this->join('some_table');
$join->addField('other_name', ['actual' => 'name']);
That would lead to replacing the name of the model's name field with the name field from the joined table, which was sensibly declared in the model as other_name
There was a problem hiding this comment.
In the demo there was a Model with few fields with actual defined and even if i don't use it i try to fix the right field name to return back in api response
There was a problem hiding this comment.
Thinking of it this way:
A Model can have each field name only once. So no trouble using this as array key.
When using actual property, we can have duplicate array keys, which leads to false/incomplete data passed via API.
I think actual is meant for Persistences only.
As already done in #18 by @PhilippGrashoff i continue from there and i add some test to all the cases.