-
Notifications
You must be signed in to change notification settings - Fork 65
Fix CakePHP date(time) object values to be not destroyed but exported… #55
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
Conversation
|
Locally, the tests passed (no .000000 noise added). |
|
@savant How shall we proceed? |
|
Non-travis (windows, ubuntu, ...) it is always: |
|
It might be the default datetime format on travis is being set to ISO8601? Not sure how to force it to be one or the other... |
|
If we could fix the core issue about how objects are being returned in array form instead of keeping the DateTime object as a whole we could resolve this at the root :) |
|
Yeah I got it! 🎱 |
Fix CakePHP date(time) object values to be not destroyed but exported…
|
Now all you have to do is write more tests to increase our coverage ;) |
|
There is only one real edge case you could test: https://coveralls.io/builds/4792330/source?filename=src%2FView%2FCsvView.php#L136 |
|
The bom stuff was never tested. |
… again.
Resolves #46
The main problem is Hash::extract() and that it converts the date into an array upon lookup.
The isset($x[0]) check cannot work on an array that is
So this fixes it but for every other object it will most likely fail, Maybe the array transformation should not be happening for objects in Hash::extract()? Or there should be a to string cast instead.
Either way, this PR at least now solves our specific issue at hand.
I tested it in my sandbox:
http://sandbox3.dereuromark.de/sandbox/csv/pagination
Currently the datetime fields would be indeed just gone, but as soon as this is merged, I will update the examples and you can see that it works as expected.