From b9d411a4ebd0fa84855e9a68af9555a4c73c678a Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Sat, 4 Apr 2020 13:31:34 +0300 Subject: [PATCH] FIX: Topic.time_to_first_response should include sub-sub-categories (#9349) --- app/models/category.rb | 19 +++++++++++++++++++ app/models/topic.rb | 11 +++-------- lib/topic_query.rb | 20 +------------------- spec/models/topic_spec.rb | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 6e0a073a6bb..8a29f612508 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -153,6 +153,25 @@ class Category < ActiveRecord::Base Category.reset_topic_ids_cache end + def self.subcategory_ids(category_id) + sql = <<~SQL + WITH RECURSIVE subcategories AS ( + SELECT :category_id id, 1 depth + UNION + SELECT categories.id, (subcategories.depth + 1) depth + FROM categories + JOIN subcategories ON subcategories.id = categories.parent_category_id + WHERE subcategories.depth < :max_category_nesting + ) + SELECT id FROM subcategories + SQL + DB.query_single( + sql, + category_id: category_id, + max_category_nesting: SiteSetting.max_category_nesting + ) + end + def self.scoped_to_permissions(guardian, permission_types) if guardian.try(:is_admin?) all diff --git a/app/models/topic.rb b/app/models/topic.rb index a50c35dcb88..7a3a1a5b0b0 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -189,11 +189,6 @@ class Topic < ActiveRecord::Base where("topics.category_id IS NULL OR topics.category_id IN (SELECT id FROM categories WHERE #{condition[0]})", condition[1]) } - IN_CATEGORY_AND_SUBCATEGORIES_SQL = <<~SQL - t.category_id = :category_id - OR t.category_id IN (SELECT id FROM categories WHERE categories.parent_category_id = :category_id) - SQL - scope :in_category_and_subcategories, lambda { |category_id| where("topics.category_id = ? OR topics.category_id IN (SELECT id FROM categories WHERE categories.parent_category_id = ?)", category_id, @@ -1300,7 +1295,7 @@ class Topic < ActiveRecord::Base builder = DB.build(sql) builder.where("t.created_at >= :start_date", start_date: opts[:start_date]) if opts[:start_date] builder.where("t.created_at < :end_date", end_date: opts[:end_date]) if opts[:end_date] - builder.where(IN_CATEGORY_AND_SUBCATEGORIES_SQL, category_id: opts[:category_id]) if opts[:category_id] + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id])) if opts[:category_id] builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.where("p.deleted_at IS NULL") @@ -1339,7 +1334,7 @@ class Topic < ActiveRecord::Base builder = DB.build(WITH_NO_RESPONSE_SQL) builder.where("t.created_at >= :start_date", start_date: start_date) if start_date builder.where("t.created_at < :end_date", end_date: end_date) if end_date - builder.where(IN_CATEGORY_AND_SUBCATEGORIES_SQL, category_id: category_id) if category_id + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(category_id)) if category_id builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.query_hash @@ -1359,7 +1354,7 @@ class Topic < ActiveRecord::Base def self.with_no_response_total(opts = {}) builder = DB.build(WITH_NO_RESPONSE_TOTAL_SQL) - builder.where(IN_CATEGORY_AND_SUBCATEGORIES_SQL, category_id: opts[:category_id]) if opts[:category_id] + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id])) if opts[:category_id] builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.query_single.first.to_i diff --git a/lib/topic_query.rb b/lib/topic_query.rb index f77d37e7464..24a35b021e2 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -1054,25 +1054,7 @@ class TopicQuery def subcategory_ids(category_id) @subcategory_ids ||= {} - @subcategory_ids[category_id] ||= - begin - sql = <<~SQL - WITH RECURSIVE subcategories AS ( - SELECT :category_id id, 1 depth - UNION - SELECT categories.id, (subcategories.depth + 1) depth - FROM categories - JOIN subcategories ON subcategories.id = categories.parent_category_id - WHERE subcategories.depth < :max_category_nesting - ) - SELECT id FROM subcategories - SQL - DB.query_single( - sql, - category_id: category_id, - max_category_nesting: SiteSetting.max_category_nesting - ) - end + @subcategory_ids[category_id] ||= Category.subcategory_ids(category_id) end def sanitize_sql_array(input) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 525541e4147..84df06dd0d9 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -2348,6 +2348,20 @@ describe Topic do expect(Topic.time_to_first_response_total).to eq(1) end + it "should have results if there's a topic with replies" do + SiteSetting.max_category_nesting = 3 + + category = Fabricate(:category_with_definition) + subcategory = Fabricate(:category_with_definition, parent_category_id: category.id) + subsubcategory = Fabricate(:category_with_definition, parent_category_id: subcategory.id) + + topic = Fabricate(:topic, category: subsubcategory, created_at: 3.hours.ago) + Fabricate(:post, topic: topic, user: topic.user, post_number: 1, created_at: 3.hours.ago) + Fabricate(:post, topic: topic, post_number: 2, created_at: 2.hours.ago) + + expect(Topic.time_to_first_response_total(category_id: category.id)).to eq(1) + end + it "should only count regular posts as the first response" do topic = Fabricate(:topic, created_at: 5.hours.ago) Fabricate(:post, topic: topic, user: topic.user, post_number: 1, created_at: 5.hours.ago)