search

cviebrock / eloquent-taggable

Easily add the ability to tag your Eloquent models in Laravel.
MIT License
537 stars 72 forks source link

Count of tagged items is incorrect #107

Closed marketingsignals closed 4 years ago

marketingsignals commented 4 years ago

The SQL generated within TagService::getPopularTags compiles as this:

(If you do not limit by tag entity type and use 0 for both the limit and minCount)

SELECT t.*, COUNT(t.tag_id) AS taggable_count FROM taggable_tags t LEFT JOIN taggable_taggables tt ON tt.tag_id=t.tag_id GROUP BY t.tag_id, t.name, t.normalized, t.created_at, t.updated_at ORDER BY taggable_count DESC

If you have a tag that is not used by any taggable, then you get a count of 1 rather than 0.

Should it not be updated to:

SELECT t.*, COUNT(tt.tag_id) AS taggable_count FROM taggable_tags t LEFT JOIN taggable_taggables tt ON tt.tag_id=t.tag_id GROUP BY t.tag_id, t.name, t.normalized, t.created_at, t.updated_at ORDER BY taggable_count DESC

This means you get the correct value of 0 if you get all tags and some of them don't have a tagged relation.

cviebrock commented 4 years ago

@marketingsignals hmmm ... you may be right. To confirm, can you make a PR with a failing test?

cviebrock commented 4 years ago

Closing due to no response. Feel free to re-open or comment if you are still having issues.