tag_list is declared an attribute

[akostadinov]

tag_list being an attribute seems to be introduced with [1]. Now whenever a list of taggables are converted to JSON, there are N+1 queries performed to fetch all tags. Actually 2 additional queries per object:

  ActsAsTaggableOn::Tagging Load (0.3ms)  SELECT `taggings`.* FROM `taggings` WHERE `taggings`.`taggable_id` = 3 AND `taggings`.`taggable_type` = 'CMS::File'
  ActsAsTaggableOn::Tag Load (0.4ms)  SELECT `tags`.* FROM `tags` INNER JOIN `taggings` ON `tags`.`id` = `taggings`.`tag_id` WHERE `taggings`.`taggable_id` = 3 AND `taggings`.`taggable_type` = 'CMS::File' AND (taggings.context = 'tags' AND taggings.tagger_id IS NULL)

To avoid this, I have to add to the model a method like this:

  def as_json(options = {})
    except = Array.wrap(options[:except]) + [:tag_list]
    super options.merge({except: except})
  end

I believe it is safer (and less unexpected) not to declare tag_list as an attribute. It is more natural to have it in includes than always being retrieved. No different from any other linked model. I don't think tag_list is more important than e.g. orders or invoices.

[1] https://github.com/mbleigh/acts-as-taggable-on/pull/887/files#diff-f3570227aa94bb5887e3cb70c6a69fb30861dbecaa1ee498153371a1e10fc314R41

[seuros]

Do you want to open a PR to fix this ?

[akostadinov]

@seuros , it probably won't be hard to fix. If it was on me I'd just remove this attribute declaration. But I have no idea it if has any usefulness for anybody. In case there are good use cases, another approach might be to hack #as_json from acts-as-taggable-on (which I don't really like).

WDYT, can we just remove it as an attribute or do you know good use cases for that?

[Adverbly]

This is especially troublesome when combined with the lack of eager-loading functionality(https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770).

I found myself needing something like the following to get certain use cases to work:

  def tag_names
    tags.loaded? ? tags&.map(&:name) : tag_list
  end

  def as_json
    super(except: :tag_list).merge(tag_list: tag_names)
  end