From d65a839577e2a9911a2a75b5029f214f9db745e7 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 13 Aug 2020 17:20:23 -0400 Subject: [PATCH] FEATURE: allow group membership to unmute categories and tags For sites that are configured to mute some or all categories and tags for users by default, groups can now be configured to set members' notification level to normal from the group manage UI. --- .../controllers/group-manage-categories.js | 7 ++- .../app/controllers/group-manage-tags.js | 7 ++- .../javascripts/discourse/app/models/group.js | 42 ++++++++++------- .../app/templates/group/manage/categories.hbs | 30 +++++++++---- .../app/templates/group/manage/tags.hbs | 24 ++++++++-- app/controllers/groups_controller.rb | 2 +- app/models/group.rb | 2 +- app/models/group_user.rb | 4 +- app/serializers/basic_group_serializer.rb | 4 +- config/locales/client.en.yml | 2 + spec/models/group_spec.rb | 6 +++ spec/models/group_user_spec.rb | 45 +++++++++++-------- .../group-manage-categories-test.js | 4 +- .../acceptance/group-manage-tags-test.js | 4 +- 14 files changed, 124 insertions(+), 59 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js b/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js index 629679cc34d..01059518f2c 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js +++ b/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js @@ -6,9 +6,12 @@ export default Controller.extend({ "model.watchingCategories.[]", "model.watchingFirstPostCategories.[]", "model.trackingCategories.[]", + "model.regularCategories.[]", "model.mutedCategories.[]" ) - selectedCategories(watching, watchingFirst, tracking, muted) { - return [].concat(watching, watchingFirst, tracking, muted).filter(t => t); + selectedCategories(watching, watchingFirst, tracking, regular, muted) { + return [] + .concat(watching, watchingFirst, tracking, regular, muted) + .filter(t => t); } }); diff --git a/app/assets/javascripts/discourse/app/controllers/group-manage-tags.js b/app/assets/javascripts/discourse/app/controllers/group-manage-tags.js index 151365c1da9..0066b58e472 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-manage-tags.js +++ b/app/assets/javascripts/discourse/app/controllers/group-manage-tags.js @@ -6,9 +6,12 @@ export default Controller.extend({ "model.watching_tags.[]", "model.watching_first_post_tags.[]", "model.tracking_tags.[]", + "model.regular_tags.[]", "model.muted_tags.[]" ) - selectedTags(watching, watchingFirst, tracking, muted) { - return [].concat(watching, watchingFirst, tracking, muted).filter(t => t); + selectedTags(watching, watchingFirst, tracking, regular, muted) { + return [] + .concat(watching, watchingFirst, tracking, regular, muted) + .filter(t => t); } }); diff --git a/app/assets/javascripts/discourse/app/models/group.js b/app/assets/javascripts/discourse/app/models/group.js index 883aedc0bef..1f3681b28e0 100644 --- a/app/assets/javascripts/discourse/app/models/group.js +++ b/app/assets/javascripts/discourse/app/models/group.js @@ -200,6 +200,14 @@ const Group = RestModel.extend({ ); }, + @observes("regular_category_ids") + _updateRegularCategories() { + this.set( + "regularCategories", + Category.findByIds(this.regular_category_ids) + ); + }, + @observes("muted_category_ids") _updateMutedCategories() { this.set("mutedCategories", Category.findByIds(this.muted_category_ids)); @@ -240,25 +248,27 @@ const Group = RestModel.extend({ publish_read_state: this.publish_read_state }; - ["muted", "watching", "tracking", "watching_first_post"].forEach(s => { - let prop = - s === "watching_first_post" - ? "watchingFirstPostCategories" - : s + "Categories"; + ["muted", "regular", "watching", "tracking", "watching_first_post"].forEach( + s => { + let prop = + s === "watching_first_post" + ? "watchingFirstPostCategories" + : s + "Categories"; - let categories = this.get(prop); + let categories = this.get(prop); - if (categories) { - attrs[s + "_category_ids"] = - categories.length > 0 ? categories.map(c => c.get("id")) : [-1]; + if (categories) { + attrs[s + "_category_ids"] = + categories.length > 0 ? categories.map(c => c.get("id")) : [-1]; + } + + let tags = this.get(s + "_tags"); + + if (tags) { + attrs[s + "_tags"] = tags.length > 0 ? tags : [""]; + } } - - let tags = this.get(s + "_tags"); - - if (tags) { - attrs[s + "_tags"] = tags.length > 0 ? tags : [""]; - } - }); + ); if (this.flair_type === "icon") { attrs["flair_icon"] = this.flair_icon; diff --git a/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs b/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs index c130ce5a8a5..61af9de7c5b 100644 --- a/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs +++ b/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs @@ -5,11 +5,11 @@
- + {{category-selector categories=model.watchingCategories - blacklist=selectedCategories + blocklist=selectedCategories onChange=(action (mut model.watchingCategories)) }} @@ -19,11 +19,11 @@
- + {{category-selector categories=model.trackingCategories - blacklist=selectedCategories + blocklist=selectedCategories onChange=(action (mut model.trackingCategories)) }} @@ -33,11 +33,11 @@
- + {{category-selector categories=model.watchingFirstPostCategories - blacklist=selectedCategories + blocklist=selectedCategories onChange=(action (mut model.watchingFirstPostCategories)) }} @@ -47,11 +47,25 @@
- + + + {{category-selector + categories=model.regularCategories + blocklist=selectedCategories + onChange=(action (mut model.regularCategories)) + }} + +
+ {{i18n "groups.manage.categories.regular_categories_instructions"}} +
+
+ +
+ {{category-selector categories=model.mutedCategories - blacklist=selectedCategories + blocklist=selectedCategories onChange=(action (mut model.mutedCategories)) }} diff --git a/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs b/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs index 6a5a91019a4..604ac185247 100644 --- a/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs +++ b/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs @@ -5,7 +5,7 @@
- + {{tag-chooser tags=model.watching_tags @@ -21,7 +21,7 @@
- + {{tag-chooser tags=model.tracking_tags @@ -37,7 +37,7 @@
- + {{tag-chooser tags=model.watching_first_post_tags @@ -53,7 +53,23 @@
- + + + {{tag-chooser + tags=model.regular_tags + blacklist=selectedTags + allowCreate=false + everyTag=true + unlimitedTagCount=true + }} + +
+ {{i18n "groups.manage.tags.regular_tags_instructions"}} +
+
+ +
+ {{tag-chooser tags=model.muted_tags diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 53ca79530b2..c473ce6d282 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -611,7 +611,7 @@ class GroupsController < ApplicationController end if !automatic || current_user.admin - [:muted, :tracking, :watching, :watching_first_post].each do |level| + [:muted, :regular, :tracking, :watching, :watching_first_post].each do |level| permitted_params << { "#{level}_category_ids" => [] } permitted_params << { "#{level}_tags" => [] } end diff --git a/app/models/group.rb b/app/models/group.rb index 9b99d4667dc..61a986443f9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -765,7 +765,7 @@ class Group < ActiveRecord::Base flair_icon.presence || flair_upload&.short_path end - [:muted, :tracking, :watching, :watching_first_post].each do |level| + [:muted, :regular, :tracking, :watching, :watching_first_post].each do |level| define_method("#{level}_category_ids=") do |category_ids| @category_notifications ||= {} @category_notifications[level] = category_ids diff --git a/app/models/group_user.rb b/app/models/group_user.rb index f2806219e10..921641d4c64 100644 --- a/app/models/group_user.rb +++ b/app/models/group_user.rb @@ -86,7 +86,7 @@ class GroupUser < ActiveRecord::Base higher_level_category_ids = user_levels.values.flatten - [:muted, :tracking, :watching_first_post, :watching].each do |level| + [:muted, :regular, :tracking, :watching_first_post, :watching].each do |level| level_num = NotificationLevels.all[level] higher_level_category_ids -= (user_levels[level_num] || []) if group_category_ids = group_levels[level_num] @@ -118,7 +118,7 @@ class GroupUser < ActiveRecord::Base higher_level_tag_ids = user_levels.values.flatten - [:muted, :tracking, :watching_first_post, :watching].each do |level| + [:muted, :regular, :tracking, :watching_first_post, :watching].each do |level| level_num = NotificationLevels.all[level] higher_level_tag_ids -= (user_levels[level_num] || []) if group_tag_ids = group_levels[level_num] diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index 1b2f9d0ece9..c75189022f3 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -69,10 +69,12 @@ class BasicGroupSerializer < ApplicationSerializer admin_or_owner_attributes :watching_category_ids, :tracking_category_ids, :watching_first_post_category_ids, + :regular_category_ids, :muted_category_ids, :watching_tags, :watching_first_post_tags, :tracking_tags, + :regular_tags, :muted_tags def include_display_name? @@ -121,7 +123,7 @@ class BasicGroupSerializer < ApplicationSerializer scope.can_see_group_members?(object) end - [:watching, :tracking, :watching_first_post, :muted].each do |level| + [:watching, :regular, :tracking, :watching_first_post, :muted].each do |level| define_method("#{level}_category_ids") do GroupCategoryNotificationDefault.lookup(object, level).pluck(:category_id) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9a0a16a1332..46b2680b795 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -663,6 +663,7 @@ en: watched_categories_instructions: "Automatically watch all topics in these categories. Group members will be notified of all new posts and topics, and a count of new posts will also appear next to the topic." tracked_categories_instructions: "Automatically track all topics in these categories. A count of new posts will appear next to the topic." watching_first_post_categories_instructions: "Users will be notified of the first post in each new topic in these categories." + regular_categories_instructions: "If these categories are muted, they will be unmuted for group members. Users will be notified if they are mentioned or someone replies to them." muted_categories_instructions: "Users will not be notified of anything about new topics in these categories, and they will not appear on the categories or latest topics pages." tags: title: Tags @@ -671,6 +672,7 @@ en: watched_tags_instructions: "Automatically watch all topics with these tags. Group members will be notified of all new posts and topics, and a count of new posts will also appear next to the topic." tracked_tags_instructions: "Automatically track all topics with these tags. A count of new posts will appear next to the topic." watching_first_post_tags_instructions: "Users will be notified of the first post in each new topic with these tags." + regular_tags_instructions: "If these tags are muted, they will be unmuted for group members. Users will be notified if they are mentioned or someone replies to them." muted_tags_instructions: "Users will not be notified of anything about new topics with these tags, and they will not appear in latest." logs: title: "Logs" diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d57b8b996d4..dad0ef6dad1 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1059,18 +1059,22 @@ describe Group do let(:category1) { Fabricate(:category) } let(:category2) { Fabricate(:category) } let(:category3) { Fabricate(:category) } + let(:category4) { Fabricate(:category) } let(:tag1) { Fabricate(:tag) } let(:tag2) { Fabricate(:tag) } let(:tag3) { Fabricate(:tag) } + let(:tag4) { Fabricate(:tag) } let(:synonym1) { Fabricate(:tag, target_tag: tag1) } let(:synonym2) { Fabricate(:tag, target_tag: tag2) } it "can set category notifications" do group.watching_category_ids = [category1.id, category2.id] group.tracking_category_ids = [category3.id] + group.regular_category_ids = [category4.id] group.save! expect(GroupCategoryNotificationDefault.lookup(group, :watching).pluck(:category_id)).to contain_exactly(category1.id, category2.id) expect(GroupCategoryNotificationDefault.lookup(group, :tracking).pluck(:category_id)).to eq([category3.id]) + expect(GroupCategoryNotificationDefault.lookup(group, :regular).pluck(:category_id)).to eq([category4.id]) new_group = Fabricate.build(:group) new_group.watching_category_ids = [category1.id, category2.id] @@ -1097,9 +1101,11 @@ describe Group do end it "can set tag notifications" do + group.regular_tags = [tag4.name] group.watching_tags = [tag1.name, tag2.name] group.tracking_tags = [tag3.name] group.save! + expect(GroupTagNotificationDefault.lookup(group, :regular).pluck(:tag_id)).to eq([tag4.id]) expect(GroupTagNotificationDefault.lookup(group, :watching).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id) expect(GroupTagNotificationDefault.lookup(group, :tracking).pluck(:tag_id)).to eq([tag3.id]) diff --git a/spec/models/group_user_spec.rb b/spec/models/group_user_spec.rb index cd35144c9b3..6b3029bab54 100644 --- a/spec/models/group_user_spec.rb +++ b/spec/models/group_user_spec.rb @@ -39,6 +39,7 @@ describe GroupUser do let(:category2) { Fabricate(:category) } let(:category3) { Fabricate(:category) } let(:category4) { Fabricate(:category) } + let(:category5) { Fabricate(:category) } def levels CategoryUser.notification_levels @@ -50,16 +51,18 @@ describe GroupUser do it "adds new category notifications" do group.muted_category_ids = [category1.id] - group.tracking_category_ids = [category2.id] - group.watching_category_ids = [category3.id] - group.watching_first_post_category_ids = [category4.id] + group.regular_category_ids = [category2.id] + group.tracking_category_ids = [category3.id] + group.watching_category_ids = [category4.id] + group.watching_first_post_category_ids = [category5.id] group.save! - expect { group.add(user) }.to change { CategoryUser.count }.by(4) + expect { group.add(user) }.to change { CategoryUser.count }.by(5) h = CategoryUser.notification_levels_for(Guardian.new(user)) expect(h[category1.id]).to eq(levels[:muted]) - expect(h[category2.id]).to eq(levels[:tracking]) - expect(h[category3.id]).to eq(levels[:watching]) - expect(h[category4.id]).to eq(levels[:watching_first_post]) + expect(h[category2.id]).to eq(levels[:regular]) + expect(h[category3.id]).to eq(levels[:tracking]) + expect(h[category4.id]).to eq(levels[:watching]) + expect(h[category5.id]).to eq(levels[:watching_first_post]) end it "only upgrades notifications" do @@ -67,11 +70,12 @@ describe GroupUser do CategoryUser.create!(user: user, category_id: category2.id, notification_level: levels[:tracking]) CategoryUser.create!(user: user, category_id: category3.id, notification_level: levels[:watching_first_post]) CategoryUser.create!(user: user, category_id: category4.id, notification_level: levels[:watching]) - group.watching_first_post_category_ids = [category1.id, category2.id, category3.id, category4.id] + group.regular_category_ids = [category1.id] + group.watching_first_post_category_ids = [category2.id, category3.id, category4.id] group.save! group.add(user) h = CategoryUser.notification_levels_for(Guardian.new(user)) - expect(h[category1.id]).to eq(levels[:watching_first_post]) + expect(h[category1.id]).to eq(levels[:regular]) expect(h[category2.id]).to eq(levels[:watching_first_post]) expect(h[category3.id]).to eq(levels[:watching_first_post]) expect(h[category4.id]).to eq(levels[:watching]) @@ -100,6 +104,7 @@ describe GroupUser do let(:tag2) { Fabricate(:tag) } let(:tag3) { Fabricate(:tag) } let(:tag4) { Fabricate(:tag) } + let(:tag5) { Fabricate(:tag) } let(:synonym1) { Fabricate(:tag, target_tag: tag1) } def levels @@ -112,15 +117,17 @@ describe GroupUser do it "adds new tag notifications" do group.muted_tags = [synonym1.name] - group.tracking_tags = [tag2.name] - group.watching_tags = [tag3.name] - group.watching_first_post_tags = [tag4.name] + group.regular_tags = [tag2.name] + group.tracking_tags = [tag3.name] + group.watching_tags = [tag4.name] + group.watching_first_post_tags = [tag5.name] group.save! - expect { group.add(user) }.to change { TagUser.count }.by(4) + expect { group.add(user) }.to change { TagUser.count }.by(5) expect(TagUser.lookup(user, :muted).pluck(:tag_id)).to eq([tag1.id]) - expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to eq([tag2.id]) - expect(TagUser.lookup(user, :watching).pluck(:tag_id)).to eq([tag3.id]) - expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to eq([tag4.id]) + expect(TagUser.lookup(user, :regular).pluck(:tag_id)).to eq([tag2.id]) + expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to eq([tag3.id]) + expect(TagUser.lookup(user, :watching).pluck(:tag_id)).to eq([tag4.id]) + expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to eq([tag5.id]) end it "only upgrades notifications" do @@ -128,13 +135,15 @@ describe GroupUser do TagUser.create!(user: user, tag_id: tag2.id, notification_level: levels[:tracking]) TagUser.create!(user: user, tag_id: tag3.id, notification_level: levels[:watching_first_post]) TagUser.create!(user: user, tag_id: tag4.id, notification_level: levels[:watching]) - group.watching_first_post_tags = [tag1.name, tag2.name, tag3.name, tag4.name] + group.regular_tags = [tag1.name] + group.watching_first_post_tags = [tag2.name, tag3.name, tag4.name] group.save! group.add(user) expect(TagUser.lookup(user, :muted).pluck(:tag_id)).to be_empty + expect(TagUser.lookup(user, :regular).pluck(:tag_id)).to eq([tag1.id]) expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to be_empty expect(TagUser.lookup(user, :watching).pluck(:tag_id)).to eq([tag4.id]) - expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id, tag3.id) + expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to contain_exactly(tag2.id, tag3.id) end it "merges notifications" do diff --git a/test/javascripts/acceptance/group-manage-categories-test.js b/test/javascripts/acceptance/group-manage-categories-test.js index 5bf7c88ea32..00aaf77ef40 100644 --- a/test/javascripts/acceptance/group-manage-categories-test.js +++ b/test/javascripts/acceptance/group-manage-categories-test.js @@ -16,7 +16,7 @@ QUnit.test("As an admin", async assert => { await visit("/g/discourse/manage/categories"); assert.ok( - find(".groups-notifications-form .category-selector").length === 4, + find(".groups-notifications-form .category-selector").length === 5, "it should display category inputs" ); }); @@ -27,7 +27,7 @@ QUnit.test("As a group owner", async assert => { await visit("/g/discourse/manage/categories"); assert.ok( - find(".groups-notifications-form .category-selector").length === 4, + find(".groups-notifications-form .category-selector").length === 5, "it should display category inputs" ); }); diff --git a/test/javascripts/acceptance/group-manage-tags-test.js b/test/javascripts/acceptance/group-manage-tags-test.js index d5140e199b5..86b3cb82833 100644 --- a/test/javascripts/acceptance/group-manage-tags-test.js +++ b/test/javascripts/acceptance/group-manage-tags-test.js @@ -16,7 +16,7 @@ QUnit.test("As an admin", async assert => { await visit("/g/discourse/manage/tags"); assert.ok( - find(".groups-notifications-form .tag-chooser").length === 4, + find(".groups-notifications-form .tag-chooser").length === 5, "it should display tag inputs" ); }); @@ -27,7 +27,7 @@ QUnit.test("As a group owner", async assert => { await visit("/g/discourse/manage/tags"); assert.ok( - find(".groups-notifications-form .tag-chooser").length === 4, + find(".groups-notifications-form .tag-chooser").length === 5, "it should display tag inputs" ); });