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

Closed
wants to merge 1 commit into from

Conversation

kirba
Copy link

@kirba kirba commented Sep 13, 2018

Same as #25559, except this time targeting new version as you said you wouldn't change something like this on a point release.

Below is a comment from previous PR:
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

I see no point in changing this honestly. It's been this way for years without problems.

@kirba
Copy link
Author

kirba commented Sep 13, 2018

@taylorotwell it has been 17 days like this not for years. $attributes property was accessed directly and overriding getAttributes() method didn't cause a problem with performInsert(). Now if we override getAttbibutes() and return some transient attributes there as well we will have a problem with insert. Before 17 days we didn't have this problem.

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