egeriis / laravel-jsonapi

Make it a breeze to create a jsonapi.org compliant APIs with Laravel 5.
MIT License
146 stars 27 forks source link

Adds validation error handling #16

Closed ninjapenguin closed 9 years ago

ninjapenguin commented 9 years ago

MultiErrorResponse - Supports error output with multiple errors Exception\Validation - Excepts Laravel messagebag of validation fails

Models simply overload the getValidationRules to return a ruleset array (see http://laravel.com/docs/5.0/validation)

An example validation failure might look like:

{
   "errors":[
      {
         "status":400,
         "code":1040,
         "title":"Validation Fail",
         "detail":"The price must be a number.",
         "meta":{
            "field":"price"
         }
      },
      {
         "status":400,
         "code":1040,
         "title":"Validation Fail",
         "detail":"The ingredients field is required.",
         "meta":{
            "field":"ingredients"
         }
      }
   ]
}
egeriis commented 9 years ago

Thank you for another pull request!

I think that an improvement to this, we could utilise the Laravel 5 Form Request validation. If you're not familiar with this concept, you can read about it here: http://laravel.com/docs/5.0/validation#form-request-validation

It sets up a framework for handling validation errors. Although they name it form request validation, I believe it could be used here and provide a better structure.

There is also a video at Laracasts about this topic: https://laracasts.com/series/laravel-5-fundamentals/episodes/12

Let me know what you think.

Also keep in mind the original exception handling convention; any method can throw a EchoIt\JsonApi\Exception which should be handled by the controller.

ninjapenguin commented 9 years ago

Hi

Thanks for the feedback, I am actually familiar with the form validation but wasn't sure it was really suitable here - I'll have another look at it though!

With regards to the original concept, EchoIt\JsonApi\Exception\Validation extends EchoIt\JsonApi\Exception so the original catch in the ApiController will still catch that exception (based on PHP inheritance model)

Thanks very much

egeriis commented 9 years ago

I didn't notice that EchoIt\JsonApi\Exception\Validation was extending the Exception class. That's great. I was just taking notice of the added layer of error responding in MultiErrorResponse. Can you elaborate on the purpose of adding this layer instead of handling all errors in the controllers?

ninjapenguin commented 9 years ago

Certainly, The key difference here is between ErrorResponse and MultiErrorResponse, these classes represent the encapsulation of the logic for the presenting of the exception as a JSONApi JSON response. This again extends from the Laravel JsonResponse class and so maintains the )already existing) separation of concerns within the package

Hope that helps!