From 15c229195f505436f86b820bff7fe63b6a5c43cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 14 Dec 2015 23:17:09 +0100 Subject: [PATCH] FEATURE: notification_level on a per-group basis --- .../group-notifications-button.js.es6 | 11 ++++++++ .../javascripts/discourse/models/group.js.es6 | 10 ++++++- .../routes/user-private-messages-group.js.es6 | 20 +++++++++++--- .../discourse/templates/user-topics-list.hbs | 13 +++++---- app/assets/stylesheets/desktop/user.scss | 4 +++ app/controllers/groups_controller.rb | 13 +++++++++ app/models/topic.rb | 4 +-- app/models/topic_notifier.rb | 8 +++--- app/models/topic_user.rb | 2 +- app/serializers/basic_group_serializer.rb | 13 ++++++++- app/serializers/listable_topic_serializer.rb | 1 + app/services/post_alerter.rb | 18 ++++++++++--- config/locales/client.en.yml | 13 +++++++++ config/routes.rb | 1 + ...2_add_notification_level_to_group_users.rb | 6 +++++ lib/topic_creator.rb | 27 ++++++++++++++----- .../admin/groups_controller_spec.rb | 3 ++- 17 files changed, 137 insertions(+), 30 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/group-notifications-button.js.es6 create mode 100644 db/migrate/20151214165852_add_notification_level_to_group_users.rb diff --git a/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 b/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 new file mode 100644 index 00000000000..6010cf02a25 --- /dev/null +++ b/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 @@ -0,0 +1,11 @@ +import NotificationsButton from 'discourse/components/notifications-button'; + +export default NotificationsButton.extend({ + classNames: ['notification-options', 'group-notification-menu'], + notificationLevel: Em.computed.alias('group.notification_level'), + i18nPrefix: 'groups.notifications', + + clicked(id) { + this.get('group').setNotification(id); + } +}); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 61744e0c67b..d0048cc9577 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -133,7 +133,15 @@ const Group = Discourse.Model.extend({ return Em.Object.create(p); }); }); - } + }, + + setNotification(notification_level) { + this.set("notification_level", notification_level); + return Discourse.ajax(`/groups/${this.get("name")}/notifications`, { + data: { notification_level }, + type: "POST" + }); + }, }); Group.reopenClass({ diff --git a/app/assets/javascripts/discourse/routes/user-private-messages-group.js.es6 b/app/assets/javascripts/discourse/routes/user-private-messages-group.js.es6 index 80f498273dd..933a4390454 100644 --- a/app/assets/javascripts/discourse/routes/user-private-messages-group.js.es6 +++ b/app/assets/javascripts/discourse/routes/user-private-messages-group.js.es6 @@ -1,13 +1,25 @@ +import Group from 'discourse/models/group'; import createPMRoute from "discourse/routes/build-user-topic-list-route"; export default createPMRoute('groups', 'private-messages-groups').extend({ model(params) { - return this.store.findFiltered("topicList", { filter: "topics/private-messages-group/" + this.modelFor("user").get("username_lower") + "/" + params.name }); + const username = this.modelFor("user").get("username_lower"); + return this.store.findFiltered("topicList", { + filter: `topics/private-messages-group/${username}/${params.name}` + }); }, - setupController(controller,model) { + afterModel(model) { + const groupName = _.last(model.get("filter").split('/')); + Group.findAll().then(groups => { + const group = _.first(groups.filterBy("name", groupName)); + this.controllerFor("user-topics-list").set("group", group) + }); + }, + + setupController(controller, model) { this._super.apply(this, arguments); - const filter = _.last(model.get("filter").split('/')); - this.controllerFor("user").set("groupFilter", filter); + const group = _.last(model.get("filter").split('/')); + this.controllerFor("user").set("groupFilter", group); } }); diff --git a/app/assets/javascripts/discourse/templates/user-topics-list.hbs b/app/assets/javascripts/discourse/templates/user-topics-list.hbs index a63fc87f94b..a2aae2de1b3 100644 --- a/app/assets/javascripts/discourse/templates/user-topics-list.hbs +++ b/app/assets/javascripts/discourse/templates/user-topics-list.hbs @@ -1,8 +1,11 @@ -{{#if showNewPM}} -
- {{fa-icon "envelope"}}{{i18n 'user.new_private_message'}} -
-{{/if}} +
+ {{#if group}} + {{group-notifications-button group=group}} + {{/if}} + {{#if showNewPM}} + {{d-button class="btn-primary pull-right new-private-message" action="composePrivateMessage" icon="envelope" label="user.new_private_message"}} + {{/if}} +
{{basic-topic-list topicList=model hideCategory=hideCategory diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index 71d1481196b..1cfdfab7ef8 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -630,6 +630,10 @@ clear: both; margin-bottom: 10px; } + .group-notification-menu .dropdown-menu { + top: 30px; + bottom: auto; + } } .paginated-topics-list { diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 68849d03daa..e33dfa449e1 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,5 +1,7 @@ class GroupsController < ApplicationController + before_filter :ensure_logged_in, only: [:set_notifications] + def show render_serialized(find_group(:id), BasicGroupSerializer) end @@ -123,6 +125,17 @@ class GroupsController < ApplicationController end + def set_notifications + group = find_group(:id) + notification_level = params.require(:notification_level) + + GroupUser.where(group_id: group.id) + .where(user_id: current_user.id) + .update_all(notification_level: notification_level) + + render json: success_json + end + private def find_group(param_name) diff --git a/app/models/topic.rb b/app/models/topic.rb index 9e4bd7d4c6e..ace0481d2cb 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -19,9 +19,9 @@ class Topic < ActiveRecord::Base def_delegator :featured_users, :choose, :feature_topic_users def_delegator :notifier, :watch!, :notify_watch! - def_delegator :notifier, :tracking!, :notify_tracking! + def_delegator :notifier, :track!, :notify_tracking! def_delegator :notifier, :regular!, :notify_regular! - def_delegator :notifier, :muted!, :notify_muted! + def_delegator :notifier, :mute!, :notify_muted! def_delegator :notifier, :toggle_mute, :toggle_mute attr_accessor :allowed_user_ids diff --git a/app/models/topic_notifier.rb b/app/models/topic_notifier.rb index ee25a159244..db30a8e471b 100644 --- a/app/models/topic_notifier.rb +++ b/app/models/topic_notifier.rb @@ -3,10 +3,10 @@ class TopicNotifier @topic = topic end - { :watch! => :watching, - :tracking! => :tracking, - :regular! => :regular, - :muted! => :muted }.each_pair do |method_name, level| + { :watch! => :watching, + :track! => :tracking, + :regular! => :regular, + :mute! => :muted }.each_pair do |method_name, level| define_method method_name do |user_id| change_level user_id, level diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index 89c452146c1..c49f13a43e3 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -103,7 +103,7 @@ class TopicUser < ActiveRecord::Base if rows == 0 now = DateTime.now - auto_track_after = User.select(:auto_track_topics_after_msecs).find_by(id: user_id).auto_track_topics_after_msecs + auto_track_after = User.select(:auto_track_topics_after_msecs).find_by(id: user_id).try(:auto_track_topics_after_msecs) auto_track_after ||= SiteSetting.default_other_auto_track_topics_after_msecs if auto_track_after >= 0 && auto_track_after <= (attrs[:total_msecs_viewed].to_i || 0) diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index cc7ea1a229c..925ac236504 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -10,9 +10,20 @@ class BasicGroupSerializer < ApplicationSerializer :primary_group, :title, :grant_trust_level, - :incoming_email + :incoming_email, + :notification_level def include_incoming_email? scope.is_staff? end + + def notification_level + # TODO: fix this N+1 + GroupUser.where(group_id: object.id, user_id: scope.user.id).first.try(:notification_level) + end + + def include_notification_level? + scope.authenticated? + end + end diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb index 94963913e22..609bf1d72a8 100644 --- a/app/serializers/listable_topic_serializer.rb +++ b/app/serializers/listable_topic_serializer.rb @@ -64,6 +64,7 @@ class ListableTopicSerializer < BasicTopicSerializer def notification_level object.user_data.notification_level end + def include_notification_level? object.user_data.present? end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 4f5c0d58c9f..2a1bd9cd197 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -15,22 +15,27 @@ class PostAlerter end def after_save_post(post, new_record = false) - - reply_to_user = post.reply_notification_target - notified = [post.user].compact if new_record && post.topic.private_message? # If it's a private message, notify the topic_allowed_users allowed_users(post).each do |user| - if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:tracking] + case TopicUser.get(post.topic, user).try(:notification_level) + when TopicUser.notification_levels[:tracking] next unless post.reply_to_post_number || post.reply_to_post.try(:user_id) == user.id + when TopicUser.notification_levels[:regular] + next unless post.reply_to_post.try(:user_id) == user.id + when TopicUser.notification_levels[:muted] + notified += [user] + next end create_notification(user, Notification.types[:private_message], post) notified += [user] end end + reply_to_user = post.reply_notification_target + if new_record && reply_to_user && post.post_type == Post.types[:regular] notify_users(reply_to_user, :replied, post) end @@ -133,6 +138,11 @@ class PostAlerter # skip if muted on the topic return if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:muted] + # skip if muted on the group + if group = opts[:group] + return if GroupUser.find_by(group_id: opts[:group_id], user_id: user.id).try(:notification_level) == TopicUser.notification_levels[:muted] + end + # Don't notify the same user about the same notification on the same post existing_notification = user.notifications .order("notifications.id desc") diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1b37af813fb..b44e809b708 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -355,6 +355,19 @@ en: trust_levels: title: "Trust level automatically granted to members when they're added:" none: "None" + notifications: + watching: + title: "Watching" + description: "You will be notified of every new post in every message, and a count of new replies will be shown." + tracking: + title: "Tracking" + description: "You will be notified if someone mentions your @name or replies to you, and a count of new replies will be shown." + regular: + title: "Normal" + description: "You will be notified if someone mentions your @name or replies to you." + muted: + title: "Muted" + description: "You will never be notified of anything about new topics in this group." user_action_groups: "1": "Likes Given" diff --git a/config/routes.rb b/config/routes.rb index e28edf8e580..c74d7740407 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -351,6 +351,7 @@ Discourse::Application.routes.draw do member do put "members" => "groups#add_members" delete "members" => "groups#remove_member" + post "notifications" => "groups#set_notifications" end end diff --git a/db/migrate/20151214165852_add_notification_level_to_group_users.rb b/db/migrate/20151214165852_add_notification_level_to_group_users.rb new file mode 100644 index 00000000000..a1f2a5d8116 --- /dev/null +++ b/db/migrate/20151214165852_add_notification_level_to_group_users.rb @@ -0,0 +1,6 @@ +class AddNotificationLevelToGroupUsers < ActiveRecord::Migration + def change + # defaults to TopicUser.notification_levels[:watching] + add_column :group_users, :notification_level, :integer, default: 3, null: false + end +end diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index a69f179387d..79a2a92ef1f 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -64,15 +64,28 @@ class TopicCreator topic.notifier.watch_topic!(topic.user_id) end - user_ids = topic.topic_allowed_users(true).pluck(:user_id) - user_ids += topic.topic_allowed_groups(true).map { |t| t.group.users.pluck(:id) }.flatten - - user_ids.uniq.reject{ |id| id == topic.user_id }.each do |user_id| - topic.notifier.watch_topic!(user_id, nil) unless user_id == -1 + topic.topic_allowed_users(true).each do |tau| + next if tau.user_id == -1 || tau.user_id == topic.user_id + topic.notifier.watch!(tau.user_id) end - CategoryUser.auto_watch_new_topic(topic) - CategoryUser.auto_track_new_topic(topic) + topic.topic_allowed_groups(true).each do |tag| + tag.group.group_users.each do |gu| + next if gu.user_id == -1 || gu.user_id == topic.user_id + action = case gu.notification_level + when TopicUser.notification_levels[:tracking] then "track!" + when TopicUser.notification_levels[:regular] then "regular!" + when TopicUser.notification_levels[:muted] then "mute!" + else "watch!" + end + topic.notifier.send(action, gu.user_id) + end + end + + unless topic.private_message? + CategoryUser.auto_watch_new_topic(topic) + CategoryUser.auto_track_new_topic(topic) + end end def setup_topic_params diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 677bc07890e..35c9420be8e 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -31,7 +31,8 @@ describe Admin::GroupsController do "title"=>nil, "primary_group"=>false, "grant_trust_level"=>nil, - "incoming_email"=>nil + "incoming_email"=>nil, + "notification_level"=>3, }]) end