mongodb / laravel-mongodb

A MongoDB based Eloquent model and Query builder for Laravel (Moloquent)
https://www.mongodb.com/docs/drivers/php/laravel-mongodb/
MIT License
7.01k stars 1.43k forks source link

Execution order of getMutators #973

Closed AmmarRahman closed 6 years ago

AmmarRahman commented 8 years ago

When Having a getMutator and a relation of the same name, standard Eloquent behaviour would prioritise the mutator. However, when the relation is embedOneOrMany, the relation would have the priority over the mutator.

Calling $model->foo on the following would return an ordered collection

public function getFooAttribute(){
//this will never be called
        return $this->foo()->sortBy('created_at','desc');
    }
public function 
        return $this->embedsMany('Foo');
    }

However it would have a different behaviour for other relations

pi0 commented 8 years ago

This is not a library related issue
@iceheat You may use Eloquent Query Scopes. (They are fully supported by laravel-mongodb One simple approach is applying a global scope to Foo model, so every query on it (thus any relation) will return sorted form :)
App\Foo.php

class Foo extends Moloquent
{

    protected static function boot()
    {
        parent::boot();
        static::addGlobalScope('sorted_scope', function (Builder $builder) {
            $builder->orderBy('created_at', 'desc');
        });
    }
AmmarRahman commented 8 years ago

@pi0 Thank you for the reply. Query Scopes are a good tool, but in the specific case I have more than query adjustment to be done. I did track down the problem to Mongodb\Eloquent\Model@getAttribute. The problem is that the method searches for relationships before it passes to parent:getAtttribute(). Only the parent method checks for hasGetMutator($key).

My current hack is to add

   public function getAttribute($key)
    {
        // Check for get mutator first
        return $this->hasGetMutator($key) 
            ? $this->getAttributeValue($key)
            : parent::getAttribute($key);
    }

to my BaseModel class

pi0 commented 8 years ago

@iceheat Thanks for your nice discovery :) Your workaround is applied on Moloquent. (a36bc4e)

AmmarRahman commented 8 years ago

@pi0 I had a look at your commit. Unfortunately, this wouldn't solve the problem since it is run at the end of the method. hasGetMutator should run before checking for embedded relations.

pi0 commented 8 years ago

@iceheat Please checkout 6a536f67277927e5a778ca1fe4ab108d079977ec ;)

AmmarRahman commented 8 years ago

That looks perfect!! Don't you think overriding the parent method makes more sense than extending it in this case?

pi0 commented 8 years ago

@iceheat Would you please explain more about that ? ( or opening PR on Moloquent )