diff --git a/assets/js/app/ajax-save.js b/assets/js/app/ajax-save.js index d19fa8177..5f53abc51 100644 --- a/assets/js/app/ajax-save.js +++ b/assets/js/app/ajax-save.js @@ -46,7 +46,8 @@ $(document).ready(function() { $('#toastTitle').addClass(['toast-header', typeClass]); $('#toastNotification').append(notification); $('#toastType').append(toastType); - $('#toastBody').append(toastMessage); + // Set as text, because validation messages can contain the value that was rejected. + $('#toastBody').text(toastMessage); $(document).ready(function() { let toastElList = [].slice.call(document.querySelectorAll('.toast')); @@ -59,6 +60,27 @@ $(document).ready(function() { }); }); } + + // Shows the validation errors of a rejected save above the form, the way the classic + // (non-ajax) save shows them, see `templates/content/edit.html.twig`. + function showValidationErrors(errors) { + let container = $('
'); + + errors.forEach(function(error) { + let alert = $(''); + alert.text(error.property ? error.property + ': ' + error.message : error.message); + container.append(alert); + }); + + clearValidationErrors(); + $(form).before(container); + container[0].scrollIntoView({ block: 'nearest' }); + } + + function clearValidationErrors() { + $('#editcontent-validation-errors').remove(); + } + this.href = window.location.pathname; let duplicate_id = this.href.substring(this.href.lastIndexOf('/') + 1); @@ -86,6 +108,7 @@ $(document).ready(function() { elementButton.prop('disabled', false); }, success: function(data, textStatus) { + clearValidationErrors(); if (!record_id) { window.location.replace(data.url); } else if (window.location.pathname === '/bolt/duplicate/' + duplicate_id) { @@ -99,6 +122,21 @@ $(document).ready(function() { } }, error: function(jq, status, err) { + let response = jq.responseJSON; + + // The save was rejected by the content validator, which returns its + // violations as JSON, so we can show the validator's own messages. + if (jq.status === 422 && response && Array.isArray(response.errors)) { + let messages = response.errors.map(function(error) { + return error.message; + }); + + showValidationErrors(response.errors); + showToast(response.type, messages.join(' '), response.status, response.notification, dom_element); + + return; + } + // eslint-disable-next-line no-console console.log(status, err); showToast(); diff --git a/src/Controller/Backend/ContentEditController.php b/src/Controller/Backend/ContentEditController.php index 204f8927f..d932683dc 100644 --- a/src/Controller/Backend/ContentEditController.php +++ b/src/Controller/Backend/ContentEditController.php @@ -42,6 +42,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Validator\ConstraintViolationListInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; use Symfony\Contracts\Translation\TranslatorInterface; @@ -153,6 +154,13 @@ public function save(Request $request, ?Content $originalContent = null, ?Conten if ($enableContentValidator && $contentValidator) { $constraintViolations = $contentValidator->validate($content); if (count($constraintViolations) > 0) { + // When "Saving Ajaxy" the editor isn't re-rendered, so the violations that + // renderEditor() puts in the HTML would never reach the browser. Return them + // as JSON instead, for `assets/js/app/ajax-save.js` to display. + if ($request->isXmlHttpRequest()) { + return $this->renderValidationErrors($request, $constraintViolations); + } + $this->addFlash('danger', 'content.validation_errors'); return $this->renderEditor($request, $content, $constraintViolations); @@ -651,4 +659,27 @@ private function renderEditor(Request $request, Content $content, $errors = null return $this->render('@bolt/content/edit.html.twig', $twigvars); } + + private function renderValidationErrors(Request $request, ConstraintViolationListInterface $constraintViolations): JsonResponse + { + $locale = $request->getLocale(); + + $errors = []; + foreach ($constraintViolations as $constraintViolation) { + $errors[] = [ + 'property' => $constraintViolation->getPropertyPath(), + 'message' => (string) $constraintViolation->getMessage(), + ]; + } + + return new JsonResponse( + [ + 'status' => 'danger', + 'type' => $this->translator->trans('warning', [], null, $locale), + 'notification' => $this->translator->trans('flash_messages.notification', [], null, $locale), + 'errors' => $errors, + ], + Response::HTTP_UNPROCESSABLE_ENTITY + ); + } } diff --git a/tests/php/Controller/Backend/ContentEditControllerSaveTest.php b/tests/php/Controller/Backend/ContentEditControllerSaveTest.php new file mode 100644 index 000000000..ae88a3d6f --- /dev/null +++ b/tests/php/Controller/Backend/ContentEditControllerSaveTest.php @@ -0,0 +1,142 @@ +createController(); + + $response = $controller->save( + $this->ajaxSaveRequest(), + new Content(), + $this->contentValidatorRejectingWith( + new ConstraintViolation('End datetime is required', null, [], null, 'end_datetime', null), + // Validators built by hand often leave the property path empty. + new ConstraintViolation('The record is not valid', null, [], null, null, null) + ) + ); + + self::assertInstanceOf(JsonResponse::class, $response); + self::assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); + + $payload = json_decode((string) $response->getContent(), true); + + self::assertSame('danger', $payload['status']); + self::assertSame('warning', $payload['type']); + self::assertSame('flash_messages.notification', $payload['notification']); + self::assertSame([ + ['property' => 'end_datetime', 'message' => 'End datetime is required'], + ['property' => '', 'message' => 'The record is not valid'], + ], $payload['errors']); + } + + private function createController(): ContentEditController + { + $controller = new ContentEditController( + $this->createMock(TaxonomyRepository::class), + $this->createMock(RelationRepository::class), + $this->createMock(ContentRepository::class), + $this->createMock(MediaRepository::class), + $this->createMock(EntityManagerInterface::class), + $this->createMock(UrlGeneratorInterface::class), + $this->createMock(ContentFillListener::class), + $this->createMock(EventDispatcherInterface::class), + 'en', + $this->translatorReturningKeys(), + $this->createMock(ContentHelper::class) + ); + + $csrfTokenManager = $this->createMock(CsrfTokenManagerInterface::class); + $csrfTokenManager->method('isTokenValid')->willReturn(true); + $controller->setCsrfTokenManager($csrfTokenManager); + + $authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authorizationChecker->method('isGranted')->willReturn(true); + + $container = new Container(); + $container->set('security.authorization_checker', $authorizationChecker); + $container->set('security.token_storage', $this->createMock(TokenStorageInterface::class)); + $controller->setContainer($container); + + // Of the collaborators that `setAutowire()` injects, this code path only reads the + // config, so that is the only one set here. + (new ReflectionProperty(TwigAwareController::class, 'config'))->setValue($controller, $this->configWithValidatorEnabled()); + + return $controller; + } + + private function configWithValidatorEnabled(): Config + { + $config = $this->createMock(Config::class); + $config->method('get')->willReturnCallback( + static fn (string $path, $default = null) => $path === 'general/validator_options/enable' ? true : $default + ); + + return $config; + } + + private function translatorReturningKeys(): TranslatorInterface + { + $translator = $this->createMock(TranslatorInterface::class); + $translator->method('trans')->willReturnArgument(0); + + return $translator; + } + + private function contentValidatorRejectingWith(ConstraintViolation ...$violations): ContentValidatorInterface + { + $contentValidator = $this->createMock(ContentValidatorInterface::class); + $contentValidator->method('validate')->willReturn(new ConstraintViolationList($violations)); + + return $contentValidator; + } + + private function ajaxSaveRequest(): Request + { + $request = Request::create('/bolt/edit/1', Request::METHOD_POST, [ + '_csrf_token' => 'valid', + '_edit_locale' => 'en', + 'status' => Statuses::DRAFT, + ]); + $request->headers->set('X-Requested-With', 'XMLHttpRequest'); + + return $request; + } +}