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.

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

Date format hard-coded into Attachment #155

Open
hackel opened this issue Apr 20, 2016 · 3 comments
Open

Date format hard-coded into Attachment #155

hackel opened this issue Apr 20, 2016 · 3 comments

Comments

@hackel
Copy link

hackel commented Apr 20, 2016

https://github.com/CodeSleeve/stapler/blob/master/src/Attachment.php#L134

When using a different date format (such as ISO dates for MongoDB), this causes a frustrating InvalidArgumentException: "Unexpected data found. Data missing"

For Laravel, the correct date format is stored in the model instance's protected $dateFormat property, which unfortunately can only be retrieved by the protected getDateFormat method. The trick is to access the method statically, which gets handled by the __callStatic magic method:

$this->instanceWrite('updated_at', date(call_user_func([get_class($this->instance), 'getDateFormat'])));

I'm not sure how this could be best generalized to a non-Laravel-specific solution, I guess adding the format as a config property? In the meantime, I had to override the setAttribute method on my model class to detect and correctly parse the SQL date format.

@tabennett
Copy link
Contributor

Can you show me an example of what you're talking about here? It would really help me understand what the issue so that I can get this one addressed too.

@hackel
Copy link
Author

hackel commented Aug 10, 2016

The issue is this line:

$this->instanceWrite('updated_at', date('Y-m-d H:i:s'));

That doesn't use whatever custom date format is defined in the model. So the model tries to set the updated_at attribute by first converting it to a DateTime in Model::asDateTime($value) by calling Carbon::createFromFormat($this->getDateFormat(), $value).

In my case, getDateFormat() returns 'Y-m-d\TH:i:s.uP', so Carbon isn't able to correctly parse the SQL-style date, generating the error. Hopefully this makes sense!

@tabennett
Copy link
Contributor

I'm going to update this so that the the Attachment class is just setting that column as a simple DateTime instance like this:

$this->instanceWrite('updated_at', new DateTime);

This should default the formatting of that column to whatever the default format is on the DB's storage connection, which allows Stapler to be more format agnostic and helps to keep control of that kind of thing in the hands of the developer. In your case, using this change you should just be able to add that field to your the protected $dates property of your model to have it passed through the model's fromDateTime() method and formatted correctly. If you see any reason why this might not work please let me know. I'm going to be writing some tests for this and pushing it out in the next release.

tabennett added a commit that referenced this issue Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants