github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 08:49:06 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FEATURE: Allow List for PMs (#10270)

Browse Source 
* FEATURE: Allow List for PMs

This feature adds a new user setting that is disabled by default that
allows them to specify a list of users that are allowed to send them
private messages. This way they don't have to maintain a large list of
users they don't want to here from and instead just list the people they
know they do want. Staff will still always be able to send messages to
the user.

* Update PR based on feedback
This commit is contained in:
Blake Erickson 2020-07-20 15:23:49 -06:00 committed by GitHub
parent 2abfd30d22
commit 690f17bcbe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 316 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/discourse/app/controllers/preferences/interface.js
View File

@ -38,6 +38,7 @@ export default Controller.extend({
"enable_defer",
"automatically_unpin_topics",
"allow_private_messages",
"enable_allowed_pm_users",
"homepage_id",
"hide_profile_and_presence",
"text_size",

9
app/assets/javascripts/discourse/app/controllers/preferences/notifications.js
View File

@ -2,6 +2,7 @@ import I18n from "I18n";
import Controller from "@ember/controller";
import { NotificationLevels } from "discourse/lib/notification-levels";
import { popupAjaxError } from "discourse/lib/ajax-error";
import discourseComputed from "discourse-common/utils/decorators";
export default Controller.extend({
init() {
@ -14,7 +15,8 @@ export default Controller.extend({
"auto_track_topics_after_msecs",
"notification_level_when_replying",
"like_notification_frequency",
"allow_private_messages"
"allow_private_messages",
"enable_allowed_pm_users"
];
this.likeNotificationFrequencies = [
@ -91,6 +93,11 @@ export default Controller.extend({
this.isIOS = caps.isIOS;
},
@discourseComputed("model.user_option.allow_private_messages")
disableAllowPmUsersSetting(allowPrivateMessages) {
return !allowPrivateMessages;
},
actions: {
save() {
this.set("saved", false);

30
app/assets/javascripts/discourse/app/controllers/preferences/users.js
View File

@ -1,13 +1,18 @@
import { makeArray } from "discourse-common/lib/helpers";
import { alias, gte, or } from "@ember/object/computed";
import { alias, gte, or, and } from "@ember/object/computed";
import { action, computed } from "@ember/object";
import Controller from "@ember/controller";
import { popupAjaxError } from "discourse/lib/ajax-error";
export default Controller.extend({
ignoredUsernames: alias("model.ignored_usernames"),
allowedPmUsernames: alias("model.allowed_pm_usernames"),
userIsMemberOrAbove: gte("model.trust_level", 2),
ignoredEnabled: or("userIsMemberOrAbove", "model.staff"),
allowPmUsersEnabled: and(
"model.user_option.enable_allowed_pm_users",
"model.user_option.allow_private_messages"
),
mutedUsernames: computed("model.muted_usernames", {
get() {
@ -21,10 +26,26 @@ export default Controller.extend({
}
}),
allowedPmUsernames: computed("model.allowed_pm_usernames", {
get() {
let usernames = this.model.allowed_pm_usernames;
if (typeof usernames === "string") {
usernames = usernames.split(",").filter(Boolean);
}
return makeArray(usernames).uniq();
}
}),
init() {
this._super(...arguments);
this.saveAttrNames = ["muted_usernames", "ignored_usernames"];
this.saveAttrNames = [
"muted_usernames",
"ignored_usernames",
"allowed_pm_usernames"
];
},
@action
@ -32,6 +53,11 @@ export default Controller.extend({
this.model.set("muted_usernames", usernames.uniq().join(","));
},
@action
onChangeAllowedPmUsernames(usernames) {
this.model.set("allowed_pm_usernames", usernames.uniq().join(","));
},
@action
save() {
this.set("saved", false);

2
app/assets/javascripts/discourse/app/models/user.js
View File

@ -276,6 +276,7 @@ const User = RestModel.extend({
"user_fields",
"muted_usernames",
"ignored_usernames",
"allowed_pm_usernames",
"profile_background_upload_url",
"card_background_upload_url",
"muted_tags",
@ -311,6 +312,7 @@ const User = RestModel.extend({
"include_tl0_in_digests",
"theme_ids",
"allow_private_messages",
"enable_allowed_pm_users",
"homepage_id",
"hide_profile_and_presence",
"text_size",

8
app/assets/javascripts/discourse/app/templates/preferences/notifications.hbs
View File

@ -61,7 +61,15 @@
labelKey="user.allow_private_messages"
checked=model.user_option.allow_private_messages}}
</div>
<div class="controls">
{{preference-checkbox
labelKey="user.allow_private_messages_from_specific_users"
checked=model.user_option.enable_allowed_pm_users
disabled=disableAllowPmUsersSetting}}
</div>
</div>
{{/if}}
{{plugin-outlet name="user-preferences-notifications" args=(hash model=model save=(action "save"))}}

19
app/assets/javascripts/discourse/app/templates/preferences/users.hbs
View File

@ -25,6 +25,25 @@
<div class="instructions">{{i18n "user.muted_users_instructions"}}</div>
</div>
{{#if allowPmUsersEnabled}}
<div class="control-group user-allow-pm">
<div class="controls tracking-controls">
<label>
{{d-icon "far-envelope" class="icon"}}
<span>{{i18n "user.allowed_pm_users"}}</span>
</label>
{{user-chooser
value=allowedPmUsernames
onChange=(action "onChangeAllowedPmUsernames")
options=(hash
excludeCurrentUser=true
)
}}
</div>
<div class="instructions">{{i18n "user.allowed_pm_users_instructions"}}</div>
</div>
{{/if}}
{{plugin-outlet name="user-custom-controls" args=(hash model=model)}}
{{#save-controls model=model action=(action "save") saved=saved}}{{/save-controls}}

1
app/controllers/users_controller.rb
View File

@ -1581,6 +1581,7 @@ class UsersController < ApplicationController
:date_of_birth,
:muted_usernames,
:ignored_usernames,
:allowed_pm_usernames,
:theme_ids,
:locale,
:bio_raw,

22
app/models/allowed_pm_user.rb Normal file
View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
class AllowedPmUser < ActiveRecord::Base
belongs_to :user
belongs_to :allowed_pm_user, class_name: "User"
end
# == Schema Information
#
# Table name: allowed_pm_users
#
# id :bigint not null, primary key
# user_id :integer not null
# allowed_pm_user_id :integer not null
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_allowed_pm_users_on_allowed_pm_user_id_and_user_id (allowed_pm_user_id,user_id) UNIQUE
# index_allowed_pm_users_on_user_id_and_allowed_pm_user_id (user_id,allowed_pm_user_id) UNIQUE
#

1
app/models/user_option.rb
View File

@ -227,6 +227,7 @@ end
# title_count_mode_key :integer default(0), not null
# enable_defer :boolean default(FALSE), not null
# timezone :string
# enable_allowed_pm_users :boolean default(FALSE), not null
#
# Indexes
#

1
app/serializers/user_option_serializer.rb
View File

@ -23,6 +23,7 @@ class UserOptionSerializer < ApplicationSerializer
:theme_ids,
:theme_key_seq,
:allow_private_messages,
:enable_allowed_pm_users,
:homepage_id,
:hide_profile_and_presence,
:text_size,

5
app/serializers/user_serializer.rb
View File

@ -49,6 +49,7 @@ class UserSerializer < UserCardSerializer
:has_title_badges,
:muted_usernames,
:ignored_usernames,
:allowed_pm_usernames,
:mailing_list_posts_per_day,
:can_change_bio,
:can_change_location,
@ -228,6 +229,10 @@ class UserSerializer < UserCardSerializer
IgnoredUser.where(user_id: object.id).joins(:ignored_user).pluck(:username)
end
def allowed_pm_usernames
AllowedPmUser.where(user_id: object.id).joins(:allowed_pm_user).pluck(:username)
end
def system_avatar_upload_id
# should be left blank
end

27
app/services/user_updater.rb
View File

@ -37,6 +37,7 @@ class UserUpdater
:include_tl0_in_digests,
:theme_ids,
:allow_private_messages,
:enable_allowed_pm_users,
:homepage_id,
:hide_profile_and_presence,
:text_size,
@ -164,6 +165,10 @@ class UserUpdater
update_ignored_users(attributes[:ignored_usernames])
end
if attributes.key?(:allowed_pm_usernames)
update_allowed_pm_users(attributes[:allowed_pm_usernames])
end
name_changed = user.name_changed?
if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
(name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0)
@ -225,6 +230,28 @@ class UserUpdater
end
end
def update_allowed_pm_users(usernames)
#return unless guardian.can_ignore_users?
usernames ||= ""
desired_usernames = usernames.split(",").reject { |username| user.username == username }
desired_ids = User.where(username: desired_usernames).where(admin: false, moderator: false).pluck(:id)
if desired_ids.empty?
AllowedPmUser.where(user_id: user.id).destroy_all
else
AllowedPmUser.where('user_id = ? AND allowed_pm_user_id not in (?)', user.id, desired_ids).destroy_all
# SQL is easier here than figuring out how to do the same in AR
DB.exec(<<~SQL, now: Time.zone.now, user_id: user.id, desired_ids: desired_ids)
INSERT into allowed_pm_users(user_id, allowed_pm_user_id, created_at, updated_at)
SELECT :user_id, id, :now, :now
FROM users
WHERE id in (:desired_ids)
ON CONFLICT DO NOTHING
SQL
end
end
private
attr_reader :user, :guardian

3
config/locales/client.en.yml
View File

@ -975,6 +975,9 @@ en:
users: "Users"
muted_users: "Muted"
muted_users_instructions: "Suppress all notifications and PMs from these users."
allowed_pm_users: "Allowed"
allowed_pm_users_instructions: "Only allow PMs from these users."
allow_private_messages_from_specific_users: "Only allow specific users to send me personal messages"
ignored_users: "Ignored"
ignored_users_instructions: "Suppress all posts, notifications, and PMs from these users."
tracked_topics_link: "Show"

13
db/migrate/20200717193118_create_allowed_pm_users.rb Normal file
View File

@ -0,0 +1,13 @@
# frozen_string_literal: true
class CreateAllowedPmUsers < ActiveRecord::Migration[6.0]
def change
create_table :allowed_pm_users do |t|
t.integer :user_id, null: false
t.integer :allowed_pm_user_id, null: false
t.timestamps null: false
end
add_index :allowed_pm_users, [:user_id, :allowed_pm_user_id], unique: true
add_index :allowed_pm_users, [:allowed_pm_user_id, :user_id], unique: true
end
end

7
db/migrate/20200718154308_add_enable_allowed_pm_users_to_user_options.rb Normal file
View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddEnableAllowedPmUsersToUserOptions < ActiveRecord::Migration[6.0]
def change
add_column :user_options, :enable_allowed_pm_users, :boolean, default: false, null: false
end
end

30
lib/post_creator.rb
View File

@ -110,7 +110,7 @@ class PostCreator
return false
end
# Make sure none of the users have muted the creator
# Make sure none of the users have muted or ignored the creator
users = User.where(username_lower: names).pluck(:id, :username).to_h
User
@ -128,6 +128,23 @@ class PostCreator
errors.add(:base, I18n.t(:not_accepting_pms, username: users[m]))
end
# Is Allowed PM users list enabled for any recipients?
users_with_allowed_pms = allowed_pms_enabled(users).pluck(:id).uniq
# If any of the users has allowed_pm_users enabled check to see if the creator
# is in their list
if users_with_allowed_pms.any?
users_sender_can_pm = allowed_pms_enabled(users)
.where("allowed_pm_users.allowed_pm_user_id" => @user.id.to_i)
.pluck(:id).uniq
# If not in the list add an error
users_not_allowed = users_with_allowed_pms - users_sender_can_pm
users_not_allowed.each do |id|
errors.add(:base, I18n.t(:not_accepting_pms, username: users[id]))
end
end
return false if errors[:base].present?
end
@ -430,6 +447,17 @@ class PostCreator
private
def allowed_pms_enabled(users)
User
.joins("LEFT JOIN user_options ON user_options.user_id = users.id")
.joins("LEFT JOIN allowed_pm_users ON allowed_pm_users.user_id = users.id")
.where("
user_options.user_id IS NOT NULL AND
user_options.user_id IN (:user_ids) AND
user_options.enable_allowed_pm_users
", user_ids: users.keys)
end
def create_topic
return if @topic
begin

135
spec/components/post_creator_spec.rb
View File

@ -1321,6 +1321,141 @@ describe PostCreator do
end
context "private message to user in allow list" do
fab!(:sender) { Fabricate(:evil_trout) }
fab!(:allowed_user) { Fabricate(:user) }
context "when post author is allowed" do
let!(:allowed_pm_user) { Fabricate(:allowed_pm_user, user: allowed_user, allowed_pm_user: sender) }
it 'should succeed' do
allowed_user.user_option.update!(enable_allowed_pm_users: true)
pc = PostCreator.new(
sender,
title: 'this message is to someone who is in my allow list!',
raw: "you will have to see this because I'm in your allow list!",
archetype: Archetype.private_message,
target_usernames: "#{allowed_user.username}"
)
expect(pc).to be_valid
expect(pc.errors).to be_blank
end
end
context "when personal messages are disabled" do
let!(:allowed_pm_user) { Fabricate(:allowed_pm_user, user: allowed_user, allowed_pm_user: sender) }
it 'should fail' do
allowed_user.user_option.update!(allow_private_messages: false)
allowed_user.user_option.update!(enable_allowed_pm_users: true)
pc = PostCreator.new(
sender,
title: 'this message is to someone who is in my allow list!',
raw: "you will have to see this because I'm in your allow list!",
archetype: Archetype.private_message,
target_usernames: "#{allowed_user.username}"
)
expect(pc).not_to be_valid
expect(pc.errors.full_messages).to contain_exactly(
I18n.t(:not_accepting_pms, username: allowed_user.username)
)
end
end
end
context "private message to user not in allow list" do
fab!(:sender) { Fabricate(:evil_trout) }
fab!(:allowed_user) { Fabricate(:user) }
fab!(:not_allowed_user) { Fabricate(:user) }
context "when post author is not allowed" do
let!(:allowed_pm_user) { Fabricate(:allowed_pm_user, user: not_allowed_user, allowed_pm_user: allowed_user) }
it 'should fail' do
not_allowed_user.user_option.update!(enable_allowed_pm_users: true)
pc = PostCreator.new(
sender,
title: 'this message is to someone who is not in my allowed list!',
raw: "you will have to see this even if you don't want message from me!",
archetype: Archetype.private_message,
target_usernames: "#{not_allowed_user.username}"
)
expect(pc).not_to be_valid
expect(pc.errors.full_messages).to contain_exactly(
I18n.t(:not_accepting_pms, username: not_allowed_user.username)
)
end
it 'should succeed when not enabled' do
not_allowed_user.user_option.update!(enable_allowed_pm_users: false)
pc = PostCreator.new(
sender,
title: 'this message is to someone who is not in my allowed list!',
raw: "you will have to see this even if you don't want message from me!",
archetype: Archetype.private_message,
target_usernames: "#{not_allowed_user.username}"
)
expect(pc).to be_valid
expect(pc.errors).to be_blank
end
end
end
context "private message when post author is admin who is not in allow list" do
fab!(:staff_user) { Fabricate(:admin) }
fab!(:allowed_user) { Fabricate(:user) }
fab!(:not_allowed_user) { Fabricate(:user) }
fab!(:allowed_pm_user) { Fabricate(:allowed_pm_user, user: staff_user, allowed_pm_user: allowed_user) }
it 'succeeds if the user is staff' do
pc = PostCreator.new(
staff_user,
title: 'this message is to someone who did not allow me!',
raw: "you will have to see this even if you did not allow me!",
archetype: Archetype.private_message,
target_usernames: "#{not_allowed_user.username}"
)
expect(pc).to be_valid
expect(pc.errors).to be_blank
end
end
context "private message to multiple users and one is not allowed" do
fab!(:sender) { Fabricate(:evil_trout) }
fab!(:allowed_user) { Fabricate(:user) }
fab!(:not_allowed_user) { Fabricate(:user) }
context "when post author is not allowed" do
let!(:allowed_pm_user) { Fabricate(:allowed_pm_user, user: allowed_user, allowed_pm_user: sender) }
it 'should fail' do
allowed_user.user_option.update!(enable_allowed_pm_users: true)
not_allowed_user.user_option.update!(enable_allowed_pm_users: true)
pc = PostCreator.new(
sender,
title: 'this message is to someone who is not in my allowed list!',
raw: "you will have to see this even if you don't want message from me!",
archetype: Archetype.private_message,
target_usernames: "#{allowed_user.username},#{not_allowed_user.username}"
)
expect(pc).not_to be_valid
expect(pc.errors.full_messages).to contain_exactly(
I18n.t(:not_accepting_pms, username: not_allowed_user.username)
)
end
end
end
context "private message recipients limit (max_allowed_message_recipients) reached" do
fab!(:target_user1) { Fabricate(:coding_horror) }
fab!(:target_user2) { Fabricate(:evil_trout) }

5
spec/fabricators/allowed_pm_users.rb Normal file
View File

@ -0,0 +1,5 @@
# frozen_string_literal: true
Fabricator(:allowed_pm_user) do
user
end

2
spec/serializers/web_hook_user_serializer_spec.rb
View File

@ -23,7 +23,7 @@ RSpec.describe WebHookUserSerializer do
it 'should only include the required keys' do
count = serializer.as_json.keys.count
difference = count - 46
difference = count - 47
expect(difference).to eq(0), lambda {
message = (difference < 0 ?