From 1ca81fbb95ec32fce448c477cbb824e884574c93 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 6 Aug 2020 12:27:27 -0400 Subject: [PATCH] FEATURE: set notification levels when added to a group (#10378) * FEATURE: set notification levels when added to a group This feature allows admins and group owners to define default category and tag tracking levels that will be applied to user preferences automatically at the time when users are added to the group. Users are free to change those preferences afterwards. When removed from a group, the user's notification preferences aren't changed. --- .../controllers/group-manage-categories.js | 14 +++ .../app/controllers/group-manage-tags.js | 14 +++ .../discourse/app/controllers/group-manage.js | 8 ++ .../javascripts/discourse/app/models/group.js | 49 ++++++++ .../discourse/app/routes/app-route-map.js | 2 + .../app/routes/group-manage-categories.js | 10 ++ .../discourse/app/routes/group-manage-tags.js | 10 ++ .../app/templates/group/manage/categories.hbs | 64 ++++++++++ .../app/templates/group/manage/tags.hbs | 72 +++++++++++ .../stylesheets/common/base/groups.scss | 13 ++ app/assets/stylesheets/desktop/group.scss | 5 + app/controllers/groups_controller.rb | 7 ++ app/models/concerns/roleable.rb | 9 ++ app/models/group.rb | 36 ++++++ .../group_category_notification_default.rb | 77 ++++++++++++ app/models/group_tag_notification_default.rb | 57 +++++++++ app/models/group_user.rb | 66 ++++++++++ app/models/tag_user.rb | 37 +++++- app/serializers/basic_group_serializer.rb | 28 +++++ config/locales/client.en.yml | 16 +++ config/routes.rb | 2 + ...730205554_create_group_default_tracking.rb | 27 ++++ spec/models/group_spec.rb | 117 +++++++++++++++++ spec/models/group_user_spec.rb | 119 ++++++++++++++++++ spec/requests/groups_controller_spec.rb | 20 ++- .../group-manage-categories-test.js | 33 +++++ .../acceptance/group-manage-tags-test.js | 33 +++++ 27 files changed, 937 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/controllers/group-manage-categories.js create mode 100644 app/assets/javascripts/discourse/app/controllers/group-manage-tags.js create mode 100644 app/assets/javascripts/discourse/app/routes/group-manage-categories.js create mode 100644 app/assets/javascripts/discourse/app/routes/group-manage-tags.js create mode 100644 app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs create mode 100644 app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs create mode 100644 app/models/group_category_notification_default.rb create mode 100644 app/models/group_tag_notification_default.rb create mode 100644 db/migrate/20200730205554_create_group_default_tracking.rb create mode 100644 test/javascripts/acceptance/group-manage-categories-test.js create mode 100644 test/javascripts/acceptance/group-manage-tags-test.js diff --git a/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js b/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js new file mode 100644 index 00000000000..629679cc34d --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/group-manage-categories.js @@ -0,0 +1,14 @@ +import discourseComputed from "discourse-common/utils/decorators"; +import Controller from "@ember/controller"; + +export default Controller.extend({ + @discourseComputed( + "model.watchingCategories.[]", + "model.watchingFirstPostCategories.[]", + "model.trackingCategories.[]", + "model.mutedCategories.[]" + ) + selectedCategories(watching, watchingFirst, tracking, muted) { + return [].concat(watching, watchingFirst, tracking, 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 new file mode 100644 index 00000000000..151365c1da9 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/group-manage-tags.js @@ -0,0 +1,14 @@ +import discourseComputed from "discourse-common/utils/decorators"; +import Controller from "@ember/controller"; + +export default Controller.extend({ + @discourseComputed( + "model.watching_tags.[]", + "model.watching_first_post_tags.[]", + "model.tracking_tags.[]", + "model.muted_tags.[]" + ) + selectedTags(watching, watchingFirst, tracking, muted) { + return [].concat(watching, watchingFirst, tracking, muted).filter(t => t); + } +}); diff --git a/app/assets/javascripts/discourse/app/controllers/group-manage.js b/app/assets/javascripts/discourse/app/controllers/group-manage.js index b4fab8990bc..455c7a9b9ae 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-manage.js +++ b/app/assets/javascripts/discourse/app/controllers/group-manage.js @@ -13,6 +13,14 @@ export default Controller.extend({ route: "group.manage.interaction", title: "groups.manage.interaction.title" }, + { + route: "group.manage.categories", + title: "groups.manage.categories.title" + }, + { + route: "group.manage.tags", + title: "groups.manage.tags.title" + }, { route: "group.manage.logs", title: "groups.manage.logs.title" } ]; diff --git a/app/assets/javascripts/discourse/app/models/group.js b/app/assets/javascripts/discourse/app/models/group.js index 889272fe30a..883aedc0bef 100644 --- a/app/assets/javascripts/discourse/app/models/group.js +++ b/app/assets/javascripts/discourse/app/models/group.js @@ -176,6 +176,35 @@ const Group = RestModel.extend({ } }, + @observes("watching_category_ids") + _updateWatchingCategories() { + this.set( + "watchingCategories", + Category.findByIds(this.watching_category_ids) + ); + }, + + @observes("tracking_category_ids") + _updateTrackingCategories() { + this.set( + "trackingCategories", + Category.findByIds(this.tracking_category_ids) + ); + }, + + @observes("watching_first_post_category_ids") + _updateWatchingFirstPostCategories() { + this.set( + "watchingFirstPostCategories", + Category.findByIds(this.watching_first_post_category_ids) + ); + }, + + @observes("muted_category_ids") + _updateMutedCategories() { + this.set("mutedCategories", Category.findByIds(this.muted_category_ids)); + }, + asJSON() { const attrs = { name: this.name, @@ -211,6 +240,26 @@ 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"; + + let categories = this.get(prop); + + 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 : [""]; + } + }); + if (this.flair_type === "icon") { attrs["flair_icon"] = this.flair_icon; } else if (this.flair_type === "image") { diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index 23aa5e23a85..1e9df27af2c 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -94,6 +94,8 @@ export default function() { this.route("interaction"); this.route("email"); this.route("members"); + this.route("categories"); + this.route("tags"); this.route("logs"); }); diff --git a/app/assets/javascripts/discourse/app/routes/group-manage-categories.js b/app/assets/javascripts/discourse/app/routes/group-manage-categories.js new file mode 100644 index 00000000000..7a03095eba9 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/group-manage-categories.js @@ -0,0 +1,10 @@ +import I18n from "I18n"; +import DiscourseRoute from "discourse/routes/discourse"; + +export default DiscourseRoute.extend({ + showFooter: true, + + titleToken() { + return I18n.t("groups.manage.categories.title"); + } +}); diff --git a/app/assets/javascripts/discourse/app/routes/group-manage-tags.js b/app/assets/javascripts/discourse/app/routes/group-manage-tags.js new file mode 100644 index 00000000000..a05c2580574 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/group-manage-tags.js @@ -0,0 +1,10 @@ +import I18n from "I18n"; +import DiscourseRoute from "discourse/routes/discourse"; + +export default DiscourseRoute.extend({ + showFooter: true, + + titleToken() { + return I18n.t("groups.manage.tags.title"); + } +}); diff --git a/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs b/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs new file mode 100644 index 00000000000..c130ce5a8a5 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/group/manage/categories.hbs @@ -0,0 +1,64 @@ +
+
+ +
{{i18n "groups.manage.categories.description"}}
+
+ +
+ + + {{category-selector + categories=model.watchingCategories + blacklist=selectedCategories + onChange=(action (mut model.watchingCategories)) + }} + +
+ {{i18n "groups.manage.categories.watched_categories_instructions"}} +
+
+ +
+ + + {{category-selector + categories=model.trackingCategories + blacklist=selectedCategories + onChange=(action (mut model.trackingCategories)) + }} + +
+ {{i18n "groups.manage.categories.tracked_categories_instructions"}} +
+
+ +
+ + + {{category-selector + categories=model.watchingFirstPostCategories + blacklist=selectedCategories + onChange=(action (mut model.watchingFirstPostCategories)) + }} + +
+ {{i18n "groups.manage.categories.watching_first_post_categories_instructions"}} +
+
+ +
+ + + {{category-selector + categories=model.mutedCategories + blacklist=selectedCategories + onChange=(action (mut model.mutedCategories)) + }} + +
+ {{i18n "groups.manage.categories.muted_categories_instructions"}} +
+
+ + {{group-manage-save-button model=model}} +
diff --git a/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs b/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs new file mode 100644 index 00000000000..6a5a91019a4 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/group/manage/tags.hbs @@ -0,0 +1,72 @@ +
+
+ +
{{i18n "groups.manage.tags.description"}}
+
+ +
+ + + {{tag-chooser + tags=model.watching_tags + blacklist=selectedTags + allowCreate=false + everyTag=true + unlimitedTagCount=true + }} + +
+ {{i18n "groups.manage.tags.watched_tags_instructions"}} +
+
+ +
+ + + {{tag-chooser + tags=model.tracking_tags + blacklist=selectedTags + allowCreate=false + everyTag=true + unlimitedTagCount=true + }} + +
+ {{i18n "groups.manage.tags.tracked_tags_instructions"}} +
+
+ +
+ + + {{tag-chooser + tags=model.watching_first_post_tags + blacklist=selectedTags + allowCreate=false + everyTag=true + unlimitedTagCount=true + }} + +
+ {{i18n "groups.manage.tags.watching_first_post_tags_instructions"}} +
+
+ +
+ + + {{tag-chooser + tags=model.muted_tags + blacklist=selectedTags + allowCreate=false + everyTag=true + unlimitedTagCount=true + }} + +
+ {{i18n "groups.manage.tags.muted_tags_instructions"}} +
+
+ + {{group-manage-save-button model=model}} +
diff --git a/app/assets/stylesheets/common/base/groups.scss b/app/assets/stylesheets/common/base/groups.scss index 2ae627b9d28..3d4e0b33abb 100644 --- a/app/assets/stylesheets/common/base/groups.scss +++ b/app/assets/stylesheets/common/base/groups.scss @@ -146,4 +146,17 @@ .control-group-inline { display: inline; } + &.groups-notifications-form { + .control-instructions { + color: $primary-medium; + margin-bottom: 10px; + font-size: $font-down-1; + line-height: $line-height-large; + } + + .category-selector, + .tag-chooser { + width: 100%; + } + } } diff --git a/app/assets/stylesheets/desktop/group.scss b/app/assets/stylesheets/desktop/group.scss index 8513149f843..9ab28f7a509 100644 --- a/app/assets/stylesheets/desktop/group.scss +++ b/app/assets/stylesheets/desktop/group.scss @@ -50,3 +50,8 @@ top: 20px; right: 20px; } + +.groups-form.groups-notifications-form { + width: 500px; + max-width: 100%; +} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 3e4ecdb1fb9..47b1c16dea2 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -604,6 +604,13 @@ class GroupsController < ApplicationController default_params end + if !automatic || current_user.admin + [:muted, :tracking, :watching, :watching_first_post].each do |level| + permitted_params << { "#{level}_category_ids" => [] } + permitted_params << { "#{level}_tags" => [] } + end + end + params.require(:group).permit(*permitted_params) end diff --git a/app/models/concerns/roleable.rb b/app/models/concerns/roleable.rb index ba900486382..cba29f7ca19 100644 --- a/app/models/concerns/roleable.rb +++ b/app/models/concerns/roleable.rb @@ -23,6 +23,7 @@ module Roleable set_permission('moderator', true) auto_approve_user enqueue_staff_welcome_message(:moderator) + set_default_notification_levels(:moderators) end def revoke_moderation! @@ -34,6 +35,7 @@ module Roleable set_permission('admin', true) auto_approve_user enqueue_staff_welcome_message(:admin) + set_default_notification_levels(:admins) end def revoke_admin! @@ -52,6 +54,13 @@ module Roleable save_and_refresh_staff_groups! end + def set_default_notification_levels(group_name) + Group.set_category_and_tag_default_notification_levels!(self, group_name) + if group_name == :admins || group_name == :moderators + Group.set_category_and_tag_default_notification_levels!(self, :staff) + end + end + private def auto_approve_user diff --git a/app/models/group.rb b/app/models/group.rb index ce3dd234ffb..9b99d4667dc 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -29,6 +29,8 @@ class Group < ActiveRecord::Base has_many :group_histories, dependent: :destroy has_many :category_reviews, class_name: 'Category', foreign_key: :reviewable_by_group_id, dependent: :nullify has_many :reviewables, foreign_key: :reviewable_by_group_id, dependent: :nullify + has_many :group_category_notification_defaults, dependent: :destroy + has_many :group_tag_notification_defaults, dependent: :destroy belongs_to :flair_upload, class_name: 'Upload' @@ -51,6 +53,7 @@ class Group < ActiveRecord::Base after_commit :trigger_group_created_event, on: :create after_commit :trigger_group_updated_event, on: :update after_commit :trigger_group_destroyed_event, on: :destroy + after_commit :set_default_notifications, on: [:create, :update] def expire_cache ApplicationSerializer.expire_cache_fragment!("group_names") @@ -382,6 +385,13 @@ class Group < ActiveRecord::Base end end + def self.set_category_and_tag_default_notification_levels!(user, group_name) + if group = lookup_group(group_name) + GroupUser.set_category_notifications(group, user) + GroupUser.set_tag_notifications(group, user) + end + end + def self.refresh_automatic_group!(name) return unless id = AUTO_GROUPS[name] @@ -755,6 +765,32 @@ class Group < ActiveRecord::Base flair_icon.presence || flair_upload&.short_path end + [:muted, :tracking, :watching, :watching_first_post].each do |level| + define_method("#{level}_category_ids=") do |category_ids| + @category_notifications ||= {} + @category_notifications[level] = category_ids + end + + define_method("#{level}_tags=") do |tag_names| + @tag_notifications ||= {} + @tag_notifications[level] = tag_names + end + end + + def set_default_notifications + if @category_notifications + @category_notifications.each do |level, category_ids| + GroupCategoryNotificationDefault.batch_set(self, level, category_ids) + end + end + + if @tag_notifications + @tag_notifications.each do |level, tag_names| + GroupTagNotificationDefault.batch_set(self, level, tag_names) + end + end + end + def imap_mailboxes return [] if self.imap_server.blank? || self.email_username.blank? || diff --git a/app/models/group_category_notification_default.rb b/app/models/group_category_notification_default.rb new file mode 100644 index 00000000000..858ef422b51 --- /dev/null +++ b/app/models/group_category_notification_default.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +class GroupCategoryNotificationDefault < ActiveRecord::Base + belongs_to :group + belongs_to :category + + def self.notification_levels + NotificationLevels.all + end + + def self.lookup(group, level) + self.where(group: group, notification_level: notification_levels[level]) + end + + def self.batch_set(group, level, category_ids) + level_num = notification_levels[level] + category_ids = Category.where(id: category_ids).pluck(:id) + + changed = false + + # Update pre-existing + if category_ids.present? && GroupCategoryNotificationDefault + .where(group_id: group.id, category_id: category_ids) + .where.not(notification_level: level_num) + .update_all(notification_level: level_num) > 0 + + changed = true + end + + # Remove extraneous category users + if GroupCategoryNotificationDefault + .where(group_id: group.id, notification_level: level_num) + .where.not(category_id: category_ids) + .delete_all > 0 + + changed = true + end + + if category_ids.present? + params = { + group_id: group.id, + level_num: level_num, + } + + sql = <<~SQL + INSERT INTO group_category_notification_defaults (group_id, category_id, notification_level) + SELECT :group_id, :category_id, :level_num + ON CONFLICT DO NOTHING + SQL + + # we could use VALUES here but it would introduce a string + # into the query, plus it is a bit of a micro optimisation + category_ids.each do |category_id| + params[:category_id] = category_id + if DB.exec(sql, params) > 0 + changed = true + end + end + end + + changed + end +end + +# == Schema Information +# +# Table name: group_category_notification_defaults +# +# id :bigint not null, primary key +# group_id :integer not null +# category_id :integer not null +# notification_level :integer not null +# +# Indexes +# +# idx_group_category_notification_defaults_unique (group_id,category_id) UNIQUE +# diff --git a/app/models/group_tag_notification_default.rb b/app/models/group_tag_notification_default.rb new file mode 100644 index 00000000000..a72d554b5be --- /dev/null +++ b/app/models/group_tag_notification_default.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +class GroupTagNotificationDefault < ActiveRecord::Base + belongs_to :group + belongs_to :tag + + def self.notification_levels + NotificationLevels.all + end + + def self.lookup(group, level) + self.where(group: group, notification_level: notification_levels[level]) + end + + def self.batch_set(group, level, tag_names) + tag_names ||= [] + changed = false + + records = self.where(group: group, notification_level: notification_levels[level]) + old_ids = records.pluck(:tag_id) + + tag_ids = tag_names.empty? ? [] : Tag.where_name(tag_names).pluck(:id) + + Tag.where_name(tag_names).joins(:target_tag).each do |tag| + tag_ids[tag_ids.index(tag.id)] = tag.target_tag_id + end + + tag_ids.uniq! + + remove = (old_ids - tag_ids) + if remove.present? + records.where('tag_id in (?)', remove).destroy_all + changed = true + end + + (tag_ids - old_ids).each do |id| + self.create!(group: group, tag_id: id, notification_level: notification_levels[level]) + changed = true + end + + changed + end +end + +# == Schema Information +# +# Table name: group_tag_notification_defaults +# +# id :bigint not null, primary key +# group_id :integer not null +# tag_id :integer not null +# notification_level :integer not null +# +# Indexes +# +# idx_group_tag_notification_defaults_unique (group_id,tag_id) UNIQUE +# diff --git a/app/models/group_user.rb b/app/models/group_user.rb index 89084984503..f2806219e10 100644 --- a/app/models/group_user.rb +++ b/app/models/group_user.rb @@ -12,6 +12,8 @@ class GroupUser < ActiveRecord::Base before_create :set_notification_level after_save :grant_trust_level + after_save :set_category_notifications + after_save :set_tag_notifications def self.notification_levels NotificationLevels.all @@ -64,6 +66,70 @@ class GroupUser < ActiveRecord::Base Promotion.recalculate(user) end + + def set_category_notifications + self.class.set_category_notifications(group, user) + end + + def self.set_category_notifications(group, user) + group_levels = group.group_category_notification_defaults.each_with_object({}) do |r, h| + h[r.notification_level] ||= [] + h[r.notification_level] << r.category_id + end + + return if group_levels.empty? + + user_levels = CategoryUser.where(user_id: user.id).each_with_object({}) do |r, h| + h[r.notification_level] ||= [] + h[r.notification_level] << r.category_id + end + + higher_level_category_ids = user_levels.values.flatten + + [:muted, :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] + CategoryUser.batch_set( + user, + level, + group_category_ids + (user_levels[level_num] || []) - higher_level_category_ids + ) + end + end + end + + def set_tag_notifications + self.class.set_tag_notifications(group, user) + end + + def self.set_tag_notifications(group, user) + group_levels = group.group_tag_notification_defaults.each_with_object({}) do |r, h| + h[r.notification_level] ||= [] + h[r.notification_level] << r.tag_id + end + + return if group_levels.empty? + + user_levels = TagUser.where(user_id: user.id).each_with_object({}) do |r, h| + h[r.notification_level] ||= [] + h[r.notification_level] << r.tag_id + end + + higher_level_tag_ids = user_levels.values.flatten + + [:muted, :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] + TagUser.batch_set( + user, + level, + group_tag_ids + (user_levels[level_num] || []) - higher_level_tag_ids + ) + end + end + end end # == Schema Information diff --git a/app/models/tag_user.rb b/app/models/tag_user.rb index b4bc5d9f179..22328e21d22 100644 --- a/app/models/tag_user.rb +++ b/app/models/tag_user.rb @@ -19,23 +19,50 @@ class TagUser < ActiveRecord::Base records = TagUser.where(user: user, notification_level: notification_levels[level]) old_ids = records.pluck(:tag_id) - tag_ids = tags.empty? ? [] : Tag.where_name(tags).pluck(:id) + tag_ids = if tags.empty? + [] + elsif tags.first&.is_a?(String) + Tag.where_name(tags).pluck(:id) + else + tags + end - Tag.where_name(tags).joins(:target_tag).each do |tag| + Tag.where(id: tag_ids).joins(:target_tag).each do |tag| tag_ids[tag_ids.index(tag.id)] = tag.target_tag_id end tag_ids.uniq! + if tag_ids.present? && + TagUser.where(user_id: user.id, tag_id: tag_ids) + .where + .not(notification_level: notification_levels[level]) + .update_all(notification_level: notification_levels[level]) > 0 + + changed = true + end + remove = (old_ids - tag_ids) if remove.present? records.where('tag_id in (?)', remove).destroy_all changed = true end - (tag_ids - old_ids).each do |id| - TagUser.create!(user: user, tag_id: id, notification_level: notification_levels[level]) - changed = true + now = Time.zone.now + + new_records_attrs = (tag_ids - old_ids).map do |tag_id| + { + user_id: user.id, + tag_id: tag_id, + notification_level: notification_levels[level], + created_at: now, + updated_at: now + } + end + + unless new_records_attrs.empty? + result = TagUser.insert_all(new_records_attrs) + changed = true if result.rows.length > 0 end if changed diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index ef999031bd6..1b2f9d0ece9 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -57,6 +57,24 @@ class BasicGroupSerializer < ApplicationSerializer :imap_old_emails, :imap_new_emails + def self.admin_or_owner_attributes(*attrs) + attributes(*attrs) + attrs.each do |attr| + define_method "include_#{attr}?" do + scope.is_admin? || (include_is_group_owner? && is_group_owner) + end + end + end + + admin_or_owner_attributes :watching_category_ids, + :tracking_category_ids, + :watching_first_post_category_ids, + :muted_category_ids, + :watching_tags, + :watching_first_post_tags, + :tracking_tags, + :muted_tags + def include_display_name? object.automatic end @@ -103,6 +121,16 @@ class BasicGroupSerializer < ApplicationSerializer scope.can_see_group_members?(object) end + [:watching, :tracking, :watching_first_post, :muted].each do |level| + define_method("#{level}_category_ids") do + GroupCategoryNotificationDefault.lookup(object, level).pluck(:category_id) + end + + define_method("#{level}_tags") do + GroupTagNotificationDefault.lookup(object, level).joins(:tag).pluck('tags.name') + end + end + private def staff? diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1bb1d5b2b28..ee1ac1a9983 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -656,6 +656,22 @@ en: membership: title: Membership access: Access + categories: + title: Categories + long_title: "Category default notifications" + description: "When users are added to this group, their category notification settings will be set to these defaults. Afterwards, they can change them." + 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." + 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 + long_title: "Tags default notifications" + description: "When users are added to this group, their tag notification settings will be set to these defaults. Afterwards, they can change them." + 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." + 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" when: "When" diff --git a/config/routes.rb b/config/routes.rb index c62265d0431..906cb2b206a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -577,6 +577,8 @@ Discourse::Application.routes.draw do manage/membership manage/interaction manage/email + manage/categories + manage/tags manage/logs }.each do |path| get path => 'groups#show' diff --git a/db/migrate/20200730205554_create_group_default_tracking.rb b/db/migrate/20200730205554_create_group_default_tracking.rb new file mode 100644 index 00000000000..1a1ac25a992 --- /dev/null +++ b/db/migrate/20200730205554_create_group_default_tracking.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CreateGroupDefaultTracking < ActiveRecord::Migration[6.0] + def change + create_table :group_category_notification_defaults do |t| + t.integer :group_id, null: false + t.integer :category_id, null: false + t.integer :notification_level, null: false + end + + add_index :group_category_notification_defaults, + [:group_id, :category_id], + unique: true, + name: :idx_group_category_notification_defaults_unique + + create_table :group_tag_notification_defaults do |t| + t.integer :group_id, null: false + t.integer :tag_id, null: false + t.integer :notification_level, null: false + end + + add_index :group_tag_notification_defaults, + [:group_id, :tag_id], + unique: true, + name: :idx_group_tag_notification_defaults_unique + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0eef371dbda..d57b8b996d4 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1054,4 +1054,121 @@ describe Group do expect(group.name).to eq("Bücherwurm") # NFC end end + + describe "default notifications" do + let(:category1) { Fabricate(:category) } + let(:category2) { Fabricate(:category) } + let(:category3) { Fabricate(:category) } + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { 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.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]) + + new_group = Fabricate.build(:group) + new_group.watching_category_ids = [category1.id, category2.id] + new_group.save! + expect(GroupCategoryNotificationDefault.lookup(new_group, :watching).pluck(:category_id)).to contain_exactly(category1.id, category2.id) + end + + it "can remove categories" do + [category1, category2].each do |category| + GroupCategoryNotificationDefault.create!( + group: group, + category: category, + notification_level: GroupCategoryNotificationDefault.notification_levels[:watching] + ) + end + + group.watching_category_ids = [category2.id] + group.save! + expect(GroupCategoryNotificationDefault.lookup(group, :watching).pluck(:category_id)).to eq([category2.id]) + + group.watching_category_ids = [] + group.save! + expect(GroupCategoryNotificationDefault.lookup(group, :watching).pluck(:category_id)).to be_empty + end + + it "can set tag notifications" do + group.watching_tags = [tag1.name, tag2.name] + group.tracking_tags = [tag3.name] + group.save! + 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]) + + new_group = Fabricate.build(:group) + new_group.watching_first_post_tags = [tag1.name, tag3.name] + new_group.save! + expect(GroupTagNotificationDefault.lookup(new_group, :watching_first_post).pluck(:tag_id)).to contain_exactly(tag1.id, tag3.id) + end + + it "can take tag synonyms" do + group.tracking_tags = [synonym1.name, synonym2.name, tag3.name] + group.save! + expect(GroupTagNotificationDefault.lookup(group, :tracking).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id, tag3.id) + + group.tracking_tags = [synonym1.name, synonym2.name, tag1.name, tag2.name, tag3.name] + group.save! + expect(GroupTagNotificationDefault.lookup(group, :tracking).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id, tag3.id) + end + + it "can remove tags" do + [tag1, tag2].each do |tag| + GroupTagNotificationDefault.create!( + group: group, + tag: tag, + notification_level: GroupTagNotificationDefault.notification_levels[:watching] + ) + end + + group.watching_tags = [tag2.name] + group.save! + expect(GroupTagNotificationDefault.lookup(group, :watching).pluck(:tag_id)).to eq([tag2.id]) + + group.watching_tags = [] + group.save! + expect(GroupTagNotificationDefault.lookup(group, :watching)).to be_empty + end + + it "can apply default notifications for admins group" do + group = Group.find(Group::AUTO_GROUPS[:admins]) + group.tracking_category_ids = [category1.id] + group.tracking_tags = [tag1.name] + group.save! + user.grant_admin! + expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([category1.id]) + expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to eq([tag1.id]) + end + + it "can apply default notifications for staff group" do + group = Group.find(Group::AUTO_GROUPS[:staff]) + group.tracking_category_ids = [category1.id] + group.tracking_tags = [tag1.name] + group.save! + user.grant_admin! + expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([category1.id]) + expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to eq([tag1.id]) + end + + it "can apply default notifications from two automatic groups" do + staff = Group.find(Group::AUTO_GROUPS[:staff]) + staff.tracking_category_ids = [category1.id] + staff.tracking_tags = [tag1.name] + staff.save! + admins = Group.find(Group::AUTO_GROUPS[:admins]) + admins.tracking_category_ids = [category2.id] + admins.tracking_tags = [tag2.name] + admins.save! + user.grant_admin! + expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to contain_exactly(category1.id, category2.id) + expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id) + end + end end diff --git a/spec/models/group_user_spec.rb b/spec/models/group_user_spec.rb index 0004aeeb84a..cd35144c9b3 100644 --- a/spec/models/group_user_spec.rb +++ b/spec/models/group_user_spec.rb @@ -32,4 +32,123 @@ describe GroupUser do expect(gu.notification_level).to eq(NotificationLevels.all[:regular]) end + describe "default category notifications" do + let(:group) { Fabricate(:group) } + let(:user) { Fabricate(:user) } + let(:category1) { Fabricate(:category) } + let(:category2) { Fabricate(:category) } + let(:category3) { Fabricate(:category) } + let(:category4) { Fabricate(:category) } + + def levels + CategoryUser.notification_levels + end + + it "doesn't change anything with no configured defaults" do + expect { group.add(user) }.to_not change { CategoryUser.count } + end + + 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.save! + expect { group.add(user) }.to change { CategoryUser.count }.by(4) + 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]) + end + + it "only upgrades notifications" do + CategoryUser.create!(user: user, category_id: category1.id, notification_level: levels[:muted]) + 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.save! + group.add(user) + h = CategoryUser.notification_levels_for(Guardian.new(user)) + expect(h[category1.id]).to eq(levels[:watching_first_post]) + 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]) + end + + it "merges notifications" do + CategoryUser.create!(user: user, category_id: category1.id, notification_level: CategoryUser.notification_levels[:tracking]) + CategoryUser.create!(user: user, category_id: category2.id, notification_level: CategoryUser.notification_levels[:watching]) + CategoryUser.create!(user: user, category_id: category4.id, notification_level: CategoryUser.notification_levels[:watching_first_post]) + group.muted_category_ids = [category3.id] + group.tracking_category_ids = [category4.id] + group.save! + group.add(user) + h = CategoryUser.notification_levels_for(Guardian.new(user)) + expect(h[category1.id]).to eq(levels[:tracking]) + expect(h[category2.id]).to eq(levels[:watching]) + expect(h[category3.id]).to eq(levels[:muted]) + expect(h[category4.id]).to eq(levels[:watching_first_post]) + end + end + + describe "default tag notifications" do + let(:group) { Fabricate(:group) } + let(:user) { Fabricate(:user) } + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let(:tag4) { Fabricate(:tag) } + let(:synonym1) { Fabricate(:tag, target_tag: tag1) } + + def levels + TagUser.notification_levels + end + + it "doesn't change anything with no configured defaults" do + expect { group.add(user) }.to_not change { TagUser.count } + end + + 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.save! + expect { group.add(user) }.to change { TagUser.count }.by(4) + 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]) + end + + it "only upgrades notifications" do + TagUser.create!(user: user, tag_id: tag1.id, notification_level: levels[:muted]) + 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.save! + group.add(user) + expect(TagUser.lookup(user, :muted).pluck(:tag_id)).to be_empty + 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) + end + + it "merges notifications" do + TagUser.create!(user: user, tag_id: tag1.id, notification_level: levels[:tracking]) + TagUser.create!(user: user, tag_id: tag2.id, notification_level: levels[:watching]) + TagUser.create!(user: user, tag_id: tag4.id, notification_level: levels[:watching_first_post]) + group.muted_tags = [tag3.name] + group.tracking_tags = [tag2.name] + group.save! + group.add(user) + expect(TagUser.lookup(user, :muted).pluck(:tag_id)).to eq([tag3.id]) + expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to eq([tag1.id]) + expect(TagUser.lookup(user, :watching).pluck(:tag_id)).to eq([tag2.id]) + expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to eq([tag4.id]) + end + end end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 0129bef8592..dc9a9e4399e 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -615,6 +615,8 @@ describe GroupsController do public_exit: false ) end + let(:category) { Fabricate(:category) } + let(:tag) { Fabricate(:tag) } context "custom_fields" do before do @@ -688,7 +690,9 @@ describe GroupsController do allow_membership_requests: true, membership_request_template: 'testing', default_notification_level: 1, - name: 'testing' + name: 'testing', + tracking_category_ids: [category.id], + tracking_tags: [tag.name] } } end.to change { GroupHistory.count }.by(13) @@ -716,6 +720,8 @@ describe GroupsController do expect(group.primary_group).to eq(false) expect(group.incoming_email).to eq(nil) expect(group.grant_trust_level).to eq(0) + expect(group.group_category_notification_defaults.first&.category).to eq(category) + expect(group.group_tag_notification_defaults.first&.tag).to eq(tag) end it 'should not be allowed to update automatic groups' do @@ -753,7 +759,9 @@ describe GroupsController do automatic_membership_email_domains: 'test.org', grant_trust_level: 2, visibility_level: 1, - members_visibility_level: 3 + members_visibility_level: 3, + tracking_category_ids: [category.id], + tracking_tags: [tag.name] } } @@ -768,6 +776,8 @@ describe GroupsController do expect(group.members_visibility_level).to eq(3) expect(group.automatic_membership_email_domains).to eq('test.org') expect(group.grant_trust_level).to eq(2) + expect(group.group_category_notification_defaults.first&.category).to eq(category) + expect(group.group_tag_notification_defaults.first&.tag).to eq(tag) expect(Jobs::AutomaticGroupMembership.jobs.first["args"].first["group_id"]) .to eq(group.id) @@ -790,7 +800,9 @@ describe GroupsController do visibility_level: 1, mentionable_level: 1, messageable_level: 1, - default_notification_level: 1 + default_notification_level: 1, + tracking_category_ids: [category.id], + tracking_tags: [tag.name] } } @@ -803,6 +815,8 @@ describe GroupsController do expect(group.mentionable_level).to eq(1) expect(group.messageable_level).to eq(1) expect(group.default_notification_level).to eq(1) + expect(group.group_category_notification_defaults.first&.category).to eq(category) + expect(group.group_tag_notification_defaults.first&.tag).to eq(tag) end it 'triggers a extensibility event' do diff --git a/test/javascripts/acceptance/group-manage-categories-test.js b/test/javascripts/acceptance/group-manage-categories-test.js new file mode 100644 index 00000000000..5bf7c88ea32 --- /dev/null +++ b/test/javascripts/acceptance/group-manage-categories-test.js @@ -0,0 +1,33 @@ +import { acceptance, updateCurrentUser } from "helpers/qunit-helpers"; + +acceptance("Managing Group Category Notification Defaults"); +QUnit.test("As an anonymous user", async assert => { + await visit("/g/discourse/manage/categories"); + + assert.ok( + count(".group-members tr") > 0, + "it should redirect to members page for an anonymous user" + ); +}); + +acceptance("Managing Group Category Notification Defaults", { loggedIn: true }); + +QUnit.test("As an admin", async assert => { + await visit("/g/discourse/manage/categories"); + + assert.ok( + find(".groups-notifications-form .category-selector").length === 4, + "it should display category inputs" + ); +}); + +QUnit.test("As a group owner", async assert => { + updateCurrentUser({ moderator: false, admin: false }); + + await visit("/g/discourse/manage/categories"); + + assert.ok( + find(".groups-notifications-form .category-selector").length === 4, + "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 new file mode 100644 index 00000000000..d5140e199b5 --- /dev/null +++ b/test/javascripts/acceptance/group-manage-tags-test.js @@ -0,0 +1,33 @@ +import { acceptance, updateCurrentUser } from "helpers/qunit-helpers"; + +acceptance("Managing Group Tag Notification Defaults"); +QUnit.test("As an anonymous user", async assert => { + await visit("/g/discourse/manage/tags"); + + assert.ok( + count(".group-members tr") > 0, + "it should redirect to members page for an anonymous user" + ); +}); + +acceptance("Managing Group Tag Notification Defaults", { loggedIn: true }); + +QUnit.test("As an admin", async assert => { + await visit("/g/discourse/manage/tags"); + + assert.ok( + find(".groups-notifications-form .tag-chooser").length === 4, + "it should display tag inputs" + ); +}); + +QUnit.test("As a group owner", async assert => { + updateCurrentUser({ moderator: false, admin: false }); + + await visit("/g/discourse/manage/tags"); + + assert.ok( + find(".groups-notifications-form .tag-chooser").length === 4, + "it should display tag inputs" + ); +});