jarektkaczyk / eloquence

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

Compatibility with Fractal #186

Closed mmaedler closed 7 years ago

mmaedler commented 7 years ago

Hi there,

I really appreciate eloquence — so thanks for providing it! However, I came across one thing when using Mappable in conjunction with Fractal (https://github.com/thephpleague/fractal) in my Lumen API: When calling the toArray() Method on my model, the mapped fields aren't returned. I'm sure I'm doing something wrong here so I'd appreciate a hint into the right direction.

My code looks something like this

class TestModel extends Model implements ... {
   use Eloquence, Mappable;
   protected $maps = [
       'mapped_field' => 'original_field',
       ...
   ]
}

class TestTransformer extends Fractal\TransformerAbstract {
   public function transform (TestModel $test) {
      return $test->toArray();
   }
}

This would return something like

array(original_field => "value");
// instead of 
array(mapped_field => "value");

Thanks!

mmaedler commented 7 years ago

An additional observation I made today: When querying the database through the above mentioned TestModel like so:

TestModel::where('mapped_field', '=', 'testvalue');

It throws an error:

SQLSTATE[42S22]: 
Column not found: 1054 Unknown column 'mapped_field' in 'where clause' 
(SQL: select * from `test` where `mapped_field` = testvalue limit 1)

Am I doing something wrong here? According to the Wiki this should work...

uniconstructor commented 7 years ago

Good idea, @mmaedler ! One thing about it: Fractal is already have a good Laravel adapter, you can find it here: https://github.com/spatie/laravel-fractal If you want set up compatibility between Fractal and Eloquence please consider to use laravel-fractal package instead of "vanilla" Fractal.

mmaedler commented 7 years ago

Thanks for your reply @uniconstructor! Just to be clear here — the problem is less about the integration with Fractal, but more about my second comment around why I couldn't use the mapped fields in the where-function of the model. I think once that is solved, the other (initial) issue is also an easy one...

uniconstructor commented 7 years ago

@mmaedler I have an idea about your second comment: maybe you are attached traits in a wrong order. Documentation have a few words about it:

Eloquence hooks on the Model in order to apply enhancements to the core methods.

It lets you use addons as a stack (custom implementation - Pipeline similar to the one used in the Laravel core by middlewares). It evaluates the addons (traits) in either FIFO or LIFO manner, depending on the method, so the order of the traits matters. I just launched tinker for my model and I cannot reproduce your bug with "where":

TestModel::where('mapped_field', '=', 'testvalue');

Here is example:

My model:

class Partner extends Model implements ValidableContract, CleansAttributes
{
    /**
     * @see https://github.com/jarektkaczyk/eloquence/wiki/Base
     */
    use Eloquence, Mappable, Metable, Mutable, Validable; // that order of traits used in all my models
    protected $maps = [
        // old primary key - renamed now
        'partner_id' => 'id',
    ];
    protected $appends = [
        // maybe it's not needed for the proper where() request handling, but I need to keep my old pk for frontend scripts
        'partner_id',
    ];
}

IMPORTANT: placing Mappable AFTER Metable will not work! (gives null when searching by mappable field)

Trying the request:

php artisan tinker
$p = \App\Models\Partner::where('partner_id', '=', '1')->first();
=> App\Models\Partner {#1877
     id: 1,
     name: "test",
     created_at: null,
     updated_at: "2017-06-14 22:19:50",
     deleted_at: null,
   }
$p->toArray();
=> [
     "id" => 1,
     "name" => "test",
     "created_at" => null,
     "updated_at" => "2017-06-14 22:19:50",
     "deleted_at" => null,
     "partner_id" => 1,
   ]
uniconstructor commented 7 years ago

@mmaedler about your second question:

array(original_field => "value");
// instead of 
array(mapped_field => "value");

You can have this behavior by setting original field to $hidden array and mapped_field to $appends array.

    /**
     * @inheritDoc
     * @see https://laravel.com/docs/5.3/eloquent-serialization#appending-values-to-json
     */
    protected $appends = [
        'mapped_field',
    ];
    protected $hidden = ['original field'];

Check out Laravel documentation about it: https://laravel.com/docs/5.3/eloquent-serialization#appending-values-to-json

mmaedler commented 7 years ago

Thanks again! Weird — my setup looks exactly the same (except I'm not using the traits Metable, Mutable and Validable). However, when doing the following:

TestModel::where('mapped_field', '=', 'testvalue');

it fails with the unkown column error. However, if I query the test model when its part of a relation:

TestParentModel->testModel->where('mapped_field', '=', 'testvalue')->get();

it works just fine.

mmaedler commented 7 years ago

Just a quick ping on this one — anybody has a solution/idea yet?

jarektkaczyk commented 7 years ago

@mmaedler should work, please provide your setup - push your code to a repo, so I can reproduce the error.