Skip to content

Conversation

yecraftsman
Copy link

Copy link

Choose a reason for hiding this comment

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

shouldn't it be NormalizerInterface?

Copy link
Author

Choose a reason for hiding this comment

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

Well seen, since normalize and denormalize can check both.

Copy link
Contributor

Choose a reason for hiding this comment

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

"must implements" -> "must implement"

Also, there's an AbstractNormalizer which makes things easier...

@teohhanhui
Copy link
Contributor

It's unclear as to what you're trying to achieve here... (Yes, I've read api-platform/core#316 but I still don't understand what this doc section is demonstrating.)

@dunglas
Copy link
Member

dunglas commented Dec 14, 2015

@yelmontaser any update on this PR?

@yecraftsman
Copy link
Author

@teohhanhui as shown in the following post api-platform/core#316, the Dunglas API does not permit to specify a serialization group in some contexts, such as having diffirents following groups that is in the resource or in the collection or simply when a user is authenticated etc.

The problem of the method initNormalizationContext is that it applies the group in general context of the resource and collection.
The objective is not to create a new serializer but to simply add a class that helps control the context of application groups.

Have you another good easy way to do it?

@yecraftsman
Copy link
Author

I understand that it is a bit ambiguous, but I think that the proposal @dunglas is cleaner than doing it through a controller or event.

@sroze
Copy link
Contributor

sroze commented Mar 4, 2016

@yelmontaser so we should close this PR?

@yecraftsman
Copy link
Author

@sroze close if not necessary

@dunglas
Copy link
Member

dunglas commented May 3, 2016

I'm 👍 to merge this PR when comments of @teohhanhui will be fixed. It would be nice to also add a link to the related Symfony doc.

@Simperfit
Copy link
Contributor

@yelmonster, Do you have some time to finish this PR ?

@yecraftsman
Copy link
Author

@Simperfit sorry I do not have time right now and the API-Platform has changed significantly since.

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