Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Add setting to disable notifications for topic tags edits #14794

Merged
merged 1 commit into from Nov 2, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/controllers/topics_controller.rb
Expand Up @@ -387,8 +387,7 @@ def update
success = true

if changes.length > 0

bypass_bump = changes[:category_id].present? && SiteSetting.disable_category_edit_notifications
bypass_bump = should_bypass_bump?(changes)

first_post = topic.ordered_posts.first
success = PostRevisor.new(first_post, topic).revise!(current_user, changes, validate_post: false, bypass_bump: bypass_bump)
Expand Down Expand Up @@ -1117,6 +1116,11 @@ def consider_user_for_promotion
Promotion.new(current_user).review if current_user.present?
end

def should_bypass_bump?(changes)
(changes[:category_id].present? && SiteSetting.disable_category_edit_notifications) ||
(changes[:tags].present? && SiteSetting.disable_tags_edit_notifications)
end

def slugs_do_not_match
if SiteSetting.slug_generation_method != "encoded"
params[:slug] && @topic_view.topic.slug != params[:slug]
Expand Down
14 changes: 10 additions & 4 deletions app/services/post_action_notifier.rb
Expand Up @@ -102,10 +102,7 @@ def self.after_create_post_revision(post_revision)
return if post_revision.user.blank?
return if post.topic.blank?
return if post.topic.private_message?
return if SiteSetting.disable_system_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID
if SiteSetting.disable_category_edit_notifications && post_revision.modifications&.dig("category_id").present?
return
end
return if notification_is_disabled?(post_revision)

user_ids = []

Expand Down Expand Up @@ -160,4 +157,13 @@ def self.custom_post_revision_notifier_recipients
def self.add_post_revision_notifier_recipients(&block)
custom_post_revision_notifier_recipients << block
end

private

def self.notification_is_disabled?(post_revision)
modifications = post_revision.modifications
(SiteSetting.disable_system_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID) ||
(SiteSetting.disable_category_edit_notifications && modifications&.dig("category_id").present?) ||
(SiteSetting.disable_tags_edit_notifications && modifications&.dig("tags").present?)
end
end
2 changes: 2 additions & 0 deletions config/locales/server.en.yml
Expand Up @@ -2131,6 +2131,8 @@ en:

disable_category_edit_notifications: "Disable category edit notifications on topics."

disable_tags_edit_notifications: "Disable tags edit notifications on topics."

notification_consolidation_threshold: "Number of liked or membership request notifications received before the notifications are consolidated into a single one. Set to 0 to disable."

likes_notification_consolidation_window_mins: "Duration in minutes where liked notifications are consolidated into a single notification once the threshold has been reached. The threshold can be configured via `SiteSetting.notification_consolidation_threshold`."
Expand Down
3 changes: 3 additions & 0 deletions config/site_settings.yml
Expand Up @@ -2210,6 +2210,9 @@ uncategorized:
disable_category_edit_notifications:
default: false

disable_tags_edit_notifications:
default: false

notification_consolidation_threshold:
default: 3
min: 0
Expand Down
62 changes: 39 additions & 23 deletions spec/requests/topics_controller_spec.rb
Expand Up @@ -1358,34 +1358,50 @@ def topic_user_post_timings_count(user, topic)
expect(response.status).to eq(200)
end

context 'when using SiteSetting.disable_category_edit_notifications' do
it "doesn't bump the topic if the setting is enabled" do
SiteSetting.disable_category_edit_notifications = true
last_bumped_at = topic.bumped_at
expect(last_bumped_at).not_to be_nil
context 'when using SiteSetting.disable_category_edit_notifications or SiteSetting.disable_tags_edit_notifications' do
shared_examples 'a topic bump suppressor' do
it "doesn't bump the topic if the setting is enabled" do
enable_setting
last_bumped_at = topic.bumped_at
expect(last_bumped_at).not_to be_nil

expect do
put "/t/#{topic.slug}/#{topic.id}.json", params: {
category_id: category.id
}
end.to change { topic.reload.category_id }.to(category.id)
expect do
put "/t/#{topic.slug}/#{topic.id}.json", params: params
end.to change { topic.reload.send(attribute_to_change) }.to(expected_new_value)

expect(response.status).to eq(200)
expect(topic.reload.bumped_at).to eq_time(last_bumped_at)
end
expect(response.status).to eq(200)
expect(topic.reload.bumped_at).to eq_time(last_bumped_at)
end

it "bumps the topic if the setting is disabled" do
last_bumped_at = topic.bumped_at
expect(last_bumped_at).not_to be_nil
it "bumps the topic if the setting is disabled" do
disable_setting
last_bumped_at = topic.bumped_at
expect(last_bumped_at).not_to be_nil

expect do
put "/t/#{topic.slug}/#{topic.id}.json", params: {
category_id: category.id
}
end.to change { topic.reload.category_id }.to(category.id)
expect do
put "/t/#{topic.slug}/#{topic.id}.json", params: params
end.to change { topic.reload.send(attribute_to_change) }.to(expected_new_value)

expect(response.status).to eq(200)
expect(topic.reload.bumped_at).not_to eq_time(last_bumped_at)
expect(response.status).to eq(200)
expect(topic.reload.bumped_at).not_to eq_time(last_bumped_at)
end
end

it_behaves_like 'a topic bump suppressor' do
let(:attribute_to_change) { :category_id }
let(:expected_new_value) { category.id }
let(:params) { { category_id: category.id } }
let(:enable_setting) { SiteSetting.disable_category_edit_notifications = true }
let(:disable_setting) { SiteSetting.disable_category_edit_notifications = false }
end

it_behaves_like 'a topic bump suppressor' do
let(:tags) { [Fabricate(:tag), Fabricate(:tag)] }
let(:attribute_to_change) { :tags }
let(:expected_new_value) { tags }
let(:params) { { tags: tags.map(&:name) } }
let(:enable_setting) { SiteSetting.disable_tags_edit_notifications = true }
let(:disable_setting) { SiteSetting.disable_tags_edit_notifications = false }
end
end

Expand Down
19 changes: 19 additions & 0 deletions spec/services/post_action_notifier_spec.rb
Expand Up @@ -172,6 +172,25 @@

end

context "tags edit notifications are disabled" do
it 'notifies a user of the revision made by another user' do
SiteSetting.disable_tags_edit_notifications = false

expect {
post.revise(evil_trout, tags: [Fabricate(:tag).name])
}.to change(post.user.notifications, :count).by(1)
end

it 'does not notify a user of the revision made by the system user' do
SiteSetting.disable_tags_edit_notifications = true

expect {
post.revise(evil_trout, tags: [Fabricate(:tag).name])
}.not_to change(post.user.notifications, :count)
end

end

context 'when using plugin API to add custom recipients' do
let(:lurker) { Fabricate(:user) }

Expand Down