mirror of
https://github.com/discourse/discourse.git
synced 2026-08-03 09:53:24 -05:00
FEATURE: Convert watched_precedence_over_muted to default site setting (#35671)
We have a setting and a user option called watched_precedence_over_muted . This is supposed to allow members to see/be notified of topics in categories they are watching even if those topics also include a tag the user has muted, or vice-versa if they disable this option. However, this setting and user option combination doesn't follow our existing default_ user preference site setting system, where admins can set a default for all users, but users can change the preference for themselves. This is leading to a bug in the topic feed logic. If the site setting watched_precedence_over_muted is set to true, and the user preference is set to false, the user preference is essentially ignored. This will lead to users seeing topics with muted tags inside a watched category feed. This commit converts watched_precedence_over_muted to a proper default user preference site setting, which admins can set for all users at once, then users can override individually.
This commit is contained in:
@@ -74,6 +74,7 @@ class SiteSetting < ActiveRecord::Base
|
||||
default_sidebar_link_to_filtered_list
|
||||
default_sidebar_show_count_of_new_items
|
||||
default_composition_mode
|
||||
default_watched_precedence_over_muted
|
||||
]
|
||||
|
||||
extend GlobalPath
|
||||
|
||||
@@ -1918,14 +1918,6 @@ class User < ActiveRecord::Base
|
||||
in_any_groups?(SiteSetting.experimental_new_new_view_groups_map)
|
||||
end
|
||||
|
||||
def watched_precedence_over_muted
|
||||
if user_option.watched_precedence_over_muted.nil?
|
||||
SiteSetting.watched_precedence_over_muted
|
||||
else
|
||||
user_option.watched_precedence_over_muted
|
||||
end
|
||||
end
|
||||
|
||||
def populated_required_custom_fields?
|
||||
UserField
|
||||
.for_all_users
|
||||
|
||||
@@ -109,6 +109,7 @@ class UserOption < ActiveRecord::Base
|
||||
self.sidebar_link_to_filtered_list = SiteSetting.default_sidebar_link_to_filtered_list
|
||||
self.sidebar_show_count_of_new_items = SiteSetting.default_sidebar_show_count_of_new_items
|
||||
self.composition_mode = SiteSetting.default_composition_mode
|
||||
self.watched_precedence_over_muted = SiteSetting.default_watched_precedence_over_muted
|
||||
|
||||
true
|
||||
end
|
||||
@@ -305,7 +306,7 @@ end
|
||||
# timezone :string
|
||||
# title_count_mode_key :integer default(0), not null
|
||||
# topics_unread_when_closed :boolean default(TRUE), not null
|
||||
# watched_precedence_over_muted :boolean
|
||||
# watched_precedence_over_muted :boolean default(FALSE), not null
|
||||
# color_scheme_id :integer
|
||||
# dark_scheme_id :integer
|
||||
# homepage_id :integer
|
||||
|
||||
@@ -311,13 +311,6 @@ class PostAlerter
|
||||
end
|
||||
|
||||
def category_or_tag_muters(topic)
|
||||
user_option_condition_sql_fragment =
|
||||
if SiteSetting.watched_precedence_over_muted
|
||||
"uo.watched_precedence_over_muted IS false"
|
||||
else
|
||||
"(uo.watched_precedence_over_muted IS NULL OR uo.watched_precedence_over_muted IS false)"
|
||||
end
|
||||
|
||||
user_ids_sql = <<~SQL
|
||||
SELECT uo.user_id FROM user_options uo
|
||||
LEFT JOIN topic_users tus ON tus.user_id = uo.user_id AND tus.topic_id = #{topic.id}
|
||||
@@ -327,7 +320,7 @@ class PostAlerter
|
||||
WHERE
|
||||
(tus.id IS NULL OR tus.notification_level != #{TopicUser.notification_levels[:watching]})
|
||||
AND (cu.notification_level = #{CategoryUser.notification_levels[:muted]} OR tu.notification_level = #{TagUser.notification_levels[:muted]})
|
||||
AND #{user_option_condition_sql_fragment}
|
||||
AND uo.watched_precedence_over_muted IS false
|
||||
SQL
|
||||
|
||||
User.where("id IN (#{user_ids_sql})")
|
||||
|
||||
@@ -76,6 +76,7 @@ class SiteSettingUpdateExistingUsers
|
||||
default_sidebar_link_to_filtered_list: "sidebar_link_to_filtered_list",
|
||||
default_sidebar_show_count_of_new_items: "sidebar_show_count_of_new_items",
|
||||
default_composition_mode: "composition_mode",
|
||||
default_watched_precedence_over_muted: "watched_precedence_over_muted",
|
||||
}
|
||||
end
|
||||
|
||||
|
||||
@@ -2685,7 +2685,7 @@ en:
|
||||
force_lowercase_tags: "Force all new tags to be entirely lowercase."
|
||||
create_post_for_category_and_tag_changes: "Create a whisper post when a topic's category or tags change, requires whisper posts to be enabled."
|
||||
automatically_clean_unused_tags: "Automatically delete tags that are not being used on any topics or personal messages on a daily basis."
|
||||
watched_precedence_over_muted: "Notify me about topics in categories or tags I’m watching that also belong to one I have muted"
|
||||
default_watched_precedence_over_muted: "Notify users about topics in categories or tags that they are watching that also belong to one they have muted"
|
||||
|
||||
company_name: "Name of your company or organization. If left blank, no boilerplate Terms of Service or Privacy Notice will be provided."
|
||||
governing_law: "Specify the jurisdiction that governs the legal aspects of the site, including Terms of Service and Privacy Policy. This is typically the country or state where the company operating the site is registered or conducts business."
|
||||
|
||||
@@ -3953,6 +3953,10 @@ user_preferences:
|
||||
default_other_enable_markdown_monospace_font:
|
||||
default: true
|
||||
area: "posts_and_topics|user_defaults"
|
||||
default_watched_precedence_over_muted:
|
||||
client: true
|
||||
default: false
|
||||
area: "categories_and_tags|user_defaults"
|
||||
|
||||
api:
|
||||
retain_web_hook_events_period_days:
|
||||
@@ -4122,10 +4126,6 @@ tags:
|
||||
default: always
|
||||
enum: RemoveMutedTagsFromLatestSiteSetting
|
||||
area: "categories_and_tags"
|
||||
watched_precedence_over_muted:
|
||||
client: true
|
||||
default: false
|
||||
area: "categories_and_tags"
|
||||
|
||||
force_lowercase_tags:
|
||||
default: true
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
# frozen_string_literal: true
|
||||
class ConvertWatchedPrecedenceOverMutedToDefaultPref < ActiveRecord::Migration[8.0]
|
||||
def up
|
||||
existing_setting_value =
|
||||
DB.query_single(
|
||||
"SELECT value FROM site_settings WHERE name = 'watched_precedence_over_muted'",
|
||||
).first
|
||||
|
||||
# Data type 5 is boolean
|
||||
DB.exec(<<~SQL, setting_value: existing_setting_value) if existing_setting_value
|
||||
INSERT INTO site_settings (name, data_type, value, created_at, updated_at)
|
||||
VALUES ('default_watched_precedence_over_muted', 5, :setting_value, NOW(), NOW())
|
||||
SQL
|
||||
|
||||
preference_value =
|
||||
if existing_setting_value.nil?
|
||||
# This is the default site setting value for default_watched_precedence_over_muted
|
||||
preference_value = false
|
||||
else
|
||||
preference_value = existing_setting_value == "t"
|
||||
end
|
||||
|
||||
DB.exec(<<~SQL, preference_value: preference_value)
|
||||
UPDATE user_options
|
||||
SET watched_precedence_over_muted = :preference_value
|
||||
WHERE watched_precedence_over_muted IS NULL;
|
||||
SQL
|
||||
|
||||
change_column_default :user_options, :watched_precedence_over_muted, from: nil, to: false
|
||||
change_column_null :user_options, :watched_precedence_over_muted, false
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,12 @@
|
||||
# frozen_string_literal: true
|
||||
class DeleteOldWatchedPrecedenceSetting < ActiveRecord::Migration[8.0]
|
||||
def up
|
||||
DB.exec(<<~SQL)
|
||||
DELETE FROM site_settings WHERE name = 'watched_precedence_over_muted';
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
||||
@@ -90,6 +90,7 @@ export const DEFAULT_USER_PREFERENCES = [
|
||||
"default_sidebar_link_to_filtered_list",
|
||||
"default_sidebar_show_count_of_new_items",
|
||||
"default_composition_mode",
|
||||
"default_watched_precedence_over_muted",
|
||||
];
|
||||
|
||||
export const MAIN_FONTS = [
|
||||
|
||||
@@ -119,10 +119,6 @@ export default class extends Controller {
|
||||
(this.model.watched_tags?.length > 0 &&
|
||||
this.model.mutedCategories?.length > 0);
|
||||
|
||||
if (show && this.model.user_option.watched_precedence_over_muted === null) {
|
||||
this.model.user_option.watched_precedence_over_muted =
|
||||
this.siteSettings.watched_precedence_over_muted;
|
||||
}
|
||||
return show;
|
||||
}
|
||||
|
||||
|
||||
+2
-3
@@ -1012,7 +1012,7 @@ class TopicQuery
|
||||
|
||||
if user
|
||||
watched_tag_ids =
|
||||
if user.watched_precedence_over_muted
|
||||
if user.user_option.watched_precedence_over_muted
|
||||
TagUser
|
||||
.where(user: user)
|
||||
.where("notification_level >= ?", TopicUser.notification_levels[:watching])
|
||||
@@ -1100,8 +1100,7 @@ class TopicQuery
|
||||
if user && !opts[:skip_categories]
|
||||
query_params[:regular] = CategoryUser.notification_levels[:regular]
|
||||
|
||||
query_params[:watching_or_infinite] = if user.watched_precedence_over_muted ||
|
||||
SiteSetting.watched_precedence_over_muted
|
||||
query_params[:watching_or_infinite] = if user.user_option.watched_precedence_over_muted
|
||||
CategoryUser.notification_levels[:watching]
|
||||
else
|
||||
99
|
||||
|
||||
@@ -2354,7 +2354,7 @@ RSpec.describe TopicQuery do
|
||||
|
||||
context "when enabled" do
|
||||
it "returns topics even if category or tag is muted but another tag or category is watched" do
|
||||
SiteSetting.watched_precedence_over_muted = true
|
||||
user.user_option.update!(watched_precedence_over_muted: true)
|
||||
query = TopicQuery.new(user).list_latest
|
||||
expect(query.topics.map(&:id)).to contain_exactly(
|
||||
topic.id,
|
||||
@@ -2366,24 +2366,11 @@ RSpec.describe TopicQuery do
|
||||
|
||||
context "when disabled" do
|
||||
it "returns topics without muted category or tag" do
|
||||
SiteSetting.watched_precedence_over_muted = false
|
||||
user.user_option.update!(watched_precedence_over_muted: false)
|
||||
query = TopicQuery.new(user).list_latest
|
||||
expect(query.topics.map(&:id)).to contain_exactly(topic.id)
|
||||
end
|
||||
end
|
||||
|
||||
context "when disabled but overridden by user" do
|
||||
it "returns topics even if category or tag is muted but another tag or category is watched" do
|
||||
SiteSetting.watched_precedence_over_muted = false
|
||||
user.user_option.update!(watched_precedence_over_muted: true)
|
||||
query = TopicQuery.new(user).list_latest
|
||||
expect(query.topics.map(&:id)).to contain_exactly(
|
||||
topic.id,
|
||||
topic_in_watched_category_and_muted_tag.id,
|
||||
topic_in_muted_category_and_watched_tag.id,
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "state parameter" do
|
||||
|
||||
@@ -230,7 +230,7 @@ RSpec.describe TagUser do
|
||||
end
|
||||
|
||||
it "sets notification level to the highest one if there are multiple tags" do
|
||||
SiteSetting.watched_precedence_over_muted = true
|
||||
user.user_option.update!(watched_precedence_over_muted: true)
|
||||
|
||||
TagUser.create!(
|
||||
user: user,
|
||||
|
||||
@@ -86,6 +86,7 @@ RSpec.describe UserOption do
|
||||
SiteSetting.default_other_external_links_in_new_tab = true
|
||||
SiteSetting.default_other_dynamic_favicon = true
|
||||
SiteSetting.default_other_skip_new_user_tips = true
|
||||
SiteSetting.default_watched_precedence_over_muted = true
|
||||
|
||||
user = Fabricate(:user)
|
||||
|
||||
@@ -96,6 +97,7 @@ RSpec.describe UserOption do
|
||||
expect(user.user_option.external_links_in_new_tab).to eq(true)
|
||||
expect(user.user_option.dynamic_favicon).to eq(true)
|
||||
expect(user.user_option.skip_new_user_tips).to eq(true)
|
||||
expect(user.user_option.watched_precedence_over_muted).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -814,7 +814,7 @@
|
||||
"type": "boolean"
|
||||
},
|
||||
"watched_precedence_over_muted": {
|
||||
"type": ["boolean", "null"]
|
||||
"type": ["boolean"]
|
||||
},
|
||||
"seen_popups": {
|
||||
"type": ["array", "null"]
|
||||
|
||||
@@ -2095,51 +2095,7 @@ RSpec.describe PostAlerter do
|
||||
)
|
||||
end
|
||||
|
||||
it "adds notification when watched_precedence_over_muted setting is true" do
|
||||
SiteSetting.watched_precedence_over_muted = true
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_tag_and_watched_category.posts.first)
|
||||
}.to change { Notification.count }.by(1)
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_category_and_watched_tag.posts.first)
|
||||
}.to change { Notification.count }.by(1)
|
||||
expect { PostAlerter.post_created(directly_watched_topic.posts.first) }.to change {
|
||||
Notification.count
|
||||
}.by(1)
|
||||
end
|
||||
|
||||
it "respects user option even if watched_precedence_over_muted site setting is true" do
|
||||
SiteSetting.watched_precedence_over_muted = true
|
||||
user.user_option.update!(watched_precedence_over_muted: false)
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_tag_and_watched_category.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_category_and_watched_tag.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect { PostAlerter.post_created(directly_watched_topic.posts.first) }.to change {
|
||||
Notification.count
|
||||
}.by(1)
|
||||
end
|
||||
|
||||
it "does not add notification when watched_precedence_over_muted setting is false" do
|
||||
SiteSetting.watched_precedence_over_muted = false
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_tag_and_watched_category.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_category_and_watched_tag.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect { PostAlerter.post_created(topic_with_watched_category.posts.first) }.to change {
|
||||
Notification.count
|
||||
}.by(1)
|
||||
expect { PostAlerter.post_created(directly_watched_topic.posts.first) }.to change {
|
||||
Notification.count
|
||||
}.by(1)
|
||||
end
|
||||
|
||||
it "respects user option even if watched_precedence_over_muted site setting is false" do
|
||||
SiteSetting.watched_precedence_over_muted = false
|
||||
it "adds notification when watched_precedence_over_muted preference is true" do
|
||||
user.user_option.update!(watched_precedence_over_muted: true)
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_tag_and_watched_category.posts.first)
|
||||
@@ -2151,6 +2107,19 @@ RSpec.describe PostAlerter do
|
||||
Notification.count
|
||||
}.by(1)
|
||||
end
|
||||
|
||||
it "does not add notifcation when watched_precedence_over_muted preference is false" do
|
||||
user.user_option.update!(watched_precedence_over_muted: false)
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_tag_and_watched_category.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect {
|
||||
PostAlerter.post_created(topic_with_muted_category_and_watched_tag.posts.first)
|
||||
}.not_to change { Notification.count }
|
||||
expect { PostAlerter.post_created(directly_watched_topic.posts.first) }.to change {
|
||||
Notification.count
|
||||
}.by(1)
|
||||
end
|
||||
end
|
||||
|
||||
context "with on change" do
|
||||
|
||||
Reference in New Issue
Block a user