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.

Sign up for GitHub

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

Jump to bottom

[Serializer] Try all possible denormalization route with union types when ALLOW_EXTRA_ATTRIBUTES=false #45861

Merged
merged 1 commit into from
Jun 19, 2022
Merged

[Serializer] Try all possible denormalization route with union types when ALLOW_EXTRA_ATTRIBUTES=false #45861

merged 1 commit into from
Jun 19, 2022

Conversation

T-bond
Copy link
Contributor

@T-bond T-bond commented Mar 27, 2022 •
edited by nicolas-grekas
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #45860
License MIT
Doc PR -

I found a similar bug, (I think this is a bug, but it is possible, it is an intended behaviour) to my previous report.

@T-bond T-bond requested a review from dunglas as a code owner March 27, 2022 00:54
@carsonbot carsonbot added this to the 4.4 milestone Mar 27, 2022
T-bond
T-bond commented Mar 27, 2022
View reviewed changes
Copy link
Contributor Author

@T-bond T-bond left a comment

Choose a reason for hiding this comment

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

Self review done.

Failing tests seems unrelated.

@carsonbot
Copy link

Hey!

I think @Th3Mouk has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@NorthBlue333
Copy link

NorthBlue333 commented Jun 12, 2022 •
edited
Loading

Hello there,
I am currently working on #46284, and was wondering about the state of this PR. I made a comment because IMO we should not merge the extraAttributes of all different types, but just return one set failing.
Ideally this should be merged before my PR because I will have to adjust the code.

@NorthBlue333 NorthBlue333 mentioned this pull request Jun 12, 2022
Open
@T-bond
Copy link
Contributor Author

T-bond commented Jun 12, 2022

This PR is ready for review.
If reviewed, and accepted I will rebase to resolve the conflicts.

@nicolas-grekas nicolas-grekas changed the title [Serializer] Try all possible denormalization route with union types … [Serializer] Try all possible denormalization route with union types when ALLOW_EXTRA_ATTRIBUTES=false Jun 19, 2022
@nicolas-grekas
Copy link
Member

Thank you @T-bond.

@nicolas-grekas nicolas-grekas merged commit 0523bd3 into symfony:4.4 Jun 19, 2022
This was referenced Jun 24, 2022
Closed
Merged
nicolas-grekas added a commit that referenced this pull request Jun 26, 2022
@nicolas-grekas
bug #46765 [Serializer] Fix denormalization union types with construc…
1b2ab75 
…tor (Gwemox)

This PR was merged into the 4.4 branch.

Discussion
----------

[Serializer] Fix denormalization union types with constructor

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #46396
| License       | MIT

Fix bug when deserialize union types with constructor. Before that, `MissingConstructorArgumentsException` was thrown even if another type matched.

Is similar to #45861

Commits
-------

33fb153 [Serializer] Fix denormalization union types with constructor
This was referenced Jun 26, 2022
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NorthBlue333 NorthBlue333 NorthBlue333 left review comments

@dunglas dunglas dunglas approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug Serializer Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@T-bond @carsonbot @NorthBlue333 @nicolas-grekas @dunglas