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

Deprecate overriding fields/associations inherited from other entities #10470

Merged
merged 1 commit into from
Feb 12, 2023

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jan 26, 2023

This was brought up in #8348, but seemingly forgotten to be implemented in later versions.

Exact reference:
https://github.com/doctrine/orm/pull/8348/files#diff-732e324167dd49e48b221477c3e0f6d7934f3eb5ec0970dbf1c06f6c7df15398R2261-R2264

Probably never really worked, because a subclass entity cannot have its fields mapped to other columns than the parent entity (just think about what that would mean for STI/JTI, for example).

Closes #10289.

Update #10519 adds the check for associations.

@mpdude mpdude changed the base branch from 2.14.x to 2.15.x January 26, 2023 21:56
@mpdude mpdude changed the title prevent entity override Trigger a deprecation notice when entity fields/associations are overridden Jan 26, 2023
@mpdude mpdude changed the title Trigger a deprecation notice when entity fields/associations are overridden Deprecate overriding fields/associations inherited from other entities Jan 26, 2023
@mpdude
Copy link
Contributor Author

mpdude commented Jan 26, 2023

I am open for suggestions regarding more precise wording.

We don’t care where the field comes from, be it a trait, a mapped superclass or maybe even a transient class.

Key point is that it may not be present in another entity of the same inheritance tree and be overridden for a subclass only.

Verified

This commit was signed with the committer’s verified signature.
morozov Sergei Morozov
…ridden

This was brought up in doctrine#8348, but seemingly forgotten to be implenented in later versions.

Closes doctrine#10289.
@mpdude mpdude force-pushed the prevent-entity-override branch from a4ca839 to c9b644d Compare February 9, 2023 22:17
@mpdude
Copy link
Contributor Author

mpdude commented Feb 11, 2023

How can we make sure not to forget enabling the exception after this is been merged up into 3.0?

@greg0ire
Copy link
Member

We have no systematic way of doing that, sadly. We will perform a grep before releasing 3.0.x, but I hope we won't stumble on this contributed by then-unreachable contributors.

@greg0ire greg0ire merged commit 3810a0b into doctrine:2.15.x Feb 12, 2023
@greg0ire greg0ire added this to the 2.15.0 milestone Feb 12, 2023
@greg0ire
Copy link
Member

I'm working on the merge up. While doing so, I noticed a comment similar to the one you changed, that one is in setAssociationOverride, the one you modified in setAttributeOverride.

@mpdude mpdude deleted the prevent-entity-override branch February 12, 2023 17:43
@mpdude
Copy link
Contributor Author

mpdude commented Feb 12, 2023

Then we should do the same there, x-ref #8348 and /cc @beberlei

@mpdude
Copy link
Contributor Author

mpdude commented Feb 13, 2023

#10520 turns the deprecation notice into an exception in 3.0.x

mpdude added a commit to mpdude/doctrine2 that referenced this pull request Feb 13, 2023

Verified

This commit was signed with the committer’s verified signature.
morozov Sergei Morozov
@mpdude
Copy link
Contributor Author

mpdude commented Feb 13, 2023

#10519 adds the check for associations.

mpdude added a commit to mpdude/doctrine2 that referenced this pull request Feb 13, 2023

Verified

This commit was signed with the committer’s verified signature.
morozov Sergei Morozov
mpdude added a commit to mpdude/doctrine2 that referenced this pull request Feb 13, 2023

Verified

This commit was signed with the committer’s verified signature.
morozov Sergei Morozov
greg0ire added a commit that referenced this pull request Feb 13, 2023

Verified

This commit was signed with the committer’s verified signature.
morozov Sergei Morozov
Turn deprecation from #10470 into an exception in 3.0.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants