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

Use Model's $dateFormat for formatting data #409

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

sarahkemp
Copy link
Contributor

When the Model uses a different $dateFormat than Audit, Carbon throws an exception. Calling $model->asDateTime() allows the model's $dateFormat to be used.

I just started with Laravel Auditing, apologies if I'm misunderstanding this method.

When the Model uses a different $dateFormat than Audit, Carbon throws an exception.
@quetzyg
Copy link
Contributor

quetzyg commented Apr 17, 2018

Hi @sarahkemp

Thanks for your pull request. At first glance your change makes total sense. Can you just give me an actual example where the current code throws an exception?

@sarahkemp
Copy link
Contributor Author

A few factors have to come together to throw an exception.

  • I have two DB connections, one is local and one is not. The local is a regular MySQL and uses probably what you would expect as a date format. The remote DB uses 'Y-m-d H:i:s.u'. The audit table is in the MySQL database, the data I'm auditing is not.
  • My Eloquent model has $dateFormat = 'Y-m-d H:i:s.u'
  • I had an Audit for a field listed in my Eloquent model's $dates array
  • I tried to use $audit->getModified() after a date field had changed

$audit->getModified() uses getDataValue() to getFormattedValue() which then tries to cast to a date using $this->asDateTime(). However, when $this is an instance of Audit, it will use the $dateFormat for the Audit model instead of the Eloquent model that declared the data was a date. E.g.

class Foo extends Model implements Auditable {

    use \OwenIt\Auditing\Auditable;

    protected $dateFormat = 'Y-m-d H:i:s.u',
                     $dates = ['start_date'];

In this example, if the start_date is changed and audited, and I try to show the changes later with $audit->getModified(), the following exception is thrown:

(1/1) InvalidArgumentExceptionTrailing data
--
in Carbon.php (line 775)
at Carbon::createFromFormat('Y-m-d H:i:s', '2018-04-19 00:00:00.000000')in HasAttributes.php (line 709)
at Model->asDateTime('2018-04-19 00:00:00.000000')in Audit.php (line 145)
at Audit->getFormattedValue(object(Foo), 'start_date', '2018-04-19 00:00:00.000000')in Audit.php (line 169)
at Audit->getDataValue('new_start_date')in Audit.php (line 212)
at Audit->getModified()
...

I will understand if you feel this case is too specific to worry about, but I believe what I'm doing is within the bounds of documented Laravel configuration.

@quetzyg quetzyg merged commit 85be8db into owen-it:master Apr 19, 2018
@quetzyg quetzyg self-requested a review April 19, 2018 09:21
@quetzyg
Copy link
Contributor

quetzyg commented Apr 19, 2018

Sorry it took me a while, busy days.

Thanks for the contribution :)

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

Successfully merging this pull request may close these issues.

2 participants