Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions app/models/nested_view_post_stat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ class NestedViewPostStat < ActiveRecord::Base
#
# Table name: nested_view_post_stats
#
# id :bigint not null, primary key
# direct_reply_count :integer default(0), not null
# total_descendant_count :integer default(0), not null
# created_at :datetime not null
# updated_at :datetime not null
# post_id :bigint not null
# id :bigint not null, primary key
# direct_reply_count :integer default(0), not null
# total_descendant_count :integer default(0), not null
# whisper_direct_reply_count :integer default(0), not null
# whisper_total_descendant_count :integer default(0), not null
# created_at :datetime not null
# updated_at :datetime not null
# post_id :bigint not null
#
# Indexes
#
Expand Down
1 change: 1 addition & 0 deletions assets/javascripts/discourse/components/nested-post.gjs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ export default class NestedPost extends Component {
(if @parentLineHighlighted "--parent-line-highlighted")
(if this.collapsed "nested-post--collapsed")
(if @isPinned "nested-post--pinned")
(if @post.isWhisper "nested-post--whisper")
(if @post.deleted "nested-post--deleted")
}}
>
Expand Down
7 changes: 7 additions & 0 deletions assets/stylesheets/common/nested-view.scss
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,13 @@
font-style: italic;
}

&--whisper {
.nested-post__content .cooked {
font-style: italic;
color: var(--primary-medium);
}
}

&__deleted-avatar-placeholder {
width: var(--nested-avatar-size);
height: var(--nested-avatar-size);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class AddWhisperCountsToNestedViewPostStats < ActiveRecord::Migration[7.2]
def change
add_column :nested_view_post_stats,
:whisper_direct_reply_count,
:integer,
default: 0,
null: false
add_column :nested_view_post_stats,
:whisper_total_descendant_count,
:integer,
default: 0,
null: false
end
end
26 changes: 22 additions & 4 deletions lib/discourse_nested_replies/tree_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ def initialize(topic:, guardian:)
@guardian = guardian
end

def visible_post_types
types = [Post.types[:regular], Post.types[:moderator_action]]
types << Post.types[:whisper] if guardian.user&.whisperer?
types
end

def op_post
@op_post ||= load_posts_for_tree(topic.posts.where(post_number: 1)).first
end
Expand All @@ -46,7 +52,7 @@ def load_posts_for_tree(scope)

def apply_visibility(scope)
scope = scope.unscope(where: :deleted_at)
scope = scope.where(post_type: [Post.types[:regular], Post.types[:moderator_action]])
scope = scope.where(post_type: visible_post_types)
scope
end

Expand Down Expand Up @@ -104,7 +110,7 @@ def batch_load_siblings(ancestors, sort)
topic_id: topic.id,
parent_numbers: parent_numbers,
limit: SIBLINGS_PER_ANCESTOR,
post_types: [Post.types[:regular], Post.types[:moderator_action]],
post_types: visible_post_types,
}

sibling_ids = DB.query_single(<<~SQL, **sql_params)
Expand Down Expand Up @@ -151,7 +157,7 @@ def batch_load_siblings(ancestors, sort)
# grandchildren, etc.) and returns them as a flat scope. Used when
# cap_nesting_depth is ON to flatten deep legacy threads at the last level.
def flat_descendants_scope(parent_post_number, sort:, offset: 0, limit: CHILDREN_PER_PAGE)
post_types = [Post.types[:regular], Post.types[:moderator_action]]
post_types = visible_post_types
order_expr = DiscourseNestedReplies::Sort.sql_order_expression(sort)

descendant_post_numbers =
Expand Down Expand Up @@ -201,16 +207,28 @@ def direct_reply_counts(post_numbers)
.with_deleted
.where(topic_id: topic.id)
.where(reply_to_post_number: post_numbers)
.where(post_type: visible_post_types)
.group(:reply_to_post_number)
.count
end

# Batch-load total descendant counts from the stats table.
# Returns { post_id => count } keyed by post ID (not post_number).
# For non-staff, subtracts whisper counts to avoid leaking whisper existence.
def total_descendant_counts(post_ids)
return {} if post_ids.empty?

