diff --git a/app/models/category_list.rb b/app/models/category_list.rb index 83698d89872..aba935d8fb6 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -31,8 +31,8 @@ class CategoryList @guardian = guardian || Guardian.new @options = options - find_relevant_topics if options[:include_topics] find_categories + find_relevant_topics if options[:include_topics] prune_empty find_user_data @@ -71,17 +71,18 @@ class CategoryList private def find_relevant_topics - @topics_by_id = {} - @topics_by_category_id = {} - - category_featured_topics = CategoryFeaturedTopic.select(%i[category_id topic_id]).order(:rank) - @all_topics = - Topic.where(id: category_featured_topics.map(&:topic_id)).includes( - :shared_draft, - :category, - { topic_thumbnails: %i[optimized_image upload] }, - ) + Topic + .secured(@guardian) + .joins( + "INNER JOIN category_featured_topics ON topics.id = category_featured_topics.topic_id", + ) + .where("category_featured_topics.category_id IN (?)", categories_with_descendants.map(&:id)) + .select( + "topics.*, category_featured_topics.category_id AS category_featured_topic_category_id", + ) + .includes(:shared_draft, :category, { topic_thumbnails: %i[optimized_image upload] }) + .order("category_featured_topics.rank") @all_topics = @all_topics.joins(:tags).where(tags: { name: @options[:tag] }) if @options[ :tag @@ -103,16 +104,18 @@ class CategoryList end @all_topics = TopicQuery.remove_muted_tags(@all_topics, @guardian.user).includes(:last_poster) + + featured_topics_by_category_id = Hash.new { |h, k| h[k] = [] } + @all_topics.each do |t| # hint for the serializer t.include_last_poster = true t.dismissed = dismissed_topic?(t) - @topics_by_id[t.id] = t + featured_topics_by_category_id[t.category_featured_topic_category_id] << t end - category_featured_topics.each do |cft| - @topics_by_category_id[cft.category_id] ||= [] - @topics_by_category_id[cft.category_id] << cft.topic_id + categories_with_descendants.each do |category| + category.displayable_topics = featured_topics_by_category_id[category.id] end end @@ -213,23 +216,6 @@ class CategoryList ) category.has_children = category.subcategories.present? end - - if @topics_by_category_id - categories_with_descendants.each do |c| - topics_in_cat = @topics_by_category_id[c.id] - if topics_in_cat.present? - c.displayable_topics = [] - topics_in_cat.each do |topic_id| - topic = @topics_by_id[topic_id] - if topic.present? && @guardian.can_see?(topic) - # topic.category is very slow under rails 4.2 - topic.association(:category).target = c - c.displayable_topics << topic - end - end - end - end - end end def prune_empty diff --git a/spec/models/category_list_spec.rb b/spec/models/category_list_spec.rb index 5fedc0467c5..c5495f6c2b8 100644 --- a/spec/models/category_list_spec.rb +++ b/spec/models/category_list_spec.rb @@ -27,21 +27,21 @@ RSpec.describe CategoryList do it "doesn't show topics that you can't view" do public_cat = Fabricate(:category_with_definition) # public category - Fabricate(:topic, category: public_cat) + topic_in_public_cat = Fabricate(:topic, category: public_cat) private_cat = Fabricate(:category_with_definition) # private category - Fabricate(:topic, category: private_cat) + topic_in_private_cat = Fabricate(:topic, category: private_cat) private_cat.set_permissions(admins: :full) private_cat.save secret_subcat = Fabricate(:category_with_definition, parent_category_id: public_cat.id) # private subcategory - Fabricate(:topic, category: secret_subcat) + topic_in_secret_subcat = Fabricate(:topic, category: secret_subcat) secret_subcat.set_permissions(admins: :full) secret_subcat.save muted_tag = Fabricate(:tag) # muted tag SiteSetting.default_tags_muted = muted_tag.name - Fabricate(:topic, category: public_cat, tags: [muted_tag]) + topic_in_public_cat_2 = Fabricate(:topic, category: public_cat, tags: [muted_tag]) muted_tag_2 = Fabricate(:tag) TagUser.create!( @@ -58,16 +58,21 @@ RSpec.describe CategoryList do .categories .find { |x| x.name == public_cat.name } .displayable_topics - .count, - ).to eq(3) + .map(&:id), + ).to contain_exactly( + topic_in_public_cat.id, + topic_in_secret_subcat.id, + topic_in_public_cat_2.id, + ) + expect( CategoryList .new(Guardian.new(admin), include_topics: true) .categories .find { |x| x.name == private_cat.name } .displayable_topics - .count, - ).to eq(1) + .map(&:id), + ).to contain_exactly(topic_in_private_cat.id) expect( CategoryList @@ -75,8 +80,9 @@ RSpec.describe CategoryList do .categories .find { |x| x.name == public_cat.name } .displayable_topics - .count, - ).to eq(2) + .map(&:id), + ).to contain_exactly(topic_in_public_cat.id, topic_in_public_cat_2.id) + expect( CategoryList .new(Guardian.new(user), include_topics: true) @@ -90,8 +96,9 @@ RSpec.describe CategoryList do .categories .find { |x| x.name == public_cat.name } .displayable_topics - .count, - ).to eq(1) + .map(&:id), + ).to contain_exactly(topic_in_public_cat.id) + expect( CategoryList .new(Guardian.new(nil), include_topics: true) @@ -112,8 +119,8 @@ RSpec.describe CategoryList do .categories .find { |x| x.name == cat.name } .displayable_topics - .count, - ).to eq(1) + .map(&:id), + ).to contain_exactly(topic.id) TopicUser.change(user.id, topic.id, notification_level: TopicUser.notification_levels[:muted])