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.

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

added error-response when running request with XHR #5710

Merged
merged 15 commits into from
Oct 27, 2019
Merged

added error-response when running request with XHR #5710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
63e50b6
added error-response when running request with XHR
oleg-andreyev Oct 9, 2019
1aefdd6
moved code into separate response
oleg-andreyev Oct 11, 2019
f1d37be
added JsonResponse type to `renderJson` and `handleXmlHttpRequestErro…
oleg-andreyev Oct 12, 2019
b191c9f
!squash fixing cs
oleg-andreyev Oct 12, 2019
a7ec33f
fixing CRUDControllerTest::testEditActionAjaxError
oleg-andreyev Oct 12, 2019
be6c947
fixing CRUDControllerTest::testCreateActionAjaxError
oleg-andreyev Oct 12, 2019
bf61478
!squash reverted return-type of renderJson
oleg-andreyev Oct 14, 2019
2a5da35
!squash removed empty-array
oleg-andreyev Oct 14, 2019
c892afe
!squash reverted renderJson return type
oleg-andreyev Oct 16, 2019
39c5298
extracted success response to handleXmlHttpRequestSuccessResponse
oleg-andreyev Oct 25, 2019
f59e4af
added Request to handleXmlHttpRequestSuccessResponse and handleXmlHtt…
oleg-andreyev Oct 25, 2019
65ef288
added deprecation notices
oleg-andreyev Oct 26, 2019
a95f9a2
added tests
oleg-andreyev Oct 26, 2019
0f6cee6
fixed flintci
oleg-andreyev Oct 26, 2019
ca5193d
fixed flintci
oleg-andreyev Oct 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 61 additions & 29 deletions src/Controller/CRUDController.php
View file
Open in desktop
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
Open in desktop
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