Merge pull request #5226 from tgxworld/allow_user_to_disable_private_messages

FEATURE: Allow users to disable new PMs.

app/assets/javascripts/discourse/controllers/preferences/interface.js.es6

@@ -13,7 +13,8 @@ export default Ember.Controller.extend(PreferencesTabController, {
       'dynamic_favicon',
       'enable_quoting',
       'disable_jump_reply',
-      'automatically_unpin_topics'
+      'automatically_unpin_topics',
+      'allow_private_messages',
     ];
 
     if (makeDefault) {

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -231,7 +231,11 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, {
     },
 
     removeAllowedUser(user) {
-      return this.get('model.details').removeAllowedUser(user);
+      return this.get('model.details').removeAllowedUser(user).then(() => {
+        if (this.currentUser.id === user.id) {
+          this.transitionToRoute("userPrivateMessages", user);
+        }
+      });
     },
 
     removeAllowedGroup(group) {

app/assets/javascripts/discourse/lib/transform-post.js.es6

@@ -127,6 +127,7 @@ export default function transformPost(currentUser, site, post, prevPost, nextPos
     postAtts.allowedGroups = details.allowed_groups;
     postAtts.allowedUsers = details.allowed_users;
     postAtts.canRemoveAllowedUsers = details.can_remove_allowed_users;
+    postAtts.canRemoveSelfId = details.can_remove_self_id;
     postAtts.canInvite = details.can_invite_to;
   }
 

app/assets/javascripts/discourse/models/user.js.es6

@@ -247,7 +247,8 @@ const User = RestModel.extend({
       'notification_level_when_replying',
       'like_notification_frequency',
       'include_tl0_in_digests',
-      'theme_key'
+      'theme_key',
+      'allow_private_messages',
     ];
 
     if (fields) {

app/assets/javascripts/discourse/templates/preferences/interface.hbs

@@ -10,6 +10,16 @@
 </div>
 {{/if}}
 
+<div class="control-group private-messages">
+  <label class="control-label">{{i18n 'user.private_messages'}}</label>
+
+  <div class="controls">
+    {{preference-checkbox
+        labelKey="user.allow_private_messages"
+        checked=model.user_option.allow_private_messages}}
+  </div>
+</div>
+
 {{#if siteSettings.allow_user_locale}}
   <div class="control-group pref-locale">
     <label class="control-label">{{i18n 'user.locale.title'}}</label>

app/assets/javascripts/discourse/widgets/post-small-action.js.es6

@@ -48,6 +48,7 @@ const icons = {
   'split_topic': 'sign-out',
   'invited_user': 'plus-circle',
   'invited_group': 'plus-circle',
+  'user_left': 'minus-circle',
   'removed_user': 'minus-circle',
   'removed_group': 'minus-circle',
   'public_topic': 'comment',

app/assets/javascripts/discourse/widgets/private-message-map.js.es6

@@ -36,8 +36,12 @@ createWidget('pm-remove-link', {
   template: hbs`{{d-icon "times"}}`,
 
   click() {
-    bootbox.confirm(I18n.t("private_message_info.remove_allowed_user", {name: this.attrs.username}), (confirmed) => {
-      if (confirmed) { this.sendWidgetAction('removeAllowedUser', this.attrs); }
+    const messageKey = this.attrs.isCurrentUser ? 'leave_message' : 'remove_allowed_user';
+
+    bootbox.confirm(I18n.t(`private_message_info.${messageKey}`, { name: this.attrs.user.username }), confirmed => {
+      if (confirmed) {
+        this.sendWidgetAction('removeAllowedUser', this.attrs.user);
+      }
     });
   }
 });
@@ -49,11 +53,12 @@ createWidget('pm-map-user', {
     const user = attrs.user;
     const avatar = avatarFor('small', { template: user.avatar_template, username: user.username });
     const link = h('a', { attributes: { href: user.get('path') } }, [ avatar, ' ', user.username ]);
-
     const result = [link];
-    if (attrs.canRemoveAllowedUsers) {
+    const isCurrentUser = attrs.canRemoveSelfId === user.get('id');
+
+    if (attrs.canRemoveAllowedUsers || isCurrentUser) {
       result.push(' ');
-      result.push(this.attach('pm-remove-link', user));
+      result.push(this.attach('pm-remove-link', { user, isCurrentUser } ));
     }
 
     return result;
@@ -67,12 +72,23 @@ export default createWidget('private-message-map', {
     const participants = [];
 
     if (attrs.allowedGroups.length) {
-      participants.push(attrs.allowedGroups.map(ag => this.attach('pm-map-user-group', {group: ag, canRemoveAllowedUsers: attrs.canRemoveAllowedUsers})));
+      participants.push(attrs.allowedGroups.map(ag => {
+        this.attach('pm-map-user-group', {
+          group: ag,
+          canRemoveAllowedUsers: attrs.canRemoveAllowedUsers
+        });
+      }));
     }
 
-    if (attrs.allowedUsers.length) {
+    const allowedUsersLength = attrs.allowedUsers.length;
+
+    if (allowedUsersLength) {
       participants.push(attrs.allowedUsers.map(au => {
-        return this.attach('pm-map-user', { user: au, canRemoveAllowedUsers: attrs.canRemoveAllowedUsers });
+        return this.attach('pm-map-user', {
+          user: au,
+          canRemoveAllowedUsers: attrs.canRemoveAllowedUsers,
+          canRemoveSelfId: attrs.canRemoveSelfId
+        });
       }));
     }
 

app/controllers/topics_controller.rb

@@ -439,9 +439,10 @@ class TopicsController < ApplicationController
   def remove_allowed_user
     params.require(:username)
     topic = Topic.find_by(id: params[:topic_id])
-    guardian.ensure_can_remove_allowed_users!(topic)
+    user = User.find_by(username: params[:username])
+    guardian.ensure_can_remove_allowed_users!(topic, user)
 
-    if topic.remove_allowed_user(current_user, params[:username])
+    if topic.remove_allowed_user(current_user, user)
       render json: success_json
     else
       render json: failed_json, status: 422

app/models/topic.rb

@@ -708,14 +708,21 @@ SQL
   end
 
   def remove_allowed_user(removed_by, username)
-    if user = User.find_by(username: username)
+    user = username.is_a?(User) ? username : User.find_by(username: username)
+
+    if user
       topic_user = topic_allowed_users.find_by(user_id: user.id)
+
       if topic_user
         topic_user.destroy
-        # we can not remove ourselves cause then we will end up adding
-        # ourselves in add_small_action
-        removed_by = Discourse.system_user if user.id == removed_by&.id
+
+        if user.id == removed_by&.id
+          removed_by = Discourse.system_user
+          add_small_action(removed_by, "user_left", user.username)
+        else
           add_small_action(removed_by, "removed_user", user.username)
+        end
+
         return true
       end
     end

app/models/user_option.rb

@@ -59,11 +59,6 @@ class UserOption < ActiveRecord::Base
     super
   end
 
-  def update_tracked_topics
-    return unless saved_change_to_auto_track_topics_after_msecs?
-    TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call
-  end
-
   def redirected_to_top_yet?
     last_redirected_to_top_at.present?
   end
@@ -133,6 +128,13 @@ class UserOption < ActiveRecord::Base
     times.max
   end
 
+  private
+
+    def update_tracked_topics
+      return unless saved_change_to_auto_track_topics_after_msecs?
+      TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call
+    end
+
 end
 
 # == Schema Information
@@ -162,6 +164,7 @@ end
 #  notification_level_when_replying :integer
 #  theme_key                        :string
 #  theme_key_seq                    :integer          default(0), not null
+#  allow_private_messages           :boolean          default(TRUE), not null
 #
 # Indexes
 #

app/serializers/topic_view_serializer.rb

@@ -114,6 +114,7 @@ class TopicViewSerializer < ApplicationSerializer
     result[:can_delete] = true if scope.can_delete?(object.topic)
     result[:can_recover] = true if scope.can_recover_topic?(object.topic)
     result[:can_remove_allowed_users] = true if scope.can_remove_allowed_users?(object.topic)
+    result[:can_remove_self_id] = scope.user.id if scope.can_remove_allowed_users?(object.topic, scope.user)
     result[:can_invite_to] = true if scope.can_invite_to?(object.topic)
     result[:can_invite_via_email] = true if scope.can_invite_via_email?(object.topic)
     result[:can_create_post] = true if scope.can_create?(Post, object.topic)

app/serializers/user_option_serializer.rb

@@ -20,7 +20,8 @@ class UserOptionSerializer < ApplicationSerializer
              :like_notification_frequency,
              :include_tl0_in_digests,
              :theme_key,
-             :theme_key_seq
+             :theme_key_seq,
+             :allow_private_messages,
 
   def auto_track_topics_after_msecs
     object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs

app/services/user_updater.rb

@@ -34,7 +34,8 @@ class UserUpdater
     :email_in_reply_to,
     :like_notification_frequency,
     :include_tl0_in_digests,
-    :theme_key
+    :theme_key,
+    :allow_private_messages,
   ]
 
   def initialize(actor, user)

config/locales/client.en.yml

@@ -143,6 +143,7 @@ en:
       split_topic: "split this topic %{when}"
       invited_user: "invited %{who} %{when}"
       invited_group: "invited %{who} %{when}"
+      user_left: "%{who} left this message %{when}"
       removed_user: "removed %{who} %{when}"
       removed_group: "removed %{who} %{when}"
       autoclosed:
@@ -595,6 +596,7 @@ en:
       disable_jump_reply: "Don't jump to my post after I reply"
       dynamic_favicon: "Show new / updated topic count on browser icon"
       theme_default_on_all_devices: "Make this my default theme on all my devices"
+      allow_private_messages: "Allow other users to send me private messages"
       external_links_in_new_tab: "Open all external links in a new tab"
       enable_quoting: "Enable quote reply for highlighted text"
       change: "change"
@@ -1029,6 +1031,7 @@ en:
     private_message_info:
       title: "Message"
       invite: "Invite Others..."
+      leave_message: "Do you really want to leave this message?"
       remove_allowed_user: "Do you really want to remove {{name}} from this message?"
       remove_allowed_group: "Do you really want to remove {{name}} from this message?"
 

db/migrate/20171006030028_add_allow_private_messages_to_user_options.rb

@@ -0,0 +1,5 @@
+class AddAllowPrivateMessagesToUserOptions < ActiveRecord::Migration[5.1]
+  def change
+    add_column :user_options, :allow_private_messages, :boolean, default: true, null: false
+  end
+end

lib/guardian.rb

@@ -286,17 +286,22 @@ class Guardian
   end
 
   def can_send_private_message?(target)
-    (target.is_a?(Group) || target.is_a?(User)) &&
+    is_user = target.is_a?(User)
+    is_group = target.is_a?(Group)
+
+    (is_group || is_user) &&
     # User is authenticated
     authenticated? &&
     # Have to be a basic level at least, or are contacting moderators
     (@user.has_trust_level?(SiteSetting.min_trust_to_send_messages) ||
       (target.is_a?(User) && target.moderator?) ||
       (target.name == Group[:moderators].name)) &&
+    # User disabled private message
+    (is_staff? || is_group || target.user_option.allow_private_messages) &&
     # PMs are enabled
     (is_staff? || SiteSetting.enable_private_messages) &&
     # Can't send PMs to suspended users
-    (is_staff? || target.is_a?(Group) || !target.suspended?) &&
+    (is_staff? || is_group || !target.suspended?) &&
     # Blocked users can only send PM to staff
     (!is_blocked? || target.staff?)
   end

lib/guardian/topic_guardian.rb

@@ -1,8 +1,13 @@
 #mixin for all guardian methods dealing with topic permisions
 module TopicGuardian
 
-  def can_remove_allowed_users?(topic)
-    is_staff?
+  def can_remove_allowed_users?(topic, target_user = nil)
+    is_staff? ||
+    (
+      topic.allowed_users.count > 1 &&
+      topic.user != target_user &&
+      !!(target_user && user == target_user)
+    )
   end
 
   # Creating Methods

lib/post_creator.rb

@@ -109,10 +109,20 @@ class PostCreator
       # Make sure none of the users have muted the creator
       users = User.where(username: names).pluck(:id, :username).to_h
 
-      MutedUser.where(user_id: users.keys, muted_user_id: @user.id).pluck(:user_id).each do |m|
+      User
+        .joins("LEFT JOIN user_options ON user_options.user_id = users.id")
+        .joins("LEFT JOIN muted_users ON muted_users.muted_user_id = #{@user.id.to_i}")
+        .where("user_options.user_id IS NOT NULL")
+        .where("
+          (user_options.user_id IN (:user_ids) AND NOT user_options.allow_private_messages) OR
+          muted_users.user_id IN (:user_ids)
+        ", user_ids: users.keys)
+        .pluck(:id).each do |m|
+
         errors[:base] << I18n.t(:not_accepting_pms, username: users[m])
-        return false
       end
+
+      return false if errors[:base].present?
     end
 
     if new_topic?

lib/topic_creator.rb

@@ -190,7 +190,7 @@ class TopicCreator
     names = usernames.split(',').flatten
     len = 0
 
-    User.where(username: names).each do |user|
+    User.includes(:user_option).where(username: names).find_each do |user|
       check_can_send_permission!(topic, user)
       @added_users << user
       topic.topic_allowed_users.build(user_id: user.id)

spec/components/guardian_spec.rb

@@ -233,6 +233,27 @@ describe Guardian do
         end
       end
     end
+
+    context 'target user has private message disabled' do
+      before do
+        another_user.user_option.update!(allow_private_messages: false)
+      end
+
+      context 'for a normal user' do
+        it 'should return false' do
+          expect(Guardian.new(user).can_send_private_message?(another_user)).to eq(false)
+        end
+      end
+
+      context 'for a staff user' do
+        it 'should return true' do
+          [admin, moderator].each do |staff_user|
+            expect(Guardian.new(staff_user).can_send_private_message?(another_user))
+              .to eq(true)
+          end
+        end
+      end
+    end
   end
 
   describe 'can_reply_as_new_topic' do
@@ -2592,4 +2613,60 @@ describe Guardian do
       end
     end
   end
+
+  describe '#can_remove_allowed_users?' do
+    context 'staff users' do
+      it 'should be true' do
+        expect(Guardian.new(moderator).can_remove_allowed_users?(topic))
+          .to eq(true)
+      end
+    end
+
+    context 'normal user' do
+      let(:topic) { Fabricate(:topic, user: Fabricate(:user)) }
+      let(:another_user) { Fabricate(:user) }
+
+      before do
+        topic.allowed_users << user
+        topic.allowed_users << another_user
+      end
+
+      it 'should be false' do
+        expect(Guardian.new(user).can_remove_allowed_users?(topic))
+          .to eq(false)
+      end
+
+      describe 'target_user is the user' do
+        describe 'when user is in a pm with another user' do
+          it 'should return true' do
+            expect(Guardian.new(user).can_remove_allowed_users?(topic, user))
+              .to eq(true)
+          end
+        end
+
+        describe 'when user is the creator of the topic' do
+          it 'should return false' do
+            expect(Guardian.new(topic.user).can_remove_allowed_users?(topic, topic.user))
+              .to eq(false)
+          end
+        end
+
+        describe 'when user is the only user in the topic' do
+          it 'should return false' do
+            topic.remove_allowed_user(Discourse.system_user, another_user.username)
+
+            expect(Guardian.new(user).can_remove_allowed_users?(topic, user))
+              .to eq(false)
+          end
+        end
+      end
+
+      describe 'target_user is not the user' do
+        it 'should return false' do
+          expect(Guardian.new(user).can_remove_allowed_users?(topic, moderator))
+            .to eq(false)
+        end
+      end
+    end
+  end
 end

spec/components/post_creator_spec.rb

@@ -954,6 +954,30 @@ describe PostCreator do
     end
   end
 
+  context 'private message to a user that has disabled private messages' do
+    let(:another_user) { Fabricate(:user) }
+
+    before do
+      another_user.user_option.update!(allow_private_messages: false)
+    end
+
+    it 'should not be valid' do
+      post_creator = PostCreator.new(
+        user,
+        title: 'this message is to someone who muted me!',
+        raw: "you will have to see this even if you muted me!",
+        archetype: Archetype.private_message,
+        target_usernames: "#{another_user.username}"
+      )
+
+      expect(post_creator).to_not be_valid
+
+      expect(post_creator.errors.full_messages).to include(I18n.t(
+        "not_accepting_pms", username: another_user.username
+      ))
+    end
+  end
+
   context "private message to a muted user" do
     let(:muted_me) { Fabricate(:evil_trout) }
 

spec/models/topic_spec.rb

@@ -6,6 +6,7 @@ require_dependency 'post_destroyer'
 describe Topic do
   let(:now) { Time.zone.local(2013, 11, 20, 8, 0) }
   let(:user) { Fabricate(:user) }
+  let(:topic) { Fabricate(:topic) }
 
   context 'validations' do
     let(:topic) { Fabricate.build(:topic) }
@@ -2061,4 +2062,23 @@ describe Topic do
       end
     end
   end
+
+  describe '#remove_allowed_user' do
+    let(:another_user) { Fabricate(:user) }
+
+    describe 'removing oneself' do
+      it 'should remove onself' do
+        topic.allowed_users << another_user
+
+        expect(topic.remove_allowed_user(another_user, another_user)).to eq(true)
+        expect(topic.allowed_users.include?(another_user)).to eq(false)
+
+        post = Post.last
+
+        expect(post.user).to eq(Discourse.system_user)
+        expect(post.post_type).to eq(Post.types[:small_action])
+        expect(post.action_code).to eq('user_left')
+      end
+    end
+  end
 end

spec/services/user_updater_spec.rb

@@ -76,8 +76,9 @@ describe UserUpdater do
                            notification_level_when_replying: 3,
                            email_in_reply_to: false,
                            date_of_birth: date_of_birth,
-                           theme_key: theme.key
-                    )
+                           theme_key: theme.key,
+                           allow_private_messages: false)
+
       expect(val).to be_truthy
 
       user.reload
@@ -92,6 +93,7 @@ describe UserUpdater do
       expect(user.user_option.email_in_reply_to).to eq false
       expect(user.user_option.theme_key).to eq theme.key
       expect(user.user_option.theme_key_seq).to eq(seq + 1)
+      expect(user.user_option.allow_private_messages).to eq(false)
       expect(user.date_of_birth).to eq(date_of_birth.to_date)
     end