jadjoubran / laravel5-angular-material-starter

Get started with Laravel 5.3 and AngularJS (material)
https://laravel-angular.readme.io/
MIT License
1.66k stars 400 forks source link

Validate Function (in Controller.php) does not work #445

Closed aceart closed 7 years ago

aceart commented 7 years ago
       if ($validator->fails()) {
            if (Route::current()->getPrefix() === 'api') {
                $message = $validator->errors()->first();
                throw new ValidationException($message);
            } else {
                throw new ValidationException($this);
            }
        }

Firstly I think the routes prefix check should be changed to:

Route::current()->getPrefix() === 'api' || starts_with(Route::current()->getPrefix(), 'api/')

I had some issues with multiple chained api routes. For example: I had a separate api route für backend issues that had another prefix called 'backend'. So the `Route::current()->getPrefix() returned "api/backend/" which is not covered by the if-statement.

Another error occured with the throw new ValidationException($message); statement. I think it should be changed to throw new ValidationException($validator);. The ValidationException only accepts classes by the type of "Validators". A paramenter type of String raises an exception.

Beside that I did change the request header to json. But I don't know if this is necessary. I do only want to communicate over json at my api routes. I don't know if its general purpose.

Here is my suggestion. Maybe it helps :)

      if ($validator->fails()) {
            if (Route::current()->getPrefix() === 'api' || starts_with(Route::current()->getPrefix(), 'api/')) {
                $request->headers->set('Accept', 'application/json');
                throw new ValidationException($validator);
            } else {
                throw new ValidationException($validator);
            }
        }
jadjoubran commented 7 years ago

Hi @aceart Thank you so much for this! I totally agree on your first point.. I didn't think about it (starts_with) Same for the second one (passing $validator)

I don't think you'd need to set the Accept header to json

Would you be able to send a PR targeting master with the first 2 changes? Thanks!