Too few arguments to function Rinvex/Attributes/Models/Attribute::values()

rinvex / laravel-attributes

⚠️ [ABANDONED] Rinvex Attributable is a robust, intelligent, and integrated Entity-Attribute-Value model (EAV) implementation for Laravel Eloquent, with powerful underlying for managing entity attributes implicitly as relations with ease. It utilizes the power of Laravel Eloquent, with smooth and seamless integration.

MIT License

433 stars 104 forks source link

Hi! Great package, thanks.

Stumbled onto the above error when trying to lazy load $attribute->values

Would there be any drawbacks of changing the method signature of Rinvex/Attributes/Models/Attribute::values() slightly?

Currently the method looks like this:

    /**
     * Get the entities attached to this attribute.
     *
     * @param string $value
     *
     * @return \Illuminate\Database\Eloquent\Relations\HasMany
     */
    public function values(string $value): HasMany
    {
        return $this->hasMany($value, 'attribute_id', 'id');
    }

So trying to access $attribute->values as a dynamic property throws an error because Attribute::values() expects a $value argument.

My proposed fix:

    /**
     * Get the entities attached to this attribute.
     *
     * @param string|null [$valueModel]
     *
     * @return \Illuminate\Database\Eloquent\Relations\HasMany
     */
    public function values(string $valueModel = null): HasMany
    {
        if (!$valueModel && array_has($this->attributes, 'type')) {
            $valueModel = self::getTypeModel($this->attributes['type']);
        }

        return $this->hasMany($valueModel, 'attribute_id', 'id');
    }

Thoughts ?

Thank you for your patience, I did a complete rewrite for the whole package and changed how collections & relationshs work. In the new refactor, it actually uses normal relationships, and should be easy and straightforward like default Laravel relationships. Although, that refactor is incomplete.

I'm still not happy with the overall performance (I believe we can reduce number of executed queries), if you want to check it out, see https://github.com/rinvex/laravel-attributes/tree/refactor-to-native-laravel-relationships

Currently no plans to merge that rewrite, but hopefully sometime I can get it to a stable state, improve performance and release it. Any help with that branch would be much appreciated! 🙂

rinvex / laravel-attributes

Too few arguments to function Rinvex/Attributes/Models/Attribute::values() #79