FIX: DeleteReplies should use the guardian instead of checking for staff (#35443)

Category moderators (who aren't staff member) are able to a topic timer
to automatically delete replies after a certain amount of time but the
background job (DeleteReplies) was deleting the "topic timer" because
the category moderators wasn't a staff member.

There was a discrepency between the UX who used "can_delete" to
show/hide the "topic timer" option and the back-end who was checking for
"staff" membership.

This fixes it by changing the backend to use the guardian's "can_delete"
method instead.

Internal ref - t/165077
This commit is contained in:
Régis Hanol
2025-10-16 18:26:41 +02:00
committed by GitHub
parent 28b3cff3f7
commit bd94fcbce6
4 changed files with 98 additions and 1 deletions
+1 -1
View File
@@ -3,7 +3,7 @@
module Jobs
class DeleteReplies < ::Jobs::TopicTimerBase
def execute_timer_action(topic_timer, topic)
unless Guardian.new(topic_timer.user).is_staff?
unless Guardian.new(topic_timer.user).can_delete?(topic)
topic_timer.trash!(Discourse.system_user)
return
end
+55
View File
@@ -41,4 +41,59 @@ RSpec.describe Jobs::DeleteReplies do
topic.posts.count
}.by(-1)
end
it "trashes the timer if user lacks delete permissions" do
user = Fabricate(:user)
topic = Fabricate(:topic)
3.times { create_post(topic:) }
timer =
Fabricate(
:topic_timer,
status_type: TopicTimer.types[:delete_replies],
duration_minutes: 2880,
user:,
topic:,
execute_at: 2.days.from_now,
)
freeze_time(2.days.from_now)
expect { described_class.new.execute(topic_timer_id: timer.id) }.not_to change {
topic.posts.count
}
expect(timer.reload.deleted_at).to be_present
end
it "allows category moderators to delete replies" do
SiteSetting.enable_category_group_moderation = true
SiteSetting.skip_auto_delete_reply_likes = 0
user = Fabricate(:user, trust_level: TrustLevel[4])
Group.user_trust_level_change!(user.id, user.trust_level)
category = Fabricate(:category)
topic = Fabricate(:topic, category:)
Fabricate(:category_moderation_group, category:, group: user.groups.first)
3.times { create_post(topic:) }
timer =
Fabricate(
:topic_timer,
status_type: TopicTimer.types[:delete_replies],
duration_minutes: 2880,
user:,
topic:,
execute_at: 2.days.from_now,
)
freeze_time(2.days.from_now)
expect { described_class.new.execute(topic_timer_id: timer.id) }.to change {
topic.posts.count
}.by(-2)
expect(timer.reload.deleted_at).to be_nil
end
end
+19
View File
@@ -5122,6 +5122,25 @@ RSpec.describe TopicsController do
expect(response.status).to eq(403)
expect(response.parsed_body["error_type"]).to eq("invalid_access")
end
it "allows category moderators to set delete_replies timer" do
user.update!(trust_level: TrustLevel[4])
Group.user_trust_level_change!(user.id, user.trust_level)
Fabricate(:category_moderation_group, category: topic.category, group: user.groups.first)
sign_in(user)
post "/t/#{topic.id}/timer.json",
params: {
duration_minutes: 1440,
status_type: "delete_replies",
}
expect(response.status).to eq(200)
topic_timer = TopicTimer.last
expect(topic_timer.status_type).to eq(TopicTimer.types[:delete_replies])
end
end
end
@@ -52,4 +52,27 @@ RSpec.describe TopicViewDetailsSerializer do
expect(serializer.as_json.dig(:topic_view_details, :can_permanently_delete)).to eq(nil)
end
end
describe "#can_delete" do
before { SiteSetting.enable_category_group_moderation = true }
it "is true for category moderators in their category" do
user = Fabricate(:user, trust_level: TrustLevel[4])
Group.user_trust_level_change!(user.id, user.trust_level)
category = Fabricate(:category)
topic = Fabricate(:topic, category:)
Fabricate(:category_moderation_group, category:, group: user.groups.first)
serializer = described_class.new(TopicView.new(topic, user), scope: Guardian.new(user))
expect(serializer.as_json.dig(:topic_view_details, :can_delete)).to eq(true)
end
it "is false for regular users" do
user = Fabricate(:user)
topic = Fabricate(:topic)
serializer = described_class.new(TopicView.new(topic, user), scope: Guardian.new(user))
expect(serializer.as_json.dig(:topic_view_details, :can_delete)).to eq(nil)
end
end
end