Skip to content

Navigation Menu

Sign in
Appearance settings

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

Merged
nicolas-grekas merged 1 commit intosymfony:6.2symfony/symfony:6.2from
MatTheCat:ticket_47166MatTheCat/symfony:ticket_47166Copy head branch name to clipboard
Aug 18, 2022
Merged

[DoctrineBridge] Do not ignore null IDs in EntityValueResolver#47242
nicolas-grekas merged 1 commit intosymfony:6.2symfony/symfony:6.2from
MatTheCat:ticket_47166MatTheCat/symfony:ticket_47166Copy head branch name to clipboard

Conversation

@MatTheCat
Copy link
Copy Markdown
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
Copy Markdown
Member

Thank you @MatTheCat.

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

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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.