From a7d43cf1ec18fd7278926cc156529e3fefb721ab Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 17 Feb 2022 10:42:02 +1100 Subject: [PATCH] FEATURE: mute subcategory when parent category is muted (#15966) When parent category or grandparent category is muted, then category should be muted as well. Still, it can be overridden by setting individual subcategory notification level. CategoryUser record is not created, mute for subcategories is purely virtual. --- .../discourse/app/components/d-navigation.js | 14 ++++++ .../discourse/app/models/category.js | 13 +++-- .../app/models/topic-tracking-state.js | 15 ++++-- .../app/templates/components/d-navigation.hbs | 2 +- .../unit/models/topic-tracking-state-test.js | 13 +++++ app/controllers/categories_controller.rb | 2 +- app/models/category_user.rb | 22 +++++++++ app/serializers/current_user_serializer.rb | 5 ++ lib/topic_query.rb | 4 +- spec/models/category_user_spec.rb | 49 +++++++++++++++++++ 10 files changed, 128 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.js b/app/assets/javascripts/discourse/app/components/d-navigation.js index cc20ce09d1a..5c51380dcf7 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.js +++ b/app/assets/javascripts/discourse/app/components/d-navigation.js @@ -3,6 +3,7 @@ import FilterModeMixin from "discourse/mixins/filter-mode"; import NavItem from "discourse/models/nav-item"; import bootbox from "bootbox"; import discourseComputed from "discourse-common/utils/decorators"; +import { NotificationLevels } from "discourse/lib/notification-levels"; import { inject as service } from "@ember/service"; export default Component.extend(FilterModeMixin, { @@ -22,6 +23,19 @@ export default Component.extend(FilterModeMixin, { return category && this.currentUser; }, + @discourseComputed("category.notification_level") + categoryNotificationLevel(notificationLevel) { + if ( + this.currentUser?.indirectly_muted_category_ids?.includes( + this.category.id + ) + ) { + return NotificationLevels.MUTED; + } else { + return notificationLevel; + } + }, + // don't show tag notification menu on tag intersections @discourseComputed("tagNotification", "additionalTags") showTagNotifications(tagNotification, additionalTags) { diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index fca4880d7d5..b5b5a751a9c 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/app/assets/javascripts/discourse/app/models/category.js @@ -324,8 +324,6 @@ const Category = RestModel.extend({ }, setNotification(notification_level) { - this.set("notification_level", notification_level); - User.currentProp( "muted_category_ids", User.current().calculateMutedIds( @@ -336,7 +334,16 @@ const Category = RestModel.extend({ ); const url = `/category/${this.id}/notifications`; - return ajax(url, { data: { notification_level }, type: "POST" }); + return ajax(url, { data: { notification_level }, type: "POST" }).then( + (data) => { + User.current().set( + "indirectly_muted_category_ids", + data.indirectly_muted_category_ids + ); + this.set("notification_level", notification_level); + this.notifyPropertyChange("notification_level"); + } + ); }, @discourseComputed("id") diff --git a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js index 270a2b18093..301f78a7157 100644 --- a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js @@ -472,8 +472,10 @@ const TopicTrackingState = EmberObject.extend({ const subcategoryIds = noSubcategories ? new Set([categoryId]) : this.getSubCategoryIds(categoryId); - const mutedCategoryIds = - this.currentUser && this.currentUser.muted_category_ids; + + const mutedCategoryIds = this.currentUser?.muted_category_ids?.concat( + this.currentUser.indirectly_muted_category_ids + ); let filterFn = type === "new" ? isNew : isUnread; return Array.from(this.states.values()).filter( @@ -772,10 +774,13 @@ const TopicTrackingState = EmberObject.extend({ } if (["new_topic", "latest"].includes(data.message_type)) { - const muted_category_ids = User.currentProp("muted_category_ids"); + const mutedCategoryIds = User.currentProp("muted_category_ids")?.concat( + User.currentProp("indirectly_muted_category_ids") + ); + if ( - muted_category_ids && - muted_category_ids.includes(data.payload.category_id) + mutedCategoryIds && + mutedCategoryIds.includes(data.payload.category_id) ) { return; } diff --git a/app/assets/javascripts/discourse/app/templates/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/templates/components/d-navigation.hbs index 5dd7b8f665e..7c6bcd38dd1 100644 --- a/app/assets/javascripts/discourse/app/templates/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/d-navigation.hbs @@ -67,7 +67,7 @@ {{!-- don't show category notification menu on tag pages --}} {{#if showCategoryNotifications}} {{category-notifications-button - value=category.notification_level + value=categoryNotificationLevel category=category onChange=(action "changeCategoryNotificationLevel") }} diff --git a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js index 380e326abe5..936edb4d17f 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js @@ -661,6 +661,19 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) { ); }); + test("topics in indirectly muted categories do not get added to the state", function (assert) { + trackingState.currentUser.setProperties({ + muted_category_ids: [], + indirectly_muted_category_ids: [123], + }); + publishToMessageBus("/new", newTopicPayload); + assert.strictEqual( + trackingState.findState(222), + undefined, + "the new topic is not in the state" + ); + }); + test("topics in muted tags do not get added to the state", function (assert) { trackingState.currentUser.set("muted_tag_ids", [44]); publishToMessageBus("/new", newTopicPayload); diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index cddfb3931d6..eddf3489478 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -211,7 +211,7 @@ class CategoriesController < ApplicationController notification_level = params[:notification_level].to_i CategoryUser.set_notification_level_for_category(current_user, notification_level, category_id) - render json: success_json + render json: success_json.merge({ indirectly_muted_category_ids: CategoryUser.indirectly_muted_category_ids(current_user) }) end def destroy diff --git a/app/models/category_user.rb b/app/models/category_user.rb index 4f8cfb10be6..be8ac02b40b 100644 --- a/app/models/category_user.rb +++ b/app/models/category_user.rb @@ -234,6 +234,28 @@ class CategoryUser < ActiveRecord::Base acc[category_user.category_id] = category_user end end + + def self.indirectly_muted_category_ids(user) + query = Category.where.not(parent_category_id: nil) + .joins("LEFT JOIN categories categories2 ON categories2.id = categories.parent_category_id") + .joins("LEFT JOIN category_users ON category_users.category_id = categories.id AND category_users.user_id = #{user.id}") + .joins("LEFT JOIN category_users category_users2 ON category_users2.category_id = categories2.id AND category_users2.user_id = #{user.id}") + .where("category_users.id IS NULL") + + if SiteSetting.max_category_nesting === 3 + query = query + .joins("LEFT JOIN categories categories3 ON categories3.id = categories2.parent_category_id") + .joins("LEFT JOIN category_users category_users3 ON category_users3.category_id = categories3.id AND category_users3.user_id = #{user.id}") + .where(" + (category_users2.notification_level = #{notification_levels[:muted]}) + OR + (category_users2.id IS NULL AND category_users3.notification_level = #{notification_levels[:muted]}) + ") + else + query = query.where("category_users2.notification_level = #{notification_levels[:muted]}") + end + query.pluck("categories.id") + end end # == Schema Information diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 0692d609195..b521e98752a 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -28,6 +28,7 @@ class CurrentUserSerializer < BasicUserSerializer :redirected_to_top, :custom_fields, :muted_category_ids, + :indirectly_muted_category_ids, :regular_category_ids, :tracked_category_ids, :watched_first_post_category_ids, @@ -202,6 +203,10 @@ class CurrentUserSerializer < BasicUserSerializer categories_with_notification_level(:muted) end + def indirectly_muted_category_ids + CategoryUser.indirectly_muted_category_ids(object) + end + def regular_category_ids categories_with_notification_level(:regular) end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 324cfed547d..5918eafc0e6 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -846,10 +846,12 @@ class TopicQuery .references("cu") .joins("LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = #{user.id}") .where("topics.category_id = :category_id - OR COALESCE(category_users.notification_level, :default) <> :muted + OR + (COALESCE(category_users.notification_level, :default) <> :muted AND (topics.category_id IS NULL OR topics.category_id NOT IN(:indirectly_muted_category_ids))) OR tu.notification_level > :regular", category_id: category_id || -1, default: CategoryUser.default_notification_level, + indirectly_muted_category_ids: CategoryUser.indirectly_muted_category_ids(user).presence || [-1], muted: CategoryUser.notification_levels[:muted], regular: TopicUser.notification_levels[:regular]) elsif SiteSetting.mute_all_categories_by_default diff --git a/spec/models/category_user_spec.rb b/spec/models/category_user_spec.rb index 58be7482491..a3eb68bbb2c 100644 --- a/spec/models/category_user_spec.rb +++ b/spec/models/category_user_spec.rb @@ -268,4 +268,53 @@ describe CategoryUser do end end end + + describe "#indirectly_muted_category_ids" do + context "max category nesting 2" do + fab!(:category1) { Fabricate(:category) } + fab!(:category2) { Fabricate(:category, parent_category: category1) } + fab!(:category3) { Fabricate(:category, parent_category: category1) } + + it "calculates muted categories based on parent category state" do + expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([]) + + category_user = CategoryUser.create!(user: user, category: category1, notification_level: CategoryUser.notification_levels[:muted]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id, category3.id) + + CategoryUser.create!(user: user, category: category3, notification_level: CategoryUser.notification_levels[:muted]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id) + + category_user.update(notification_level: CategoryUser.notification_levels[:regular]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([]) + end + end + context "max category nesting 3" do + let(:category1) { Fabricate(:category) } + let(:category2) { Fabricate(:category, parent_category: category1) } + let(:category3) { Fabricate(:category, parent_category: category2) } + + before do + SiteSetting.max_category_nesting = 3 + category1 + category2 + category3 + end + it "calculates muted categories based on parent category state" do + expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([]) + + CategoryUser.create!(user: user, category: category1, notification_level: CategoryUser.notification_levels[:muted]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id, category3.id) + + category_user3 = CategoryUser.create!(user: user, category: category3, notification_level: CategoryUser.notification_levels[:muted]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id) + + category_user3.destroy + category_user2 = CategoryUser.create!(user: user, category: category2, notification_level: CategoryUser.notification_levels[:muted]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category3.id) + + category_user2.update(notification_level: CategoryUser.notification_levels[:regular]) + expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([]) + end + end + end end