laravel-ardent / ardent

Self-validating, secure and smart models for Laravel's Eloquent ORM
BSD 3-Clause "New" or "Revised" License
1.39k stars 211 forks source link

Laravel 5 - Using form requests #254

Open simplenotezy opened 9 years ago

simplenotezy commented 9 years ago

After Laravel 5 introduces form requests (see: https://laracasts.com/series/whats-new-in-laravel-5/episodes/3), I see a few overlaps concerning validation / creating.

How are you planning to play together with Laravel 5? Do you already do it?

Newcomer here, but really love Ardent.

zzantares commented 9 years ago

any comments on this?

igorsantos07 commented 8 years ago

@canfiax @ZzAntares I'm not sure about this. First of all, sorry for taking forever to reply. I've been idle from this project, but I'm getting back to work.

I'm using to have validations tied between forms, requests and models. By this laracast you linked, it seems we would have models as simply database mirrors? If we just add validation to the form, a random developer can add invalid data in another action, for instance. On the other hand, if validation is happening only on the model, we would need to tie it manually to the request.

Quick thinking, I think the second one is cleaner?

igorsantos07 commented 8 years ago

Or maybe we could have some sort of ArdentFormRequest that understand its model's rules?

igorsantos07 commented 8 years ago

I "solved" this in my current project with the following class:

<?php namespace App\Http\Requests;

/**
 * Model requests automatically discover their model rules given their classnames.
 * Models are autoloaded from \App\Models namespace.
 * @package App\Http\Requests
 */
abstract class Model extends Request {

    public function rules() {
        /** @var \LaravelArdent\Ardent\Ardent $model */
        $model = '\App\Models\\'.class_basename(static::class);
        return $model::$rules;
    }

}

For each model, you can simply create a class User extends ModelRequest { } and it would be enough, I guess.

The way I thought about including this in the Ardent package is as follows:

  1. including a public static property Ardent::modelNamespace;
  2. using this static property to find the complete namespace for the model inside that rules() method;
  3. this way I can include this FormRequest in Ardent, and we can create those empty classes from model names when needed.

What do you think, @canfiax, @ZzAntares?

zzantares commented 8 years ago

Looks good, but lately Laravel is like speaking and class names are very descriptive, I think is cooler to write it like class StoreUserRequest extends StoreModelRequest but then another for Update would be needed, but however you like is ok, functionality is the important thing.

Just wondering why does it need to be an abstract class? And also this can't be done using traits? instead of having to create a ModelRequest one could just simply use the trait, however is just a thought, I don't know technical implications.

igorsantos07 commented 8 years ago

I don't get much of the idea of repeating namespace in the classname, such as Requests\ModelRequest. That's what namespaces are for :) However, that poses some trouble when you need different rules for different forms but still using the same model.

My best approach for now is something like this:

class UserSignupReq extends App\Http\Request {
    public function rules() {
        $rules = App\Model\User::$rules;
        // patch the rules as needed, or use "return App\Model\User::$rules" directly
        // todo: need to see how to support unique rules correctly.
        //       Probably buildUniqueExclusionRules() would need a new static
        //       version, that attaches a parametrized ID to the unique rules
        return $rules;
    }
}

Indeed, I don't think why it could not be done as a Trait. If we end up by really including it in the Ardent core, it could be a Trait. However, I'm leaning towards only documenting the approach above in the README. Any final thoughts? :)