FIX: Topic.time_to_first_response should include sub-sub-categories (#9349)

This commit is contained in:
Dan Ungureanu 2020-04-04 13:31:34 +03:00 committed by GitHub
parent a8002baaed
commit b9d411a4eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 27 deletions

View File

@ -153,6 +153,25 @@ class Category < ActiveRecord::Base
Category.reset_topic_ids_cache Category.reset_topic_ids_cache
end 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) def self.scoped_to_permissions(guardian, permission_types)
if guardian.try(:is_admin?) if guardian.try(:is_admin?)
all all

View File

@ -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]) 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| 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 = ?)", where("topics.category_id = ? OR topics.category_id IN (SELECT id FROM categories WHERE categories.parent_category_id = ?)",
category_id, category_id,
@ -1300,7 +1295,7 @@ class Topic < ActiveRecord::Base
builder = DB.build(sql) 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 >= :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("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.archetype <> '#{Archetype.private_message}'")
builder.where("t.deleted_at IS NULL") builder.where("t.deleted_at IS NULL")
builder.where("p.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 = 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 >= :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("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.archetype <> '#{Archetype.private_message}'")
builder.where("t.deleted_at IS NULL") builder.where("t.deleted_at IS NULL")
builder.query_hash builder.query_hash
@ -1359,7 +1354,7 @@ class Topic < ActiveRecord::Base
def self.with_no_response_total(opts = {}) def self.with_no_response_total(opts = {})
builder = DB.build(WITH_NO_RESPONSE_TOTAL_SQL) 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.archetype <> '#{Archetype.private_message}'")
builder.where("t.deleted_at IS NULL") builder.where("t.deleted_at IS NULL")
builder.query_single.first.to_i builder.query_single.first.to_i

View File

@ -1054,25 +1054,7 @@ class TopicQuery
def subcategory_ids(category_id) def subcategory_ids(category_id)
@subcategory_ids ||= {} @subcategory_ids ||= {}
@subcategory_ids[category_id] ||= @subcategory_ids[category_id] ||= Category.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
end end
def sanitize_sql_array(input) def sanitize_sql_array(input)

View File

@ -2348,6 +2348,20 @@ describe Topic do
expect(Topic.time_to_first_response_total).to eq(1) expect(Topic.time_to_first_response_total).to eq(1)
end 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 it "should only count regular posts as the first response" do
topic = Fabricate(:topic, created_at: 5.hours.ago) topic = Fabricate(:topic, created_at: 5.hours.ago)
Fabricate(:post, topic: topic, user: topic.user, post_number: 1, created_at: 5.hours.ago) Fabricate(:post, topic: topic, user: topic.user, post_number: 1, created_at: 5.hours.ago)