lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 124 forks source link

validateAttributes not considering required_if with enum #201

Closed artworkad closed 2 years ago

artworkad commented 2 years ago

Hello!

I am having an issue with required_if validation and enums. I want to require title_other if title is set to NoteTopic::OTHER->value. The enum is backed by int, so NoteTopic::OTHER->value would be 16.

enum NoteTopic: int
{
    // ...
    case OTHER = 16;
}

However validation does not work for title_other. The field is never required. Even if I change the rule directly to 'title_other' => 'required_if:title,16'. All the other rules work fine.

class UpdateNote
{
    use AsAction;
    use WithAttributes;

    public function rules(): array
    {
        return [
            'note.title' => ['required', new Enum(NoteTopic::class)],
            'note.title_other' => 'required_if:note.title,' .NoteTopic::OTHER->value,
            'note.date' => 'required|date_format:Y-m-d',
            'note.text' => 'required',
        ];
    }

    public function handle(Note $note, array $attributes = []): Note
    {
        // ...

        $this->set('note', $note)->fill($attributes);

        $this->validateAttributes();

        //...

        return $note;
    }
}

Since that validation works in vanilla laravel I guess the issue may be somewhere in WithAttributes. Inside WithAttributes I logged the rules:

array:4 [▼
  "note.title" => array:2 [▼
    0 => "required"
    1 => Illuminate\Validation\Rules\Enum {#2257 ▼
      #type: "App\Enums\NoteTopic"
    }
  ]
  "note.title_other" => "required_if:note.title,16"
  "note.date" => "required|date_format:Y-m-d"
  "note.text" => "required"
]

and validation data:

array:1 [▼
  "note" => App\Models\Note {#2195 ▼
    #connection: "mysql"
    #table: "notes"
    #primaryKey: "id"
    #keyType: "int"
    +incrementing: true
    #with: []
    #withCount: []
    +preventsLazyLoading: false
    #perPage: 15
    +exists: true
    +wasRecentlyCreated: false
    #escapeWhenCastingToString: false
    #attributes: array:10 [▼
      "id" => 451
      "user_id" => 3
      "record_id" => 70
      "title" => 16
      "title_other" => null
      "text" => "<div>asd</div>"
      "date" => "2022-10-16"
      "deleted_at" => null
      "created_at" => "2022-10-16 18:06:12"
      "updated_at" => "2022-10-16 19:22:38"
    ]
    #original: array:10 [▶]
    #changes: []
    #casts: array:2 [▶]
    #classCastCache: []
    #attributeCastCache: []
    #dates: []
    #dateFormat: null
    #appends: []
    #dispatchesEvents: []
    #observables: []
    #relations: array:1 [▶]
    #touches: array:1 [▶]
    +timestamps: true
    #hidden: []
    #visible: []
    #fillable: array:4 [▶]
    #guarded: array:1 [▶]
    #forceDeleting: false
    #oldAttributes: []
    +enableLoggingModelsEvents: true
  }
]

Everything is looking good and the validation should work. However the validation for title_other does not fire. I am using laravel 9.34 and actions 2.4.

artworkad commented 2 years ago

Repo to reproduce the issue: https://github.com/artworkad/enum-cast-validation

Test:

it('requires title_other if title is set to OTHER (Action)', function () {
    $post = Post::factory()->make([
        'title' => PostTopic::OTHER->value,
        'title_other' => null,
    ]);

    StorePostAction::run($post);
})->throws(ValidationException::class);

it('requires title_other if title is set to OTHER (FormRequest)', function () {
    $this->post('/posts/', [
        'title' => PostTopic::OTHER->value,
        'title_other' => null,
    ])->assertStatus(302);
});

Action:

class StorePostAction
{
    use AsAction;
    use WithAttributes;

    public function rules(): array
    {
        return [
            'post.title' => ['required', new Enum(PostTopic::class)],
            'post.title_other' => 'required_if:post.title,' . PostTopic::OTHER->value
        ];
    }

