Skip to content

Commit

Permalink
Add error-response when running request with XHR (#5710)
Browse files Browse the repository at this point in the history 
* added error-response when running request with XHR

* moved code into separate response

* added JsonResponse type to `renderJson` and `handleXmlHttpRequestErrorResponse`

* !squash fixing cs

* fixing CRUDControllerTest::testEditActionAjaxError

* fixing CRUDControllerTest::testCreateActionAjaxError

* !squash reverted return-type of renderJson

* !squash removed empty-array

* !squash reverted renderJson return type

* extracted success response to handleXmlHttpRequestSuccessResponse

* added Request to handleXmlHttpRequestSuccessResponse and handleXmlHttpRequestErrorResponse

* added deprecation notices

* added tests

* fixed flintci

* fixed flintci
  • Loading branch information
@oleg-andreyev @core23
oleg-andreyev authored and core23 committed Oct 27, 2019
1 parent 470eac1 commit cc0fdef
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 44 deletions.
90 changes: 61 additions & 29 deletions src/Controller/CRUDController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormRenderer;
use Symfony\Component\Form\FormView;
use Symfony\Component\HttpFoundation\JsonResponse;
Expand Down Expand Up @@ -355,11 +356,7 @@ public function editAction($id = null)
$existingObject = $this->admin->update($submittedObject);

if ($this->isXmlHttpRequest()) {
return $this->renderJson([
'result' => 'ok',
'objectId' => $objectId,
'objectName' => $this->escapeHtml($this->admin->toString($existingObject)),
], 200, []);
return $this->handleXmlHttpRequestSuccessResponse($request, $existingObject);
}

$this->addFlash(
Expand Down Expand Up @@ -388,16 +385,18 @@ public function editAction($id = null)

// show an error message if the form failed validation
if (!$isFormValid) {
if (!$this->isXmlHttpRequest()) {
$this->addFlash(
'sonata_flash_error',
$this->trans(
'flash_edit_error',
['%name%' => $this->escapeHtml($this->admin->toString($existingObject))],
'SonataAdminBundle'
)
);
if ($this->isXmlHttpRequest() && null !== ($response = $this->handleXmlHttpRequestErrorResponse($request, $form))) {
return $response;
}

$this->addFlash(
'sonata_flash_error',
$this->trans(
'flash_edit_error',
['%name%' => $this->escapeHtml($this->admin->toString($existingObject))],
'SonataAdminBundle'
)
);
} elseif ($this->isPreviewRequested()) {
// enable the preview template if the form was valid and preview was requested
$templateKey = 'preview';
Expand Down Expand Up @@ -619,11 +618,7 @@ public function createAction()
$newObject = $this->admin->create($submittedObject);

if ($this->isXmlHttpRequest()) {
return $this->renderJson([
'result' => 'ok',
'objectId' => $this->admin->getNormalizedIdentifier($newObject),
'objectName' => $this->escapeHtml($this->admin->toString($newObject)),
], 200, []);
return $this->handleXmlHttpRequestSuccessResponse($request, $newObject);
}

$this->addFlash(
Expand All @@ -646,16 +641,18 @@ public function createAction()

// show an error message if the form failed validation
if (!$isFormValid) {
if (!$this->isXmlHttpRequest()) {
$this->addFlash(
'sonata_flash_error',
$this->trans(
'flash_create_error',
['%name%' => $this->escapeHtml($this->admin->toString($newObject))],
'SonataAdminBundle'
)
);
if ($this->isXmlHttpRequest() && null !== ($response = $this->handleXmlHttpRequestErrorResponse($request, $form))) {
return $response;
}

$this->addFlash(
'sonata_flash_error',
$this->trans(
'flash_create_error',
['%name%' => $this->escapeHtml($this->admin->toString($newObject))],
'SonataAdminBundle'
)
);
} elseif ($this->isPreviewRequested()) {
// pick the preview template if the form was valid and preview was requested
$templateKey = 'preview';
Expand Down Expand Up @@ -1094,7 +1091,7 @@ protected function getParameter($name)
* @param int $status
* @param array $headers
*
* @return Response with json encoded data
* @return JsonResponse with json encoded data
*/
protected function renderJson($data, $status = 200, $headers = [])
{
Expand Down Expand Up @@ -1592,4 +1589,39 @@ private function setFormTheme(FormView $formView, array $theme = null): void

$twig->getRuntime(FormRenderer::class)->setTheme($formView, $theme);
}

private function handleXmlHttpRequestErrorResponse(Request $request, FormInterface $form): ?JsonResponse
{
if ('application/json' !== $request->headers->get('Accept')) {
@trigger_error('In next major version response will return 406 NOT ACCEPTABLE without `Accept: application/json`', E_USER_DEPRECATED);

return null;
}

$errors = [];
foreach ($form->getErrors(true) as $error) {
$errors[] = $error->getMessage();
}

return $this->renderJson([
'result' => 'error',
'errors' => $errors,
], 400);
}

/**
* @param object $object
*/
private function handleXmlHttpRequestSuccessResponse(Request $request, $object): JsonResponse
{
if ('application/json' !== $request->headers->get('Accept')) {
@trigger_error('In next major version response will return 406 NOT ACCEPTABLE without `Accept: application/json`', E_USER_DEPRECATED);
}

return $this->renderJson([
'result' => 'ok',
'objectId' => $this->admin->getNormalizedIdentifier($object),
'objectName' => $this->escapeHtml($this->admin->toString($object)),
], 200);
}
}
134 changes: 119 additions & 15 deletions tests/Controller/CRUDControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@
use Symfony\Bundle\FrameworkBundle\Templating\DelegatingEngine;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormRenderer;
use Symfony\Component\Form\FormView;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
Expand Down Expand Up @@ -1751,7 +1753,7 @@ public function testEditActionAjaxSuccess(): void
->method('all')
->willReturn(['field' => 'fielddata']);

$this->admin->expects($this->once())
$this->admin
->method('getNormalizedIdentifier')
->with($this->equalTo($object))
->willReturn('foo_normalized');
Expand Down Expand Up @@ -1801,26 +1803,75 @@ public function testEditActionAjaxError(): void
->method('all')
->willReturn(['field' => 'fielddata']);

$formError = $this->createMock(FormError::class);
$formError->expects($this->atLeastOnce())
->method('getMessage')
->willReturn('Form error message');

$form->expects($this->once())
->method('getErrors')
->with(true)
->willReturn([$formError]);

$this->request->setMethod('POST');
$this->request->headers->set('X-Requested-With', 'XMLHttpRequest');
$this->request->headers->set('Accept', 'application/json');

$formView = $this->createMock(FormView::class);
$this->assertInstanceOf(JsonResponse::class, $response = $this->controller->editAction(null, $this->request));
$this->assertJsonStringEqualsJsonString('{"result":"error","errors":["Form error message"]}', $response->getContent());
}

$form->expects($this->any())
/**
* @legacy
* @expectedDeprecation In next major version response will return 406 NOT ACCEPTABLE without `Accept: application/json`
*/
public function testEditActionAjaxErrorWithoutAcceptApplicationJson(): void
{
$object = new \stdClass();

$this->admin->expects($this->once())
->method('getObject')
->willReturn($object);

$this->admin->expects($this->once())
->method('checkAccess')
->with($this->equalTo('edit'))
->willReturn(true);

$form = $this->createMock(Form::class);

$this->admin->expects($this->once())
->method('getForm')
->willReturn($form);

$form->expects($this->once())
->method('isSubmitted')
->willReturn(true);

$form->expects($this->once())
->method('isValid')
->willReturn(false);

$form->expects($this->once())
->method('all')
->willReturn(['field' => 'fielddata']);

$this->request->setMethod('POST');
$this->request->headers->set('X-Requested-With', 'XMLHttpRequest');

$formView = $this->createMock(FormView::class);
$form
->method('createView')
->willReturn($formView);

$this->assertInstanceOf(Response::class, $this->controller->editAction(null, $this->request));

$this->assertInstanceOf(Response::class, $response = $this->controller->editAction(null, $this->request));
$this->assertSame($this->admin, $this->parameters['admin']);
$this->assertSame('@SonataAdmin/ajax_layout.html.twig', $this->parameters['base_template']);
$this->assertSame($this->pool, $this->parameters['admin_pool']);

$this->assertSame('edit', $this->parameters['action']);
$this->assertInstanceOf(FormView::class, $this->parameters['form']);
$this->assertSame($object, $this->parameters['object']);

$this->assertSame([], $this->session->getFlashBag()->all());
$this->assertSame(['sonata_flash_error' => [0 => null]], $this->session->getFlashBag()->all());
$this->assertSame('@SonataAdmin/CRUD/edit.html.twig', $this->template);
}

Expand Down Expand Up @@ -2518,26 +2569,79 @@ public function testCreateActionAjaxError(): void
->method('isValid')
->willReturn(false);

$formError = $this->createMock(FormError::class);
$formError->expects($this->atLeastOnce())
->method('getMessage')
->willReturn('Form error message');

$form->expects($this->once())
->method('getErrors')
->with(true)
->willReturn([$formError]);

$this->request->setMethod('POST');
$this->request->headers->set('X-Requested-With', 'XMLHttpRequest');
$this->request->headers->set('Accept', 'application/json');

$formView = $this->createMock(FormView::class);
$this->assertInstanceOf(JsonResponse::class, $response = $this->controller->createAction($this->request));
$this->assertJsonStringEqualsJsonString('{"result":"error","errors":["Form error message"]}', $response->getContent());
}

$form->expects($this->any())
/**
* @legacy
* @expectedDeprecation In next major version response will return 406 NOT ACCEPTABLE without `Accept: application/json`
*/
public function testCreateActionAjaxErrorWithoutAcceptApplicationJson(): void
{
$this->admin->expects($this->once())
->method('checkAccess')
->with($this->equalTo('create'))
->willReturn(true);

$object = new \stdClass();

$this->admin->expects($this->once())
->method('getNewInstance')
->willReturn($object);

$form = $this->createMock(Form::class);

$this->admin->expects($this->any())
->method('getClass')
->willReturn('stdClass');

$this->admin->expects($this->once())
->method('getForm')
->willReturn($form);

$form->expects($this->once())
->method('all')
->willReturn(['field' => 'fielddata']);

$form->expects($this->once())
->method('isSubmitted')
->willReturn(true);

$form->expects($this->once())
->method('isValid')
->willReturn(false);

$this->request->setMethod('POST');
$this->request->headers->set('X-Requested-With', 'XMLHttpRequest');

$formView = $this->createMock(FormView::class);
$form
->method('createView')
->willReturn($formView);

$this->assertInstanceOf(Response::class, $this->controller->createAction($this->request));

$this->assertInstanceOf(Response::class, $response = $this->controller->createAction($this->request));
$this->assertSame($this->admin, $this->parameters['admin']);
$this->assertSame('@SonataAdmin/ajax_layout.html.twig', $this->parameters['base_template']);
$this->assertSame($this->pool, $this->parameters['admin_pool']);

$this->assertSame('create', $this->parameters['action']);
$this->assertInstanceOf(FormView::class, $this->parameters['form']);
$this->assertSame($object, $this->parameters['object']);

$this->assertSame([], $this->session->getFlashBag()->all());
$this->assertSame(['sonata_flash_error' => [0 => null]], $this->session->getFlashBag()->all());
$this->assertSame('@SonataAdmin/CRUD/edit.html.twig', $this->template);
}

Expand Down

0 comments on commit cc0fdef

Please sign in to comment.