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.

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

Fixing using serializer groups along with other parsers #639

Closed
wants to merge 2 commits into from
Closed

Fixing using serializer groups along with other parsers #639

wants to merge 2 commits into from

Conversation

cristiangsp
Copy link

Description of the problem:

Some users (me included) have experienced problems using JMS Serializer groups along with Validation annotations. The problem was that the fields of the class specified as input or output were not filtered properly by the groups whenever the field had some Validation annotation attached to it.

The problem is originated by the fact that the parsers collect all the information available and after that, the results of each parser are added all together. So it doesn´t matter that the JMS Serializer groups skipped some of the fields of the input/output class, if the field was also visited by some other parser, it will be added to the final output result.

That also means that the groups were not working properly in combination with any other parser too, not only with Validation annotations.

Proposed solution:

This pull request proposes adding a new boolean field with key "excluded" to the data outputted by a parser about a input/output class field. That will allow a parser to mark a specific field as excluded and that information then will be shared to decide if the field should appear in the auto-generated docs or not.

The JmsMetadataParser has been modified to now check if a field should be skipped and if it should be skipped instead of just continue it sets "excluded" to true. The rest of the parsers, as they don´t have methods to exclude fields, have been modified to always return "excluded" as false.

The method.twig now check for the "excluded" field before rendering a field.

Tests modified

I have modified all the test related to parsers to expect the "excluded" field to be in the output generated. For the specific case of testing the JmsMetadataParser the tests related with groups have received deeper changes to maintain the new logic of not just skipping the fields that are not in specific groups.

Also the SimpleFormatterTest have been modified to take into account the new "excluded" field.

Looking forward to see your comments about it.
Thanks.

@cristiangsp
Copy link
Author

After fixing the SimpleFormatterTest the tests keep failing because of #636.

@psampaz
Copy link

psampaz commented Nov 3, 2015

@willdurand can you reopen this?
I think using validator annotations along with jms serializer groups is a common case.

@willdurand
Copy link
Collaborator

@psampaz could you take care of this PR for me please? (like fetching the commits, making sure it works with master branch, and create a new PR?)

@psampaz
Copy link

psampaz commented Nov 3, 2015

I'll do my best

@develth
Copy link

develth commented Aug 4, 2017

Hey,

whats the status here? Anything i could help to get this released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants