Skip to content

[Serializer] Add special '*' serialization group that allows any group #33540

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
Merged
fabpot merged 1 commit into from
Aug 25, 2020
Merged

[Serializer] Add special '*' serialization group that allows any group #33540

fabpot merged 1 commit into from
Aug 25, 2020

Conversation

@nrobinaubertin
Copy link
Contributor

@nrobinaubertin nrobinaubertin commented Sep 10, 2019 •
edited
Loading

Q A
Branch? master for features
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #32622
License MIT
Doc PR symfony/symfony-docs#...

Hi,
I added support for a special serialization group: '*'.
This group lets any group serialize the attribute marked with it.

The BC break comes from the fact that someone could have used the '*' group in their code.

@nrobinaubertin nrobinaubertin mentioned this pull request Sep 10, 2019
Closed
@nicolas-grekas nicolas-grekas added this to the next milestone Sep 12, 2019
@fabpot fabpot requested a review from dunglas February 8, 2020 07:57
fabpot
fabpot approved these changes Aug 16, 2020
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

LGTM.

Can you change the base branch to master and rebase?

dunglas
dunglas reviewed Aug 16, 2020
View reviewed changes
fabpot
fabpot requested changes Aug 17, 2020
View reviewed changes
@nrobinaubertin nrobinaubertin changed the base branch from 4.4 to master August 19, 2020 07:50
@nrobinaubertin
Copy link
Contributor Author

nrobinaubertin commented Aug 20, 2020

I've implemented the change proposed by @stloyd based on the idea of @dunglas, rebased on master and fixed some tests.
There's still some that fail but I'm not used to symfony internals and I don't immediately see the changes I should make. If someone more knowledgeable can help, I would be thankful.

ogizanagi
ogizanagi reviewed Aug 20, 2020
View reviewed changes
@fabpot
Copy link
Member

fabpot commented Aug 23, 2020

@nrobinaubertin Can you take @ogizanagi suggestion into account and fix the unit tests? Thank you.

@nrobinaubertin
Copy link
Contributor Author

nrobinaubertin commented Aug 24, 2020

@nrobinaubertin Can you take @ogizanagi suggestion into account and fix the unit tests? Thank you.

Done. I added two tests but the previous ones were already working.
The fails of the CI are not related to this PR

@fabpot
Copy link
Member

fabpot commented Aug 25, 2020

Thank you @nrobinaubertin.

@fabpot fabpot mentioned this pull request Aug 25, 2020
Closed
@fabpot fabpot merged commit efdc35c into symfony:master Aug 25, 2020
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
@fabpot fabpot mentioned this pull request Oct 5, 2020
Merged
@ogizanagi ogizanagi mentioned this pull request Dec 9, 2020
Merged
4 tasks
@ToGo101
Copy link

ToGo101 commented Aug 17, 2021

Could it be, that this important note:

Symfony\Component\Serializer\Normalizer/AbstractNormalizer.php:250
// If you update this check, update accordingly the one in Symfony\Component\PropertyInfo\Extractor\SerializerExtractor::getProperties()

has been overseen an the change not made to the extractor?

@SergeyCherenkov SergeyCherenkov mentioned this pull request Nov 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@dunglas dunglas dunglas approved these changes

@jderusse jderusse Awaiting requested review from jderusse

@lyrixx lyrixx Awaiting requested review from lyrixx

@sroze sroze Awaiting requested review from sroze

@xabbuh xabbuh Awaiting requested review from xabbuh

+2 more reviewers

@ogizanagi ogizanagi ogizanagi left review comments

@stloyd stloyd stloyd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BC Break Feature Serializer Status: Reviewed

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

8 participants

@nrobinaubertin @fabpot @ToGo101 @dunglas @stloyd @ogizanagi @nicolas-grekas @carsonbot