rtconner / laravel-tagging

Tag support for Laravel Eloquent models - Taggable Trait
MIT License
882 stars 168 forks source link

Model Config not being used properly #172

Closed DanWithams closed 5 years ago

DanWithams commented 5 years ago

tagging.php config allows the specification of the following keys;

// Model to use to store the tags in the database
'tag_model' => '...',

...

// Model to use for the relation between tags and tagged records
'tagged_model' => '...',

However these are not used consistently the \Conner\Tagging\Taggable trait which means the Taggable trait is pretty un-useable with custom Models. E.g.

/**
     * Adds a single tag
     *
     * @param string $tagName
     */
    private function addTag($tagName)
    {
        $tagName = trim($tagName);

        if(strlen($tagName) == 0) {
            return;
        }

        $tagSlug = TaggingUtility::normalize($tagName);

        $previousCount = $this->tagged()->where('tag_slug', '=', $tagSlug)->take(1)->count();
        if($previousCount >= 1) { return; }

        $tagged = new Tagged([  // This Model is the shipped-with Model, rather than the custom Model provided via the config file.
            'tag_name' => TaggingUtility::displayize($tagName),
            'tag_slug' => $tagSlug,
        ]);

        $this->tagged()->save($tagged);

        TaggingUtility::incrementCount($tagName, $tagSlug, 1);

        unset($this->relations['tagged']);

        event(new TagAdded($this, $tagSlug, $tagged));
    }

Is this an oversight or am I missing some wider picture? I am happy to make a PR with the changes if someone can let me know that \Conner\Tagging\Taggable should also me making use of the config classnames.

My proposition would be to extend the TaggingUtility to be able to create instances of config provided Models and use them throughout the require places in the Taggable trait in order to accomplish full customisation.

rtconner commented 5 years ago

Fixed in v3.2.3

rtconner commented 5 years ago

Thank you for reporting this!