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

added error-response when running request with XHR #5710

Merged
merged 15 commits into from
Oct 27, 2019

Conversation

oleg-andreyev
Copy link
Contributor

@oleg-andreyev oleg-andreyev commented Oct 9, 2019

Subject

Adding error-response for XHR requests

Current behaviour: when submitting form with AJAX (XHR) and if error occurs HTML will be returned instead of JSON.

I am targeting this branch, because features (BC?)

Changelog

### Changed
Added JSON response from edit/create action when form is invalid.

To do

  • Update the tests

src/Controller/CRUDController.php Outdated Show resolved Hide resolved
core23
core23 previously approved these changes Oct 13, 2019
@greg0ire
Copy link
Contributor

Adding error-response for XHR requests

Please describe the current behavior. Do we currently return an empty response?

src/Controller/CRUDController.php Outdated Show resolved Hide resolved
Copy link
Contributor Author

@oleg-andreyev oleg-andreyev left a comment

Choose a reason for hiding this comment

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

Adding error-response for XHR requests

Please describe the current behavior. Do we currently return an empty response?

Updated description

OskarStark
OskarStark previously approved these changes Oct 16, 2019
phansys
phansys previously approved these changes Oct 16, 2019
@greg0ire
Copy link
Contributor

if error occurs HTML will be returned instead of JSON

Do you think people might currently be relying on that kind of HTML?

@oleg-andreyev
Copy link
Contributor Author

@greg0ire I think it’s possible but only as part of workaround for this inconsistent behavior.

@greg0ire
Copy link
Contributor

Maybe we should apply your fix only when the client sends an Accept header that requests application/json, and trigger a deprecation otherwise? The deprecation would say that next major will reject text/html with a 406

core23
core23 previously approved these changes Oct 18, 2019
@oleg-andreyev oleg-andreyev dismissed stale reviews from phansys and OskarStark via a95f9a2 October 26, 2019 21:39
@oleg-andreyev
Copy link
Contributor Author

@greg0ire @core23 @OskarStark I've done what @greg0ire suggested above, I've done same for success result.

@core23
Copy link
Member

core23 commented Oct 27, 2019

Thansk @oleg-andreyev

@core23 core23 merged commit cc0fdef into sonata-project:3.x Oct 27, 2019
@fancyweb
Copy link
Contributor

This PR causes an issue with SonataPageBundle since this bundle uses the CRUDController and does all edit with ajax requests. Everytime validation fail on a block edit, an error flash message is added (contrary to before). Then all those messages are displayed at once on the next page the user visits. What can we do about this?

@OskarStark
Copy link
Member

Can you share a screenshot?

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.

6 participants