-
Notifications
You must be signed in to change notification settings - Fork 155
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
Return 422 status code when the form fails #488
Return 422 status code when the form fails #488
Conversation
This also brings support for Symfony UX Turbo
78d37aa
to
2f3dfc2
Compare
@@ -212,6 +212,9 @@ public function createAction(Request $request): Response | |||
|
|||
return $this->redirectHandler->redirectToResource($configuration, $newResource); | |||
} | |||
if ($request->isMethod('POST') && $form->isSubmitted() && !$form->isValid()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small improvement - we could add a blank line before if and extract the content of the check to the separate method
if ($request->isMethod('POST') && $form->isSubmitted() && !$form->isValid()) { | |
if ($this->isFormInValid($request, $form)) { |
'science_book[author][lastName]' => $newBookAuthorLastName, | ||
]); | ||
|
||
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_UNPROCESSABLE_ENTITY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it is not bc-break and if we didn't send a 400 error. But looking at the code and related PR, I assume we didn't, so I'm fine with this change
- [#341](#341) Dropping usage of Request->get ([@loic425](https://github.com/loic425), [@Zales0123](https://github.com/Zales0123)) - [#450](#450) Adjust when some runtime deprecation notices are triggered and use Symfony's trigger_deprecation() helper ([@mbabker](https://github.com/mbabker)) - [#467](#467) [README] Add development section and update links ([@lchrusciel](https://github.com/lchrusciel)) - [#478](#478) Add tests with grids ([@loic425](https://github.com/loic425)) - [#487](#487) Make CsrfTokenManager public ([@Zales0123](https://github.com/Zales0123)) - [#488](#488) Return 422 status code when the form fails ([@belmeopmenieuwesim](https://github.com/belmeopmenieuwesim), [@Zales0123](https://github.com/Zales0123))
…les0123) This PR was merged into the 1.12 branch. Discussion ---------- | Q | A | |-----------------|--------------------------------------------------------------| | Branch? | 1.12 | | Bug fix? | yes | | New feature? | no | | BC breaks? | no | | Deprecations? | no | | Related tickets | | | License | MIT | [This change](Sylius/SyliusResourceBundle#488) resulted in a different response code which we checks in our Behats. It\'s a hot-fix, as we should change it in the SyliusResourceBundle itself 💃 But I would like to unblock builds Commits ------- 5eca8c6 [Hot-fix] Do not fail scenario after 422 response page
I've taken over the #469 PR, added some tests and clean up the code. Thank you, @belmeopmenieuwesim, for the contribution 🖖