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

Recent change to refresh() and fresh() methods caused BC breakages #34954

Closed
adamthehutt opened this issue Oct 24, 2020 · 3 comments
Closed

Recent change to refresh() and fresh() methods caused BC breakages #34954

adamthehutt opened this issue Oct 24, 2020 · 3 comments

Comments

@adamthehutt
Copy link
Contributor

  • Laravel Version: 8.11.2
  • PHP Version: 7.4.5
  • Database Driver & Version: MySQL 5.7

Description:

I'm hoping it might be possible to revisit the changes in PR #34836. With those changes, Laravel now invokes the setKeysForSaveQuery() method every time an Eloquent model calls fresh() or refresh(). In my case, this broke a ton of code, since I'm currently overriding that method for my implementation of optimistic locking on certain records.

I can change my implementation of the locking logic easily enough, but before I do that, I wanted to just raise a flag, since this seems like an abuse of the setKeysForSaveQuery() method. Should Laravel really be calling that method outside of a save query? Calling it during fresh() or refresh() is unexpected based on the method's name.

Steps To Reproduce:

N/A

@taylorotwell
Copy link
Member

It's possible we could create a new method like setKeysForSelectQuery or something.

@adamthehutt
Copy link
Contributor Author

adamthehutt commented Oct 25, 2020

@taylorotwell Thanks for the reply. That would be a lot clearer and semantically consistent, IMO. I can put together a pull request for that if you'd like.

@driesvints
Copy link
Member

@adamthehutt yeah, feel free to send in a PR. Thanks 👍

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

3 participants