Skip to content

Commit

Permalink
Client creation validation refactoring [SLE-194]
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelgfeller committed Oct 23, 2023
1 parent d695fe3 commit 74fd9b0
Show file tree
Hide file tree
Showing 33 changed files with 580 additions and 369 deletions.
2 changes: 0 additions & 2 deletions config/container.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,9 @@
},
PDO::class => function (ContainerInterface $container) {
$driver = $container->get(Connection::class)->getDriver();

$class = new ReflectionClass($driver);
$method = $class->getMethod('getPdo');
$method->setAccessible(true);

return $method->invoke($driver);
},
// Used by command line to generate `schema.sql` for integration testing
Expand Down
7 changes: 6 additions & 1 deletion config/middleware.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<?php

use App\Application\Middleware\ErrorHandlerMiddleware;
use App\Application\Middleware\ForbiddenExceptionMiddleware;
use App\Application\Middleware\PhpViewExtensionMiddleware;
use App\Application\Middleware\ValidationExceptionMiddleware;
use Odan\Session\Middleware\SessionStartMiddleware;
use Selective\BasePath\BasePathMiddleware;
use Slim\App;
Expand Down Expand Up @@ -37,7 +39,10 @@
// Has to be after Routing (called before on response)
$app->add(BasePathMiddleware::class);

// Error middleware should be added last. It will not handle any exceptions/errors
// Error middlewares should be added last as they will be the first on the response (LIFO).
$app->add(ValidationExceptionMiddleware::class);
$app->add(ForbiddenExceptionMiddleware::class);

$app->add(ErrorHandlerMiddleware::class);
$app->add(ErrorMiddleware::class);
};
5 changes: 3 additions & 2 deletions docs/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,13 +317,14 @@ and I do that with the color red and orange.

