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

[5.8] Custom pivot class causes updated_at not to be updated #29321

Closed
lk77 opened this issue Jul 29, 2019 · 14 comments · Fixed by #29970
Closed

[5.8] Custom pivot class causes updated_at not to be updated #29321

lk77 opened this issue Jul 29, 2019 · 14 comments · Fixed by #29970
Labels

Comments

@lk77
Copy link

lk77 commented Jul 29, 2019

  • Laravel Version: 5.8.29
  • PHP Version: 7.2.19
  • Database Driver & Version: mysql 5.7

Description:

Hello,

we have an issue when using a custom pivot class, with an additional pivot attribute. The updated_at field is not updated, while the value of the additional field is updated, when using sync() method on the belongsToMany relation. We tried to add $timestamps = true to the custom pivot class but it's not working.

Let's say for example, that we have a users table, a roles tables, and a users_has_roles pivot table with a additional comment field.

If we sync the relation without a call to using(), so without custom pivot class, it's working fine, the comment is updated, and the updated_at field too.

But if we add using() to our relation method, so with custom pivot class, it's not working fully,
the comment is updated, but the updated_at field is not.

Steps To Reproduce:

1 / clone https://github.com/lk77/pivot-using-update-bug
2 / configure .env and execute php artisan migrate --seed
3 / php artisan serve
4 / go to localhost:8000
5 / See that the two dates differs (one is calling using() the other is not)

thanks.

@staudenmeir
Copy link
Contributor

This was caused by #27571. /cc @ralphschindler

@meyer59
Copy link

meyer59 commented Jul 30, 2019

Any news about that issue ?

@ralphschindler
Copy link
Contributor

I am trying to write a test for this behavior. Standby.

@driesvints
Copy link
Member

PR was merged.

@staudenmeir
Copy link
Contributor

@driesvints Please reopen, the linked PR is an old one. This issue hasn't been fixed yet.

@lk77
Copy link
Author

lk77 commented Aug 1, 2019

Earlier, i was thinking that this pr (28416) may be related to my issue, and commented,
but since it's not the case, i clarified it on the pr and linked to this issue,
but it's not related.

@driesvints
Copy link
Member

@staudenmeir I meant this PR: #29362

@staudenmeir
Copy link
Contributor

@driesvints That's also a different issue ;-)

@driesvints
Copy link
Member

Okay lol

@driesvints driesvints reopened this Aug 1, 2019
@driesvints driesvints added the bug label Aug 1, 2019
@mpyw
Copy link
Contributor

mpyw commented Aug 1, 2019

@driesvints Sorry for confusing mention

@mpyw
Copy link
Contributor

mpyw commented Aug 1, 2019

Very ditry hack:

 $this->newPivot([
     $this->foreignPivotKey => $this->parent->{$this->parentKey},
     $this->relatedPivotKey => $this->parseId($id),
+    (new $this->using)->getCreatedAtColumn() => null,
 ], true)->fill($attributes)->save();

It forces AsPivot::hasTimestampAttributes() to return true.

@mpyw
Copy link
Contributor

mpyw commented Aug 1, 2019

Okay I got the better implementation, I'll submit PR

@mpyw
Copy link
Contributor

mpyw commented Aug 2, 2019

@lk77 Would you test it based on this PR branch?

@lk77
Copy link
Author

lk77 commented Aug 2, 2019

@mpyw i pulled your fork and updated my demo : https://github.com/lk77/pivot-using-update-bug
the issue is fixed :

"Fri Aug 02 2019 06:58:09 GMT+0000"
"Fri Aug 02 2019 06:58:09 GMT+0000"

the dates are equal.

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants