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
434 stars 104 forks source link

Disable default unique slug generation so slug validation rule 'uniqu… #45

Closed dhildreth closed 5 years ago

dhildreth commented 6 years ago

…e' is applicable

Currently, duplicate attribute names are allowed. Looking at the rules in Attribute.php model, it appears this is not desirable because of the 'unique' rule on 'slug'.

Attribute.php:

    public function __construct(array $attributes = [])
    {
        parent::__construct($attributes);

        $this->setTable(config('rinvex.attributes.tables.attributes'));
        $this->setRules([
            'name' => 'required|string|max:150',
            'description' => 'nullable|string|max:10000',
            'slug' => 'required|alpha_dash|max:150|unique:'.config('rinvex.attributes.tables.attributes').',slug',
            'sort_order' => 'nullable|integer|max:10000000',
            'group' => 'nullable|string|max:150',
            'type' => 'required|string|max:150',
            'is_required' => 'sometimes|boolean',
            'is_collection' => 'sometimes|boolean',
            'default' => 'nullable|string|max:10000',
        ]);
    }

Since spatie/laravel-sluggable makes all slugs unique by default by appending a dash and a number to the end, this rule will never be enforced (see sluggable docs). In order to prevent this default behavior, allowDuplicateSlugs() should be called. Then, any attempt to save an attribute with an existing name will result in an error.

IsraelOrtuno commented 6 years ago

Would not be risky to have two attributes with the same slug? Considering that you can reuse the same attribute for various entities, I do not really see this needed.

Could you provide an scenario?

IsraelOrtuno commented 6 years ago

cc @dhildreth

dhildreth commented 6 years ago

The scenario that brought this to my attention was through an administration page for modifying and creating new attributes. The user would goto add a new attribute, but mistakingly recreates an existing attribute. For example, the "cpu_speed" attribute (slug = "cpu-speed") already exists but the user attempts to create it again. This should not be allowed and should alert the user via validation rules in Attribute.php. Instead, it is allowed because the slug is created as "cpu-speed-1". The validation rule

'slug' => 'required|alpha_dash|max:150|unique:'.config('rinvex.attributes.tables.attributes').',slug'

will always pass successfully as long as allowDuplicateSlugs() isn't being called.

IsraelOrtuno commented 6 years ago

But don't we agree that having 2 attributes with the same name is useless taking into consideration what I mentioned below? That they can be attached to multiple entities and reuse them.

Maybe just need to add a warning to the UI rather than blocking the creation.

What do you think @Omranic?

dhildreth commented 6 years ago

I was able to get around this issue in with my project by creating my own App\Attribute.php model that extends the original Rinvex\Attributes\Models\Attribute.php model. I still feel that this pull request is a worthwhile change for others. If not, you may as well remove the validation rule because it's not going to accomplish anything. I know it sounds trivial, but I did spend quite a bit of time debugging this one for my use case.

IsraelOrtuno commented 6 years ago

I do not really understand your issue at all, you can use the same attribute for multiple entities, isn't that what you want?

IsraelOrtuno commented 5 years ago

Info not provided