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.7] Use the getDirty method on insert instead of getAttributes #25559

Closed
wants to merge 1 commit into from
Closed

Conversation

kirba
Copy link

@kirba kirba commented Sep 10, 2018

In #25349 performInsert() was changed to use getAttributes() instead of accessing $attributes property directly, thus having more similar behavior to performUpdate().

I think it is better to use getDirty() in performInsert() as well.

Currently if we override getAttributes() method and return additional property the insert is going to fail, while update can pass if we override originalIsEquivalent() and return true for that additional property.

@taylorotwell
Copy link
Member

No way that I'm changing this on a point release 😄

@kirba
Copy link
Author

kirba commented Sep 11, 2018

should I send a PR to master or you don't like this at all?

@deleugpn
Copy link
Contributor

It's unlikely that you're going to get an answer to a closed issue since Taylor works by going through PRs on a daily basis.
His comment only indicates that this is too much of a risk to merge and release, so you can send it to master to be considered whether it should go to 5.8 or not.

@kirba
Copy link
Author

kirba commented Sep 13, 2018

Thanks, I have created a new PR to master

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.

3 participants