jarektkaczyk / eloquence

Extensions for the Eloquent ORM
http://softonsofa.com
MIT License
1.09k stars 141 forks source link

Eloquence Model doesn't work well with Validable? #9

Closed ziming closed 9 years ago

ziming commented 9 years ago

When my model code is this, seeding doesn't work

<?php namespace App;

use Sofa\Eloquence\Model;
use Sofa\Eloquence\Mutable;

class LetterType extends Model {

    use Mutable;

    protected $fillable = ['title', 'acronym', 'description'];

    /**
     * Attributes getter mutators @ Eloquence\Mutable
     *
     * @var array
     */
    protected $getterMutators = [

    ];

    /**
     * Attributes setter mutators @ Eloquence\Mutable
     *
     * @var array
     */
    protected $setterMutators = [
        'acronym' => 'trim',
    ];

    protected static $rules = [
        'title'  => 'required',
        'acronym' => 'required',
    ];
}

Seeding only works when I change to:

<?php namespace App;

use Illuminate\Database\Eloquent\Model;
use Sofa\Eloquence\Contracts\CleansAttributes;
use Sofa\Eloquence\Contracts\Validable as ValidableContract;
use Sofa\Eloquence\Eloquence;
use Sofa\Eloquence\Mappable;
use Sofa\Eloquence\Mutable;
use Sofa\Eloquence\Validable;

class LetterType extends Model implements ValidableContract, CleansAttributes {

    use Eloquence, Mutable, Validable;

    protected $fillable = ['title', 'acronym', 'description'];

    /**
     * Attributes getter mutators @ Eloquence\Mutable
     *
     * @var array
     */
    protected $getterMutators = [

    ];

    /**
     * Attributes setter mutators @ Eloquence\Mutable
     *
     * @var array
     */
    protected $setterMutators = [
        'acronym' => 'trim|strtoupper',
    ];

    /**
     * Rules for field attributes
     * Available rules as described in the laravel docs:
     * @link http://laravel.com/docs/5.0/validation#available-validation-rules
     */
    public static $rules = [
        'title'  => 'required',
        'acronym' => 'required',

    ];

}

This is strange to me because for my User model, extending Sofa\Eloquence\Model gives me no issue in seeding. But for my LetterType model, it does.

bobbybouwmann commented 9 years ago

What error do you get? Any other relevant information?

ziming commented 9 years ago

@bobbybouwmann it will say Seeded:... in console but in reality nothing got seed for that table. laravel.log has nothing too.

I just tried with a new model, it doesn't work too unless I use Eloquence and Mutable traits instead of extends Sofa\Eloquence\Model and use Mutable trait. When I look at the source code Sofa\Eloquence\Model uses Eloquence trait hence that makes it confusing for me.

User model is the only exception so far.

bobbybouwmann commented 9 years ago

You don't exstend the LetterType class with Eloquence, I think the issue is there

This:

use Illuminate\Database\Eloquent\Model;
use Sofa\Eloquence\Eloquence;
use Sofa\Eloquence\Mutable;

class LetterType extends Model {

Should be this:

use Sofa\Eloquence\Model;
use Sofa\Eloquence\Eloquence;
use Sofa\Eloquence\Mutable;

class LetterType extends Model {
ziming commented 9 years ago

Hi @bobbybouwmann in my example, seeding fails when I extends Sofa\Eloquence\Model. but it works when I extends Illuminate\Database\Eloquent\Model and use the validable traits and interface instead.

ziming commented 9 years ago

Hi further investigation shows to me that it is Validable that's causing the problem.

I tested this by creating an AppModel abstract class and extends it from my other models.

<?php

namespace App;

use Illuminate\Database\Eloquent\Model;
use Sofa\Eloquence\Contracts\CleansAttributes;
use Sofa\Eloquence\Contracts\Validable as ValidableContract;
use Sofa\Eloquence\Eloquence;
use Sofa\Eloquence\Mappable;
use Sofa\Eloquence\Mutable;
use Sofa\Eloquence\Validable;

abstract class AppModel extends Model implements ValidableContract, CleansAttributes
{
    use Eloquence, Mutable, Mappable, Validable;

}

When I remove ValidableContract, CleansAttributes and Validable, from the super class then it works.

I don't think it is an issue with my validation rules if any because when my concrete classes implements ValidableContract, CleansAttributes and use the Validable trait, seeding works fine. It just fails if these are in my superclass instead.

jarektkaczyk commented 9 years ago

@ziming @bobbybouwmann I'm on holiday, I will take a look at this in a few days. If you can do it sooner, then pls do. Thanks

bobbybouwmann commented 9 years ago

@jarektkaczyk I don't have much time this week, so I can't take a look at it for now. I bet you will be back sooner then I am ;)

ziming commented 9 years ago

Update, I notice problems again this time when I use middleware. Got to implement the contracts and traits in the class itself instead of extending the super class for it to work again. Hope it helps!.

jarektkaczyk commented 9 years ago

@ziming I've tested that on L5.0 and nothing bad happens. Seeding works as expected, so if the problem persists, please upload whole code so I can see it.

ziming commented 9 years ago

I am using laravel 5.1 and mostbof the time the issue happen when using it with laravel administrator. I'm outside now so I can't test it right now. Sometimes the problem is solved by running php artisan optimize. Sometimes isn't. Thank you.