File: `src/Application/ErrorHandler/DefaultErrorHandler.php` (most up to date version can be
found [here](https://github.com/samuelgfeller/slim-example-project/blob/master/src/Application/ErrorHandler/DefaultErrorHandler.php))

```php
<?php

namespace App\Application\ErrorHandler;

use App\Domain\Factory\LoggerFactory;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Psr\Http\Message\ResponseFactoryInterface;
use Psr\Http\Message\ResponseInterface;
Expand Down Expand Up @@ -454,7 +455,7 @@ class DefaultErrorHandler
$statusCode = (int)$exception->getCode();
}

if ($exception instanceof ValidationException) {
if ($exception instanceof ValidationExceptionOld) {
$statusCode = StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY; // 422
}

Expand Down
17 changes: 11 additions & 6 deletions public/assets/general/ajax/ajax-util/fail-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,25 @@ export function handleFail(xhr, domFieldId = null) {
if (xhr.status === 422) {
if (xhr.getResponseHeader('Content-type') === 'application/json') {
errorMsg = '';
let json = xhr.response;
let validationResponse = JSON.parse(json);
let validationResponse = JSON.parse(xhr.response);
const validationErrors = validationResponse.data.errors;
removeValidationErrorMessages();
// Best foreach loop method according to https://stackoverflow.com/a/9329476/9013718
for (const error of validationResponse.data.errors) {
displayValidationErrorMessage(error.field, error.message, domFieldId);
for (const fieldName in validationErrors) {
const fieldMessages = validationErrors[fieldName];
// There may be the case that there are multiple error messages for a single field. In such case,
// the previous error message is simply replaced by the newer one which isn't ideal but acceptable in
// this scope especially since its so rare and the worst case would be that user has to submit form once
// more to get the updated (other) error message (that he couldn't see before)
displayValidationErrorMessage(fieldName, fieldMessages[0], domFieldId);
// Flash error message with details
errorMsg += error.message + '.<br>Field "<b>' + error.field.replace(/[^a-zA-Z0-9 ]/g, ' ')
errorMsg += fieldMessages[0] + '.<br>Field "<b>' + fieldName.replace(/[^a-zA-Z0-9 ]/g, ' ')
+ '</b>".<br>';
}
noFlashMessage = true;
} else {
// Default error message when server returns 422 but not json
errorMsg = 'Validation error. Something could not have been validate on the server.';
errorMsg = 'Validation error. Something could not be validated on the server.';
}
}

Expand Down
5 changes: 4 additions & 1 deletion public/assets/general/validation/form-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export function displayValidationErrorMessage(fieldName, errorMessage, domFieldI
field = document.querySelector(`[name="${fieldName}"]`);
}
if (field === null) {
// Contenteditable field
// Contenteditable field accessed with data-name
field = document.querySelector(`[data-name="${fieldName}"]`);
}
// console.log(domFieldId, field);
Expand All @@ -29,6 +29,9 @@ export function displayValidationErrorMessage(fieldName, errorMessage, domFieldI
}
// Remove any existing message in case there was one
// (this is an additional for when this function is called not from the handleFail() that removes previous error msg)
// If there are multiple error messages for a single field, the previous one is simply replaced by the newer one
// which isn't ideal but acceptable in this scope especially since its so rare and worst case would be that user
// has to submit form one more time to get the updated (other) error message (that he couldn't see before)
field.parentNode.querySelector('strong.err-msg')?.remove();
field.insertAdjacentHTML('afterend', `<strong class="err-msg">${errorMessage}</strong>`);
let label = field.parentNode.querySelector('label');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use App\Domain\Factory\LoggerFactory;
use App\Domain\Security\Exception\SecurityException;
use App\Domain\User\Service\UserFinder;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as ServerRequest;
Expand Down Expand Up @@ -75,7 +75,7 @@ public function __invoke(ServerRequest $request, Response $response): Response
}

return $this->responder->redirectToRouteName($response, 'home-page', [], $themeQueryParams);
} catch (ValidationException $ve) {
} catch (ValidationExceptionOld $ve) {
return $this->responder->renderOnValidationError(
$response,
'authentication/login.html.php',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use App\Domain\Authentication\Exception\InvalidTokenException;
use App\Domain\Authentication\Service\PasswordChanger;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as ServerRequest;
Expand Down Expand Up @@ -83,7 +83,7 @@ public function __invoke(ServerRequest $request, Response $response): Response

// Render password
return $this->responder->render($response, 'authentication/login.html.php');
} catch (ValidationException $validationException) {
} catch (ValidationExceptionOld $validationException) {
$flash->add('error', $validationException->getMessage());
// Add token and id to php view attribute like PasswordResetAction does
$this->responder->addPhpViewAttribute('token', $parsedBody['token']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use App\Domain\Exception\DomainRecordNotFoundException;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Security\Exception\SecurityException;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as ServerRequest;
Expand Down Expand Up @@ -62,7 +62,7 @@ public function __invoke(ServerRequest $request, Response $response): Response
'User with email ' . $userValues['email'] . ' tried to request password
reset link but account doesn\'t exist'
);
} catch (ValidationException $validationException) {
} catch (ValidationExceptionOld $validationException) {
// Form error messages set in function below
return $this->responder->renderOnValidationError(
$response,
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Actions/Client/Ajax/ApiClientCreateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use App\Application\Responder\Responder;
use App\Application\Validation\MalformedRequestBodyChecker;
use App\Domain\Client\Service\ClientCreatorFromClientSubmit;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Slim\Exception\HttpBadRequestException;
Expand Down Expand Up @@ -64,7 +64,7 @@ public function __invoke(
)) {
try {
$insertId = $this->clientCreatorFromClientSubmit->createClientFromClientSubmit($clientValues);
} catch (ValidationException $exception) {
} catch (ValidationExceptionOld $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
Expand Down
66 changes: 10 additions & 56 deletions src/Application/Actions/Client/Ajax/ClientCreateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,9 @@
namespace App\Application\Actions\Client\Ajax;

use App\Application\Responder\Responder;
use App\Application\Validation\MalformedRequestBodyChecker;
use App\Domain\Authentication\Exception\ForbiddenException;
use App\Domain\Client\Service\ClientCreator;
use App\Domain\Validation\ValidationException;
use Fig\Http\Message\StatusCodeInterface;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Slim\Exception\HttpBadRequestException;

/**
* Action.
Expand All @@ -23,14 +17,10 @@ final class ClientCreateAction
*
* @param Responder $responder The responder
* @param ClientCreator $clientCreator
* @param SessionInterface $session
* @param MalformedRequestBodyChecker $malformedRequestBodyChecker
*/
public function __construct(
private readonly Responder $responder,
private readonly ClientCreator $clientCreator,
private readonly SessionInterface $session,
private readonly MalformedRequestBodyChecker $malformedRequestBodyChecker,
) {
}

Expand All @@ -42,8 +32,6 @@ public function __construct(
* @param array $args
*
* @return ResponseInterface The response
* @throws \JsonException
*
*/
public function __invoke(
ServerRequestInterface $request,
Expand All @@ -52,51 +40,17 @@ public function __invoke(
): ResponseInterface {
$clientValues = $request->getParsedBody();

// If html form names change they have to be adapted in the data class attributes too (e.g. ClientData)
// Check that request body syntax is formatted right (tested in ClientCreateActionTest - malformedRequest)
if ($this->malformedRequestBodyChecker->requestBodyHasValidKeys(
$clientValues,
[
'client_status_id',
'user_id',
'first_name',
'last_name',
'phone',
'location',
'message',
'birthdate',
'email',
], // Html radio buttons and checkboxes are not sent over by the client if they are not set hence optional
['sex', 'client_message', 'vigilance_level']
)) {
try {
$insertId = $this->clientCreator->createClient($clientValues);
} catch (ValidationException $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
);
} catch (ForbiddenException $forbiddenException) {
return $this->responder->respondWithJson(
$response,
[
'status' => 'error',
'message' => 'Not allowed to create client.',
],
StatusCodeInterface::STATUS_FORBIDDEN
);
}

if (0 !== $insertId) {
return $this->responder->respondWithJson($response, ['status' => 'success', 'data' => null], 201);
}
$response = $this->responder->respondWithJson($response, [
'status' => 'warning',
'message' => 'Client not created',
]);
// Validation and Forbidden exception caught in respective middlewares
$insertId = $this->clientCreator->createClient($clientValues);

return $response->withAddedHeader('Warning', 'The post could not be created');
if (0 !== $insertId) {
return $this->responder->respondWithJson($response, ['status' => 'success', 'data' => null], 201);
}
throw new HttpBadRequestException($request, 'Request body malformed.');
$response = $this->responder->respondWithJson($response, [
'status' => 'warning',
'message' => 'Client not created',
]);

return $response->withAddedHeader('Warning', 'The client could not be created');
}
}
4 changes: 2 additions & 2 deletions src/Application/Actions/Client/Ajax/ClientUpdateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use App\Domain\Authentication\Exception\ForbiddenException;
use App\Domain\Client\Service\ClientUpdater;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface;
Expand Down Expand Up @@ -88,7 +88,7 @@ public function __invoke(
]);

return $response->withAddedHeader('Warning', 'The client was not updated.');
} catch (ValidationException $exception) {
} catch (ValidationExceptionOld $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Actions/Note/Ajax/NoteCreateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use App\Domain\Note\Service\NoteCreator;
use App\Domain\Note\Service\NoteFinder;
use App\Domain\User\Service\UserFinder;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use IntlDateFormatter;
use Odan\Session\SessionInterface;
Expand Down Expand Up @@ -68,7 +68,7 @@ public function __invoke(
try {
$insertId = $this->noteCreator->createNote($noteValues);
$noteDataFromDb = $this->noteFinder->findNote($insertId);
} catch (ValidationException $exception) {
} catch (ValidationExceptionOld $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Actions/Note/Ajax/NoteUpdateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use App\Domain\Authentication\Exception\ForbiddenException;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Note\Service\NoteUpdater;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface;
Expand Down Expand Up @@ -75,7 +75,7 @@ public function __invoke(
]);

return $response->withAddedHeader('Warning', 'The note was not updated.');
} catch (ValidationException $exception) {
} catch (ValidationExceptionOld $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use App\Domain\Authentication\Exception\ForbiddenException;
use App\Domain\Authentication\Service\PasswordChanger;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
Expand Down Expand Up @@ -71,7 +71,7 @@ public function __invoke(ServerRequest $request, Response $response, array $args
);

return $this->responder->respondWithJson($response, ['status' => 'success', 'data' => null]);
} catch (ValidationException $validationException) {
} catch (ValidationExceptionOld $validationException) {
return $this->responder->respondWithJsonOnValidationError(
$validationException->getValidationResult(),
$response,
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Actions/User/Ajax/UserCreateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use App\Domain\Factory\LoggerFactory;
use App\Domain\Security\Exception\SecurityException;
use App\Domain\User\Service\UserCreator;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
Expand Down Expand Up @@ -67,7 +67,7 @@ public function __invoke(ServerRequest $request, Response $response): Response
}

return $this->responder->respondWithJson($response, ['status' => 'success', 'data' => null], 201);
} catch (ValidationException $validationException) {
} catch (ValidationExceptionOld $validationException) {
return $this->responder->respondWithJsonOnValidationError(
$validationException->getValidationResult(),
$response
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Actions/User/Ajax/UserUpdateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use App\Domain\Authentication\Exception\ForbiddenException;
use App\Domain\Factory\LoggerFactory;
use App\Domain\User\Service\UserUpdater;
use App\Domain\Validation\ValidationException;
use App\Domain\Validation\ValidationExceptionOld;
use Fig\Http\Message\StatusCodeInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
Expand Down Expand Up @@ -67,7 +67,7 @@ public function __invoke(
])) {
try {
$updated = $this->userUpdater->updateUser($userIdToChange, $userValuesToChange);
} catch (ValidationException $exception) {
} catch (ValidationExceptionOld $exception) {
return $this->responder->respondWithJsonOnValidationError(
$exception->getValidationResult(),
$response
Expand Down
Loading

0 comments on commit 74fd9b0

Please sign in to comment.