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

Fix page composer block validation #1322

Closed
wants to merge 1 commit into from
Closed

Fix page composer block validation #1322

wants to merge 1 commit into from

Conversation

gremo
Copy link
Contributor

@gremo gremo commented Jul 15, 2021

Subject

I am targeting this branch, because {reason}.

Closes #1318.

Changelog

### Fixed 
- page composer block validation

Explanation (copy-past from the issue #1318 )

What's happening then? Well, this commit changed the way Ajax requests are threated. This is the related PR.

To put it simple:

  1. Page composer send requests using Ajax without any Accept header, that is */*
  2. That commit changed the Ajax request handling, as */* is now inside the getAcceptableContentTypes() array:
private function handleXmlHttpRequestErrorResponse(Request $request, FormInterface $form): ?JsonResponse
{
    if (empty(array_intersect(['application/json', '*/*'], $request->getAcceptableContentTypes()))) {
        // Do not handle the XmlHttpRequest
        return null;
    }

   // Return a JSON response
}

Before that commit, the same methods looks like:

private function handleXmlHttpRequestErrorResponse(Request $request, FormInterface $form): ?JsonResponse
{
    if (!\in_array('application/json', $request->getAcceptableContentTypes(), true)) {
        // Do not handle the XmlHttpRequest
        return null;
    }

     // Return a JSON response
}
  1. As a result, a JSON response is returned but SonataPageBundle can only handle HTML responses

Easy fix is adding an Accept header in assets/src/js/composer.js, inside the props of $.ajax:

headers: {
  Accept: 'text/html, application/xhtml+xml;'
},

VincentLanglet
VincentLanglet previously approved these changes Jul 15, 2021
@VincentLanglet VincentLanglet requested a review from a team July 15, 2021 10:07
@VincentLanglet
Copy link
Member

I just find out that the same code exist here: https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Resources/public/sonata-page.back.js#L39

I don't know why it's duplicated.

@gremo
Copy link
Contributor Author

gremo commented Jul 15, 2021

@VincentLanglet you mean I have to change also that file? Isn't automatically built upon modification of composer.js?

@VincentLanglet
Copy link
Member

@VincentLanglet you mean I have to change also that file? Isn't automatically built upon modification of composer.js?

I really don't know how this works, but nothing is updated when running yarn build.

so I updated all this files manually in #1323.

@gremo
Copy link
Contributor Author

gremo commented Jul 15, 2021

Thank you @VincentLanglet 🙏 are you planning a new release soon? So I can test it right now!

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.

Block validation doesn't work properly in page composer
2 participants