From 134dcdd63a0b373564ef88b917685c7ef77da6f9 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 4 Jul 2023 15:08:29 +1000 Subject: [PATCH] FEATURE: allow user to override watched_precedence_over_muted setting (#22340) Recently, site setting watched_precedence_over_muted was introduced - https://github.com/discourse/discourse/pull/22252 In this PR, we are allowing users to override it. The option is only displayed when the user has watched categories and muted tags, or vice versa. --- .../app/controllers/preferences/tracking.js | 15 ++++++++ .../javascripts/discourse/app/models/user.js | 1 + .../app/templates/preferences/tracking.hbs | 9 +++++ app/models/user.rb | 8 ++++ app/models/user_option.rb | 1 + app/serializers/user_option_serializer.rb | 3 +- app/services/post_alerter.rb | 11 ++++-- app/services/user_updater.rb | 1 + config/locales/client.en.yml | 1 + ...d_precedence_over_muted_to_user_options.rb | 6 +++ lib/topic_query.rb | 37 ++++++++----------- spec/lib/topic_query_spec.rb | 13 +++++++ .../api/schemas/json/user_get_response.json | 3 ++ spec/services/post_alerter_spec.rb | 26 ++++++++++++- 14 files changed, 107 insertions(+), 28 deletions(-) create mode 100644 db/migrate/20230628062236_add_watched_precedence_over_muted_to_user_options.rb diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js b/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js index 0900a9fc6ff..2f515b94253 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js @@ -101,6 +101,20 @@ export default class extends Controller { ) .filter((t) => t); } + @computed( + "model.watchedCategories", + "model.mutedCategories", + "model.watched_tags.[]", + "model.muted_tags.[]" + ) + get showMutePrecedenceSetting() { + return ( + (this.model.watchedCategories?.length > 0 && + this.model.muted_tags?.length > 0) || + (this.model.watched_tags?.length > 0 && + this.model.mutedCategories?.length > 0) + ); + } @computed( "model.watchedCategories", @@ -147,6 +161,7 @@ export default class extends Controller { "watched_category_ids", "tracked_category_ids", "watched_first_post_category_ids", + "watched_precedence_over_muted", ]; if (this.siteSettings.tagging_enabled) { diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 3bf74e96858..435c2faa33e 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -131,6 +131,7 @@ let userOptionFields = [ "bookmark_auto_delete_preference", "sidebar_link_to_filtered_list", "sidebar_show_count_of_new_items", + "watched_precedence_over_muted", ]; export function addSaveableUserOptionField(fieldName) { diff --git a/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs b/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs index 5c95660f7de..2756e30642d 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs @@ -72,6 +72,15 @@ /> +{{#if this.showMutePrecedenceSetting}} +
+ +
+{{/if}} {{#if this.canSave}} do diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6de7e9f2228..6ae5abc6d18 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1248,6 +1248,7 @@ en: watched_first_post_categories_instructions: "You will be notified of the first post in each new topic in these categories." watched_first_post_tags: "Watching First Post" watched_first_post_tags_instructions: "You will be notified of the first post in each new topic with these tags." + watched_precedence_over_muted: "Notify me about topics in categories or tags I’m watching that also belong to one I have muted" muted_categories: "Muted" muted_categories_instructions: "You will not be notified of anything about new topics in these categories, and they will not appear on the categories or latest pages." diff --git a/db/migrate/20230628062236_add_watched_precedence_over_muted_to_user_options.rb b/db/migrate/20230628062236_add_watched_precedence_over_muted_to_user_options.rb new file mode 100644 index 00000000000..ed12eab9b98 --- /dev/null +++ b/db/migrate/20230628062236_add_watched_precedence_over_muted_to_user_options.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +class AddWatchedPrecedenceOverMutedToUserOptions < ActiveRecord::Migration[7.0] + def change + add_column :user_options, :watched_precedence_over_muted, :boolean + end +end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 5998a6d1d8a..3409f754586 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -898,7 +898,7 @@ class TopicQuery if user watched_tag_ids = - if SiteSetting.watched_precedence_over_muted + if user.watched_precedence_over_muted TagUser .where(user: user) .where("notification_level >= ?", TopicUser.notification_levels[:watching]) @@ -981,6 +981,19 @@ class TopicQuery end end + query_params = { tag_ids: muted_tag_ids } + + 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 + CategoryUser.notification_levels[:watching] + else + 99 + end + end + if SiteSetting.remove_muted_tags_from_latest == "always" list = list.where( @@ -991,16 +1004,7 @@ class TopicQuery WHERE tt.tag_id IN (:tag_ids) AND tt.topic_id = topics.id #{user && !opts[:skip_categories] ? "AND COALESCE(category_users.notification_level, :regular) < :watching_or_infinite" : ""})", - tag_ids: muted_tag_ids, - regular: CategoryUser.notification_levels[:regular], - watching_or_infinite: - ( - if SiteSetting.watched_precedence_over_muted - CategoryUser.notification_levels[:watching] - else - 99 - end - ), + query_params, ) else list = @@ -1013,16 +1017,7 @@ class TopicQuery AND tt.topic_id = topics.id) #{user && !opts[:skip_categories] ? "OR COALESCE(category_users.notification_level, :regular) >= :watching_or_infinite" : ""} ) OR NOT EXISTS (SELECT 1 FROM topic_tags tt WHERE tt.topic_id = topics.id)", - tag_ids: muted_tag_ids, - regular: CategoryUser.notification_levels[:regular], - watching_or_infinite: - ( - if SiteSetting.watched_precedence_over_muted - CategoryUser.notification_levels[:watching] - else - 99 - end - ), + query_params, ) end end diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb index 603f9f039e0..e3f399ce467 100644 --- a/spec/lib/topic_query_spec.rb +++ b/spec/lib/topic_query_spec.rb @@ -2160,5 +2160,18 @@ RSpec.describe TopicQuery do 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 end diff --git a/spec/requests/api/schemas/json/user_get_response.json b/spec/requests/api/schemas/json/user_get_response.json index 01cb1a18b35..ed539d524f3 100644 --- a/spec/requests/api/schemas/json/user_get_response.json +++ b/spec/requests/api/schemas/json/user_get_response.json @@ -777,6 +777,9 @@ "sidebar_show_count_of_new_items": { "type": "boolean" }, + "watched_precedence_over_muted": { + "type": ["boolean", "null"] + }, "seen_popups": { "type": ["array", "null"] } diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 2e49e23bc45..b6d6a634969 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1918,7 +1918,7 @@ RSpec.describe PostAlerter do ) end - it "adds notification when watched_precedence_over_mute setting is true" do + 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) @@ -1928,7 +1928,18 @@ RSpec.describe PostAlerter do }.to change { Notification.count }.by(1) end - it "does not add notification when watched_precedence_over_mute setting is false" do + 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 } + 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) @@ -1940,6 +1951,17 @@ RSpec.describe PostAlerter do 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 + user.user_option.update!(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) + end end context "with on change" do