Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[DoctrineBridge] Do not ignore null IDs in EntityValueResolver #47242

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

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

MatTheCat
Copy link
Contributor

Q A
Branch? 6.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #47166
License MIT
Doc PR N/A

The EntityValueResolver first try to fetch an entity by its ID but handle null and non-existent IDs the same.

If a null ID is present, it feels weird to ignore it and try matching an entity using other request attributes. Better return null in this case.

@nicolas-grekas
Copy link
Member

Thank you @MatTheCat.

@nicolas-grekas nicolas-grekas merged commit 119e7d0 into symfony:6.2 Aug 18, 2022
@janklan
Copy link

janklan commented Aug 18, 2022

You lot are legends, thank you. Any chance to pop this one in 6.1 as well?

@MatTheCat MatTheCat deleted the ticket_47166 branch August 18, 2022 21:16
@MatTheCat
Copy link
Contributor Author

@janklan the entity value resolver was introduced in 6.2 😅

@nicolas-grekas would it make sense to backport this to the framework extra bundle?

@janklan
Copy link

janklan commented Aug 18, 2022

Ah, right, we're leaving SensioFrameworkExtraBundle. Well, I hope @nicolas-grekas will say yes, but since this behaviour has been there for possibly years and nobody seemed to be bothered, maybe it's not worth your time.

@fabpot
Copy link
Member

fabpot commented Aug 19, 2022

As we’re going to deprecate the bundle and because it would be a BC break, we won’t backport this change to the bundle.

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.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.