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

Use validation exception instead of building response in a Controller #68

Merged
merged 1 commit into from
Aug 9, 2020

Conversation

mpaden3
Copy link
Contributor

@mpaden3 mpaden3 commented Aug 6, 2020

Hi!

This PR changes the way validation response is built. Previous code relied upon a function in base Controller class that builds the validation response. That approach had a couple of problems:

  • You have to inject ValidatorInterface to every POST/PATCH route.
  • You have to check manually in every route if there are any validation errors. This forces you to repeat the same logic in every route in your application (4 lines of code).
  • you are handling error case outside JsonApiErrorHandlerEvent class.

With this PR, validate() function is added to base controller that throws ValidationExeption. Then, the logic that builds JsonApi error response is moved to JsonApiErrorHandlerEvent and ValidationErrorDocument.

This removes some unnecessary repeating of logic in every specific Controller class and ValidatorInterface dependency. Also,
it moves the rest of the logic to the right place. Original validationErrorResponse() function is not removed, but marked as deprecated to keep the bundle backward compatible.

}
}

public function getStatusCode(?int $statusCode = null): int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this parameter used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function overrides rather complicated base function from AbstractErrorDocument and it needs to have the same signature. Why does getter function have a parameter, you should ask the maintainers of woohoolabs\yin package :)
It is called by Responder class to determine error code of the request. In this case I can't see a use case where we need to change to a different error code for a validation error (it should always be 422). Still, I could always add code that returns that parameter if it is != null.

@paknahad paknahad merged commit 4092f6e into paknahad:master Aug 9, 2020
@mpaden3 mpaden3 mentioned this pull request Aug 10, 2020
@mpaden3 mpaden3 deleted the fix/validation branch August 14, 2020 17:10
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.

None yet

3 participants