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

[2.19.x] Fetching entities with Composite Key Relations and null values #11506

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

michalbundyra
Copy link
Member

Changes from #11486 into 2.19.x

@michalbundyra michalbundyra force-pushed the composite-key-relations-3 branch 3 times, most recently from 23e197d to a22feed Compare June 19, 2024 07:19
@michalbundyra michalbundyra marked this pull request as ready for review June 19, 2024 07:24
@greg0ire
Copy link
Member

Please squash both commits and use a summary of the description of #11486 . This should be understandable with the git log alone.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-wise, I think it looks good, since no existing tests are changed. If it turns out this is useful, we will revert and ask people to add tests to protect their weird use cases.

@michalbundyra
Copy link
Member Author

@greg0ire thanks, squashed, rebased and force-pushed now.

@greg0ire
Copy link
Member

use a summary of the description

Can you please amend your commit message to include a summary of the description?

@greg0ire
Copy link
Member

More on this in the contributing guide.

Remove redundant condition to check if target class contains foreign
identifier in order to allow fetching a null for relations with
composite keys, when part of the key value is null.
@michalbundyra
Copy link
Member Author

@greg0ire thanks, pushed now, hope it makes sense.

@greg0ire greg0ire merged commit 40f299f into doctrine:2.19.x Jun 21, 2024
58 checks passed
@greg0ire greg0ire added this to the 2.19.6 milestone Jun 21, 2024
@greg0ire
Copy link
Member

Thanks @michalbundyra !

@michalbundyra michalbundyra deleted the composite-key-relations-3 branch June 21, 2024 06:23
@michalbundyra
Copy link
Member Author

@greg0ire thanks for merging, I guess it will be pushed forward into 3.x as well, right?

There is still the other PR open with 3.2.x target: #11486 as it is quite different in terms of tests etc.

Thanks

@greg0ire
Copy link
Member

I guess it will be pushed forward into 3.x as well, right?

Yes, it should get merged up at some point. When that happens, maybe we will have to look at your PR to resolve conflicts/adapt the code, or maybe not and then you can rebase and improve things.

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 this pull request may close these issues.

3 participants