NestedViewPostStat.where(post_id: post_ids.uniq).pluck(:post_id, :total_descendant_count).to_h
if guardian.user&.whisperer?
NestedViewPostStat
.where(post_id: post_ids.uniq)
.pluck(:post_id, :total_descendant_count)
.to_h
else
NestedViewPostStat
.where(post_id: post_ids.uniq)
.pluck(:post_id, Arel.sql("total_descendant_count - whisper_total_descendant_count"))
.to_h
end
end
end
end
43 changes: 39 additions & 4 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ module ::DiscourseNestedReplies
post_numbers = topic_view.posts.map(&:post_number)
next if post_numbers.empty?

visible_types = [Post.types[:regular], Post.types[:moderator_action]]
visible_types << Post.types[:whisper] if topic_view.guardian.user&.whisperer?

counts =
Post
.where(topic_id: topic_view.topic.id, deleted_at: nil)
.where(reply_to_post_number: post_numbers)
.where(post_type: visible_types)
.group(:reply_to_post_number)
.count

Expand Down Expand Up @@ -199,6 +203,8 @@ def reactions
# Keep nested_view_post_stats counts in sync when posts are created or deleted.
# direct_reply_count: incremented on the immediate parent only.
# total_descendant_count: incremented on ALL ancestors up the reply chain.
# whisper_* columns track the whisper subset so non-staff users see
# counts that exclude whispers (prevents leaking whisper existence).

add_model_callback(:post, :after_create) do
next if reply_to_post_number.blank?
Expand All @@ -215,28 +221,45 @@ def reactions

ancestor_ids = ancestors.map(&:id)
direct_parent_id = ancestors.find { |a| a.depth == 1 }&.id
is_whisper = post_type == Post.types[:whisper] ? 1 : 0

# Single upsert: increment total_descendant_count for all ancestors,
# direct_reply_count only for the immediate parent.
DB.exec(<<~SQL, ids: ancestor_ids, parent_id: direct_parent_id)
INSERT INTO nested_view_post_stats (post_id, direct_reply_count, total_descendant_count, created_at, updated_at)
# whisper_* columns are incremented only when the new post is a whisper.
DB.exec(<<~SQL, ids: ancestor_ids, parent_id: direct_parent_id, whisper: is_whisper)
INSERT INTO nested_view_post_stats (post_id, direct_reply_count, total_descendant_count,
whisper_direct_reply_count, whisper_total_descendant_count,
created_at, updated_at)
SELECT aid,
CASE WHEN aid = :parent_id THEN 1 ELSE 0 END,
1,
CASE WHEN aid = :parent_id THEN :whisper ELSE 0 END,
:whisper,
NOW(), NOW()
FROM unnest(ARRAY[:ids]::int[]) AS aid
ON CONFLICT (post_id) DO UPDATE SET
total_descendant_count = nested_view_post_stats.total_descendant_count + 1,
direct_reply_count = nested_view_post_stats.direct_reply_count +
CASE WHEN nested_view_post_stats.post_id = :parent_id THEN 1 ELSE 0 END,
whisper_total_descendant_count = nested_view_post_stats.whisper_total_descendant_count + :whisper,
whisper_direct_reply_count = nested_view_post_stats.whisper_direct_reply_count +
CASE WHEN nested_view_post_stats.post_id = :parent_id THEN :whisper ELSE 0 END,
updated_at = NOW()
SQL
end

add_model_callback(:post, :after_destroy) do
if reply_to_post_number.present?
my_descendants = NestedViewPostStat.where(post_id: id).pick(:total_descendant_count) || 0
stat =
NestedViewPostStat.where(post_id: id).pick(
:total_descendant_count,
:whisper_total_descendant_count,
)
my_descendants = stat&.first || 0
my_whisper_descendants = stat&.second || 0
removed = 1 + my_descendants
is_whisper = post_type == Post.types[:whisper] ? 1 : 0
whisper_removed = is_whisper + my_whisper_descendants

# Walk ancestors (including deleted — post may already be soft-deleted) for stats decrement
ancestors =
Expand All @@ -251,16 +274,28 @@ def reactions
direct_parent_id = ancestors.find { |a| a.depth == 1 }&.id

