diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index b65d54ae0c9..a0bb276fde0 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -25,51 +25,34 @@ class TagsController < ::ApplicationController
respond_to do |format|
format.html do
- tag_counts = self.class.tags_by_count(guardian, limit: 300).count(Tag::COUNT_ARG)
- @tags = self.class.tag_counts_json(tag_counts)
render :index
end
format.json do
if SiteSetting.tags_listed_by_group
- # TODO: performance is bad
- grouped_tag_counts = TagGroup.order('name ASC').preload(:tags).map do |tag_group|
- h = Tag.tags_by_count_query(limit: 300)
- .joins("LEFT JOIN tag_group_memberships ON tags.id = tag_group_memberships.tag_id")
- .where('tag_group_memberships.tag_group_id = ?', tag_group.id)
- .count(Tag::COUNT_ARG)
- { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(h) }
+ grouped_tag_counts = TagGroup.order('name ASC').includes(:tags).map do |tag_group|
+ { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(tag_group.tags) }
end
- ungrouped_tag_counts = guardian.filter_allowed_categories(
- Tag.tags_by_count_query(limit: 300)
- .where("tags.id NOT IN (select tag_id from tag_group_memberships)")
- .count(Tag::COUNT_ARG)
- )
+ ungrouped_tags = Tag.where("tags.id NOT IN (select tag_id from tag_group_memberships)")
render json: {
- tags: self.class.tag_counts_json(ungrouped_tag_counts), # tags that don't belong to a group
+ tags: self.class.tag_counts_json(ungrouped_tags), # tags that don't belong to a group
extras: { tag_groups: grouped_tag_counts }
}
else
- unrestricted_tag_counts = guardian.filter_allowed_categories(
- Tag.tags_by_count_query(limit: 300)
- .where("tags.id NOT IN (select tag_id from category_tags)")
- .count(Tag::COUNT_ARG)
- )
+ unrestricted_tags = Tag.where("tags.id NOT IN (select tag_id from category_tags)")
categories = Category.where("id in (select category_id from category_tags)")
.where("id in (?)", guardian.allowed_category_ids)
- .preload(:tags)
+ .includes(:tags)
category_tag_counts = categories.map do |c|
- h = Tag.category_tags_by_count_query(c, limit: 300).count(Tag::COUNT_ARG)
- h.merge!(c.tags.where.not(name: h.keys).inject({}) { |sum, t| sum[t.name] = 0; sum }) # unused tags
- { id: c.id, tags: self.class.tag_counts_json(h) }
+ { id: c.id, tags: self.class.tag_counts_json(c.tags) }
end
render json: {
- tags: self.class.tag_counts_json(unrestricted_tag_counts),
+ tags: self.class.tag_counts_json(unrestricted_tags),
extras: { categories: category_tag_counts }
}
end
@@ -160,7 +143,7 @@ class TagsController < ::ApplicationController
category = params[:categoryId] ? Category.find_by_id(params[:categoryId]) : nil
tags_with_counts = DiscourseTagging.filter_allowed_tags(
- Tag.tags_by_count_query(params.slice(:limit)),
+ Tag.order('topic_count DESC').limit(params[:limit]),
guardian,
for_input: params[:filterForInput],
term: params[:q],
@@ -168,7 +151,7 @@ class TagsController < ::ApplicationController
selected_tags: params[:selected_tags]
)
- tags = tags_with_counts.count(Tag::COUNT_ARG).map { |t, c| { id: t, text: t, count: c } }
+ tags = self.class.tag_counts_json(tags_with_counts)
json_response = { results: tags }
@@ -211,12 +194,8 @@ class TagsController < ::ApplicationController
raise Discourse::NotFound unless SiteSetting.tagging_enabled?
end
- def self.tags_by_count(guardian, opts = {})
- guardian.filter_allowed_categories(Tag.tags_by_count_query(opts))
- end
-
- def self.tag_counts_json(tag_counts)
- tag_counts.map { |t, c| { id: t, text: t, count: c } }
+ def self.tag_counts_json(tags)
+ tags.map { |t| { id: t.name, text: t.name, count: t.topic_count } }
end
def set_category_from_params
diff --git a/app/jobs/scheduled/ensure_db_consistency.rb b/app/jobs/scheduled/ensure_db_consistency.rb
index c86e5c31996..a4826316214 100644
--- a/app/jobs/scheduled/ensure_db_consistency.rb
+++ b/app/jobs/scheduled/ensure_db_consistency.rb
@@ -15,6 +15,7 @@ module Jobs
Badge.ensure_consistency!
CategoryUser.ensure_consistency!
UserOption.ensure_consistency!
+ Tag.ensure_consistency!
end
end
end
diff --git a/app/models/tag.rb b/app/models/tag.rb
index 7d5b4f4139a..2e8a2c66944 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -31,9 +31,24 @@ class Tag < ActiveRecord::Base
q
end
- def self.category_tags_by_count_query(category, opts = {})
- tags_by_count_query(opts).where("tags.id in (select tag_id from category_tags where category_id = ?)", category.id)
- .where("topics.category_id = ?", category.id)
+ def self.ensure_consistency!
+ update_topic_counts # topic_count counter cache can miscount
+ end
+
+ def self.update_topic_counts
+ Category.exec_sql <<~SQL
+ UPDATE tags t
+ SET topic_count = x.topic_count
+ FROM (
+ SELECT COUNT(topics.id) AS topic_count, tags.id AS tag_id
+ FROM tags
+ LEFT JOIN topic_tags ON tags.id = topic_tags.tag_id
+ LEFT JOIN topics ON topics.id = topic_tags.topic_id AND topics.deleted_at IS NULL
+ GROUP BY tags.id
+ ) x
+ WHERE x.tag_id = t.id
+ AND x.topic_count <> t.topic_count
+ SQL
end
def self.top_tags(limit_arg: nil, category: nil, guardian: nil)
diff --git a/app/views/tags/index.html.erb b/app/views/tags/index.html.erb
index 41f18eeb7b7..cc250d46849 100644
--- a/app/views/tags/index.html.erb
+++ b/app/views/tags/index.html.erb
@@ -1,24 +1,3 @@
-<% if crawler_layout? %>
-
-<% content_for :title do %><%=t "tags.title" %> - <%= SiteSetting.title %><% end %>
-
-<%=t "tags.title" %>
-
-
-
-<% end %>
-
<% content_for :head do %>
<%= raw crawlable_meta_data(title: @title, description: @description_meta) %>
<% end %>
diff --git a/test/javascripts/acceptance/tags-test.js.es6 b/test/javascripts/acceptance/tags-test.js.es6
index ca7a745a94c..d048154d114 100644
--- a/test/javascripts/acceptance/tags-test.js.es6
+++ b/test/javascripts/acceptance/tags-test.js.es6
@@ -8,4 +8,57 @@ QUnit.test("list the tags", assert => {
assert.ok($('body.tags-page').length, "has the body class");
assert.ok(exists('.tag-eviltrout'), "shows the evil trout tag");
});
-});
\ No newline at end of file
+});
+
+acceptance("Tags listed by group", {
+ loggedIn: true,
+ settings: {
+ tags_listed_by_group: true
+ }
+});
+
+
+QUnit.test("list the tags in groups", assert => {
+ server.get('/tags', () => { // eslint-disable-line no-undef
+ return [
+ 200,
+ { "Content-Type": "application/json" },
+ {
+ "tags":[{id: 'planned', text: 'planned', count: 7}],
+ "extras": { "tag_groups": [
+ {id: 2, name: "Ford Cars", tags: [
+ {id: 'escort', text: 'escort', count: 1},
+ {id: 'focus', text: 'focus', count: 3}
+ ]},
+ {id: 1, name: "Honda Cars", tags: [
+ {id: 'civic', text: 'civic', count: 4},
+ {id: 'accord', text: 'accord', count: 2}
+ ]},
+ {id: 1, name: "Makes", tags: [
+ {id: 'ford', text: 'ford', count: 5},
+ {id: 'honda', text: 'honda', count: 6}
+ ]}
+ ]}
+ }
+ ];
+ });
+
+ visit('/tags');
+ andThen(() => {
+ assert.equal($('.tag-list').length, 4, "shows separate lists for the 3 groups and the ungrouped tags");
+ assert.ok(
+ _.isEqual(
+ _.map($('.tag-list h3'), i => { return $(i).text(); } ),
+ ['Ford Cars', 'Honda Cars', 'Makes', 'Other Tags']
+ ),
+ 'shown in given order and with tags that are not in a group'
+ );
+ assert.ok(
+ _.isEqual(
+ _.map($('.tag-list:first .discourse-tag'), i => { return $(i).text(); }),
+ ['focus', 'escort']
+ ),
+ 'shows the tags in default sort (by count)'
+ );
+ });
+});