Skip to content

Conversation

@freekmurze
Copy link
Contributor

With the changes in this PR you can replace this code...

$this->update(['attribute' => now()]);

... with ...

$this->touch('attribute');

Which feels a tad nicer.

I didn't find any test around the normal behaviour of touch so I didn't add any.

If you want to accept this PR, but want tests, point me where these should be created.

public function touch()
public function touch($attribute = null)
{
if ($attribute) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would it happen if a given model does not use timestamp? - wouldn't the guard ! $this->usesTimestamps() be necessary for this use case too?

Copy link
Member

@crynobone crynobone Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be covered by Line 30. This line covers $model->touch('custom_attribute'); and should affect usesTimestamps().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usesTimestamps() only tells you if the Model is using the standard updated_at and created_at, so it should not be checked when $attribute is passed to the touch method.

It is valid to disable the updated/created timestamps (via $timestamps = false) but still use your own custom timestamp column for some other use case, which is when the $attribute argument would be used.

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.

6 participants