# Single UPDATE: decrement stats for all ancestors, clamped at 0
DB.exec(<<~SQL, ids: ancestor_ids, parent_id: direct_parent_id, removed: removed)
DB.exec(
<<~SQL,
UPDATE nested_view_post_stats
SET total_descendant_count = GREATEST(total_descendant_count - :removed, 0),
direct_reply_count = GREATEST(
direct_reply_count - CASE WHEN post_id = :parent_id THEN 1 ELSE 0 END,
0
),
whisper_total_descendant_count = GREATEST(whisper_total_descendant_count - :whisper_removed, 0),
whisper_direct_reply_count = GREATEST(
whisper_direct_reply_count - CASE WHEN post_id = :parent_id THEN :is_whisper ELSE 0 END,
0
),
updated_at = NOW()
WHERE post_id = ANY(ARRAY[:ids]::int[])
SQL
ids: ancestor_ids,
parent_id: direct_parent_id,
removed: removed,
whisper_removed: whisper_removed,
is_whisper: is_whisper,
)
end
end

Expand Down
88 changes: 88 additions & 0 deletions spec/lib/nested_replies_n_plus_one_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,51 @@ def build_chain(depth:, topic:)
expect(stat.total_descendant_count).to be >= 1
end
end

it "tracks whisper counts separately from regular counts" do
chain = build_chain(depth: 2, topic: topic)
Fabricate(
:post,
topic: topic,
user: user,
reply_to_post_number: chain.last.post_number,
post_type: Post.types[:whisper],
)

parent_stat = NestedViewPostStat.find_by(post_id: chain.last.id)
expect(parent_stat.direct_reply_count).to eq(1)
expect(parent_stat.whisper_direct_reply_count).to eq(1)
expect(parent_stat.total_descendant_count).to eq(1)
expect(parent_stat.whisper_total_descendant_count).to eq(1)
end

it "does not increment whisper counts for regular posts" do
chain = build_chain(depth: 2, topic: topic)
Fabricate(:post, topic: topic, user: user, reply_to_post_number: chain.last.post_number)

parent_stat = NestedViewPostStat.find_by(post_id: chain.last.id)
expect(parent_stat.direct_reply_count).to eq(1)
expect(parent_stat.whisper_direct_reply_count).to eq(0)
expect(parent_stat.total_descendant_count).to eq(1)
expect(parent_stat.whisper_total_descendant_count).to eq(0)
end

it "increments whisper counts on all ancestors" do
chain = build_chain(depth: 3, topic: topic)
Fabricate(
:post,
topic: topic,
user: user,
reply_to_post_number: chain.last.post_number,
post_type: Post.types[:whisper],
)

chain.each do |ancestor|
stat = NestedViewPostStat.find_by(post_id: ancestor.id)
next unless stat
expect(stat.whisper_total_descendant_count).to be >= 1
end
end
end

describe "after_destroy stats decrement" do
Expand Down Expand Up @@ -130,6 +175,49 @@ def build_chain(depth:, topic:)
leaf.destroy!
expect(NestedViewPostStat.find_by(post_id: leaf_id)).to be_nil
end

it "decrements whisper counts when a whisper is destroyed" do
chain = build_chain(depth: 2, topic: topic)
whisper =
Fabricate(
:post,
topic: topic,
user: user,
reply_to_post_number: chain.last.post_number,
post_type: Post.types[:whisper],
)

parent_stat = NestedViewPostStat.find_by(post_id: chain.last.id)
expect(parent_stat.whisper_direct_reply_count).to eq(1)
expect(parent_stat.whisper_total_descendant_count).to eq(1)

whisper.destroy!

parent_stat.reload
expect(parent_stat.direct_reply_count).to eq(0)
expect(parent_stat.whisper_direct_reply_count).to eq(0)
expect(parent_stat.total_descendant_count).to eq(0)
expect(parent_stat.whisper_total_descendant_count).to eq(0)
end

it "does not decrement whisper counts when a regular post is destroyed" do
chain = build_chain(depth: 2, topic: topic)
Fabricate(
:post,
topic: topic,
user: user,
reply_to_post_number: chain.last.post_number,
post_type: Post.types[:whisper],
)
regular =
Fabricate(:post, topic: topic, user: user, reply_to_post_number: chain.last.post_number)

regular.destroy!

parent_stat = NestedViewPostStat.find_by(post_id: chain.last.id)
expect(parent_stat.direct_reply_count).to eq(1)
expect(parent_stat.whisper_direct_reply_count).to eq(1)
end
end

describe "context endpoint" do
Expand Down
Loading