    public function handle(Post $post, array $attributes = []): Post
    {
        $this->set('post', $post)->fill($attributes);

        $this->validateAttributes();

        $post->save();

        return $post;
    }
}

Request:

class StorePostRequest extends FormRequest
{
    /**
     * Determine if the user is authorized to make this request.
     *
     * @return bool
     */
    public function authorize()
    {
        return true;
    }

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array<string, mixed>
     */
    public function rules()
    {
        return [
            'title' => ['required', new Enum(PostTopic::class)],
            'title_other' => 'required_if:title,' . PostTopic::OTHER->value
        ];
    }
}

Post:

class Post extends Model
{
    use HasFactory;

    protected $fillable = [
        'title',
        'title_other',
    ];

    /**
     * The attributes that should be cast.
     *
     * @var array<string, string>
     */
    protected $casts = [
        'title' => PostTopic::class
    ];
}

The action test fails. When 'title' => PostTopic::class is commented out, it passes again.

leandrodiogenes commented 2 years ago

Hi @artworkad.

I believe the error is in your validation. You are trying to validate an attribute within a model and not an array. I believe this is not supported by laravel.

it('requires title_other if title is set to OTHER (Action)', function () {
    $post = Post::factory()->make([
        'title' => PostTopic::OTHER->value,
        'title_other' => null,
    ]);

    validator(['post'=>$post],[
        'post.title' => ['required', new Enum(PostTopic::class)],
        'post.title_other' => 'required_if:post.title,' . PostTopic::OTHER->value
    ])->validate();

})->throws(ValidationException::class);

So, I suggest another approach.

it('requires title_other if title is set to OTHER (Action)', function () {
    $post = Post::factory()->make([
        'title' => PostTopic::OTHER->value,
        'title_other' => null,
    ]);

    StorePostAction::run($post);
})->throws(ValidationException::class);

class StorePostAction {
    use AsAction;
    use WithAttributes;

    public function rules(): array {
        return [
            'title'       => ['required', new Enum(PostTopic::class)],
            'title_other' => 'required_if:post.title,' . PostTopic::OTHER->value,
        ];
    }

    public function handle(Post $post, array $attributes = []): Post {
        $this->set('post', $post)->fill($attributes);
        $this->validateAttributes();
        $post->save();
        return $post;
    }
}
leandrodiogenes commented 2 years ago

You have 2 examples. Using laravel-actions and using formRequest, but you have different keys in the formRequest rules.

class StorePostAction {
    public function rules() {
        return [
            'post.title'       => ['required', new Enum(PostTopic::class)],
            'post.title_other' => 'required_if:post.title,' . PostTopic::OTHER->value,
        ];
    }
}

class StorePostRequest extends FormRequest {
    public function rules() {
        return [
            'title'       => ['required', new Enum(PostTopic::class)],
            'title_other' => 'required_if:title,' . PostTopic::OTHER->value,
        ];
    }
}
artworkad commented 2 years ago

@leandrodiogenes Thanks for your feedback!

Validation with nested attributes is supported: https://laravel.com/docs/9.x/validation#a-note-on-nested-attributes. The validation is working in general, just not for "post.title_other". Why? Shouldn't it fail for the other fields aswell?

Both

public static function rules(): array
{
    return [
        'post.title' => ['required', new Enum(PostTopic::class)],
        'post.title_other' => 'required_if:post.title,' . PostTopic::OTHER->value
    ];
}

and

 public static function rules(): array
{
    return [
        'title' => ['required', new Enum(PostTopic::class)],
        'title_other' => 'required_if:title,' . PostTopic::OTHER->value
    ];
}

work. The difference is, that the first one does not pick up post.title_other rule.

artworkad commented 2 years ago

@leandrodiogenes thanks for you help. I fixed this issue by converting post to array:

$this->set('post', $post->toArray())->fill($attributes);

The validation rules work now, even with "post." prefix.