Skip to content

[Serializer] Adds FormErrorNormalizer #36211

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

[Serializer] Adds FormErrorNormalizer #36211

merged 1 commit into from
Aug 23, 2020

Conversation

YaFou
Copy link
Contributor

@YaFou YaFou commented Mar 25, 2020 •
edited
Loading

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #27428
License MIT
Doc PR symfony/symfony-docs#13885

Readme

Adds FormErrorNormalizer for AJAX forms (see the suggestion: #27428). It was on the FOSRestBundle and now it is implemented in the Serializer component.

Roadmap

  • Core of the class
  • Output syntax
  • Support of the children forms
  • Doc

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 26, 2020
@YaFou YaFou mentioned this pull request Mar 27, 2020
Closed
@YaFou YaFou marked this pull request as ready for review June 22, 2020 16:25
@YaFou YaFou requested a review from dunglas as a code owner June 22, 2020 16:25
@YaFou YaFou mentioned this pull request Jun 22, 2020
Merged
@YaFou
Copy link
Contributor Author

YaFou commented Jun 25, 2020

I don't know why the test suite does not pass. Anybody has an answer?

fabpot
fabpot requested changes Aug 17, 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.

Is there any "standards"/prior art for form validation? That would be better than having our own if possible.

@YaFou
Copy link
Contributor Author

YaFou commented Aug 17, 2020

Is there any "standards"/prior art for form validation? That would be better than having our own if possible.

After some research, I did not find RFCs or something like that. However this type of normalizer can follow the specification RFC 7807 like the ConstraintViolationListNormalizer. In my opinion, we shouldn't follow that because forms can't be used in an API. This normalizer can be used in logs for instance. I need some opinions @fabpot

I will work on reviews next week.

@YaFou YaFou requested a review from fabpot August 23, 2020 15:44
@YaFou YaFou requested a review from xabbuh as a code owner August 23, 2020 16:06
fabpot
fabpot requested changes Aug 23, 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.

You should probably update the FrameworkBundle composer constraint to require Form 5.2+.

@YaFou YaFou requested a review from fabpot August 23, 2020 16:16
@fabpot
Copy link
Member

fabpot commented Aug 23, 2020

Let's wait for the tests... I think they will fail as you haven't not changed deps on FramewokrBundle as I mentioned in my previous review.

@YaFou
Copy link
Contributor Author

YaFou commented Aug 23, 2020

Let's wait for the tests... I think they will fail as you haven't not changed deps on FramewokrBundle as I mentioned in my previous review.

@fabpot, the tests passes: the Form Component constraint in the Framework Bundle was already at ^5.2.

fabpot
fabpot approved these changes Aug 23, 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.

Ok, symfony/form is already set to 5.2 :)

@fabpot
Copy link
Member

fabpot commented Aug 23, 2020

Thank you @YaFou.

@fabpot fabpot merged commit a435dfe into symfony:master Aug 23, 2020
@YaFou YaFou deleted the form-error-normalizer branch August 23, 2020 16:48
wouterj added a commit to symfony/symfony-docs that referenced this pull request Oct 3, 2020
@wouterj
feature #13885 [Serializer] Adds FormErrorNormalizer (YaFou)
58c9014 
This PR was merged into the master branch.

Discussion
----------

[Serializer] Adds FormErrorNormalizer

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/releases for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

This pull request is related to symfony/symfony#36211.

Commits
-------

9ed468d [Serializer] Adds FormErrorNormalizer
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
@fabpot fabpot mentioned this pull request Oct 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mohammed-elattar mohammed-elattar mohammed-elattar left review comments

@fabpot fabpot fabpot approved these changes

@dunglas dunglas Awaiting requested review from dunglas

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Feature Serializer Status: Reviewed
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.

[Form] Include Form Error Normalizer
6 participants
@YaFou @fabpot @mohammed-elattar @mindaugasvcs @nicolas-grekas @carsonbot