Split alias levels in mentionable and messageable levels. (#5065)

* Split alias levels in mentionable and messageable levels.

* Fixed some tests.

* Set messageable level to everyone by default.

* By defaults, groups are not mentionable or messageable.

* Made staff groups messageable by the system.
This commit is contained in:
Bianca Nenciu 2017-08-28 17:32:08 +01:00 committed by Sam
parent ce2250d7aa
commit 6bc74ceb50
20 changed files with 81 additions and 32 deletions

View File

@ -111,8 +111,13 @@
{{/unless}} {{/unless}}
<div> <div>
<label for="alias">{{i18n 'groups.alias_levels.title'}}</label> <label for="alias">{{i18n 'groups.alias_levels.mentionable'}}</label>
{{combo-box name="alias" valueAttribute="value" value=model.alias_level content=aliasLevelOptions}} {{combo-box name="alias" valueAttribute="value" value=model.mentionable_level content=aliasLevelOptions}}
</div>
<div>
<label for="alias">{{i18n 'groups.alias_levels.messageable'}}</label>
{{combo-box name="alias" valueAttribute="value" value=model.messageable_level content=aliasLevelOptions}}
</div> </div>
<div> <div>

View File

@ -108,9 +108,9 @@ const Group = RestModel.extend({
return this.get('flair_color') ? this.get('flair_color').replace(new RegExp("[^0-9a-fA-F]", "g"), "") : null; return this.get('flair_color') ? this.get('flair_color').replace(new RegExp("[^0-9a-fA-F]", "g"), "") : null;
}, },
@computed('alias_level') @computed('mentionable_level')
canEveryoneMention(aliasLevel) { canEveryoneMention(mentionableLevel) {
return aliasLevel === '99'; return mentionableLevel === '99';
}, },
@observes("visibility_level", "canEveryoneMention") @observes("visibility_level", "canEveryoneMention")
@ -131,7 +131,8 @@ const Group = RestModel.extend({
asJSON() { asJSON() {
const attrs = { const attrs = {
name: this.get('name'), name: this.get('name'),
alias_level: this.get('alias_level'), mentionable_level: this.get('mentionable_level'),
messageable_level: this.get('messageable_level'),
visibility_level: this.get('visibility_level'), visibility_level: this.get('visibility_level'),
automatic_membership_email_domains: this.get('emailDomains'), automatic_membership_email_domains: this.get('emailDomains'),
automatic_membership_retroactive: !!this.get('automatic_membership_retroactive'), automatic_membership_retroactive: !!this.get('automatic_membership_retroactive'),

View File

@ -21,8 +21,8 @@ export default Discourse.Route.extend({
}); });
} else if (params.groupname) { } else if (params.groupname) {
// send a message to a group // send a message to a group
Group.mentionable(params.groupname).then(result => { Group.messageable(params.groupname).then(result => {
if (result.mentionable) { if (result.messageable) {
Ember.run.next(() => e.send("createNewMessageViaParams", params.groupname, params.title, params.body)); Ember.run.next(() => e.send("createNewMessageViaParams", params.groupname, params.title, params.body));
} else { } else {
bootbox.alert(I18n.t("composer.cant_send_pm", { username: params.groupname })); bootbox.alert(I18n.t("composer.cant_send_pm", { username: params.groupname }));

View File

@ -60,7 +60,8 @@ class Admin::GroupsController < Admin::AdminController
def save_group(group) def save_group(group)
group.name = group_params[:name] if group_params[:name].present? && !group.automatic group.name = group_params[:name] if group_params[:name].present? && !group.automatic
group.alias_level = group_params[:alias_level].to_i if group_params[:alias_level].present? group.mentionable_level = group_params[:mentionable_level].to_i if group_params[:mentionable_level].present?
group.messageable_level = group_params[:messageable_level].to_i if group_params[:messageable_level].present?
if group_params[:visibility_level] if group_params[:visibility_level]
group.visibility_level = group_params[:visibility_level] group.visibility_level = group_params[:visibility_level]
@ -192,7 +193,8 @@ class Admin::GroupsController < Admin::AdminController
def group_params def group_params
params.require(:group).permit( params.require(:group).permit(
:name, :name,
:alias_level, :mentionable_level,
:messageable_level,
:visibility_level, :visibility_level,
:automatic_membership_email_domains, :automatic_membership_email_domains,
:automatic_membership_retroactive, :automatic_membership_retroactive,

View File

@ -3,6 +3,7 @@ class GroupsController < ApplicationController
before_filter :ensure_logged_in, only: [ before_filter :ensure_logged_in, only: [
:set_notifications, :set_notifications,
:mentionable, :mentionable,
:messageable,
:update, :update,
:messages, :messages,
:histories, :histories,
@ -203,6 +204,16 @@ class GroupsController < ApplicationController
end end
end end
def messageable
group = find_group(:name)
if group
render json: { messageable: Group.messageable(current_user).where(id: group.id).present? }
else
raise Discourse::InvalidAccess.new
end
end
def remove_member def remove_member
group = Group.find(params[:id]) group = Group.find(params[:id])
group.public_exit ? ensure_logged_in : guardian.ensure_can_edit!(group) group.public_exit ? ensure_logged_in : guardian.ensure_can_edit!(group)

View File

@ -615,7 +615,7 @@ class PostsController < ApplicationController
if usernames = result[:target_usernames] if usernames = result[:target_usernames]
usernames = usernames.split(",") usernames = usernames.split(",")
groups = Group.mentionable(current_user).where('name in (?)', usernames).pluck('name') groups = Group.messageable(current_user).where('name in (?)', usernames).pluck('name')
usernames -= groups usernames -= groups
emails = usernames.select { |user| user.match(/@/) } emails = usernames.select { |user| user.match(/@/) }
usernames -= emails usernames -= emails

View File

@ -78,7 +78,8 @@ class Group < ActiveRecord::Base
) )
end end
validates :alias_level, inclusion: { in: ALIAS_LEVELS.values } validates :mentionable_level, inclusion: { in: ALIAS_LEVELS.values }
validates :messageable_level, inclusion: { in: ALIAS_LEVELS.values }
scope :visible_groups, ->(user) { scope :visible_groups, ->(user) {
groups = Group.order(name: :asc).where("groups.id > 0") groups = Group.order(name: :asc).where("groups.id > 0")
@ -126,6 +127,23 @@ class Group < ActiveRecord::Base
scope :mentionable, lambda { |user| scope :mentionable, lambda { |user|
where("mentionable_level in (:levels) OR
(
mentionable_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (
SELECT group_id FROM group_users WHERE user_id = :user_id)
)", levels: alias_levels(user), user_id: user && user.id)
}
scope :messageable, lambda { |user|
where("messageable_level in (:levels) OR
(
messageable_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (
SELECT group_id FROM group_users WHERE user_id = :user_id)
)", levels: alias_levels(user), user_id: user && user.id)
}
def self.alias_levels(user)
levels = [ALIAS_LEVELS[:everyone]] levels = [ALIAS_LEVELS[:everyone]]
if user && user.admin? if user && user.admin?
@ -139,12 +157,8 @@ class Group < ActiveRecord::Base
ALIAS_LEVELS[:members_mods_and_admins]] ALIAS_LEVELS[:members_mods_and_admins]]
end end
where("alias_level in (:levels) OR levels
( end
alias_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (
SELECT group_id FROM group_users WHERE user_id = :user_id)
)", levels: levels, user_id: user && user.id)
}
def downcase_incoming_email def downcase_incoming_email
self.incoming_email = (incoming_email || "").strip.downcase.presence self.incoming_email = (incoming_email || "").strip.downcase.presence
@ -615,7 +629,8 @@ end
# updated_at :datetime not null # updated_at :datetime not null
# automatic :boolean default(FALSE), not null # automatic :boolean default(FALSE), not null
# user_count :integer default(0), not null # user_count :integer default(0), not null
# alias_level :integer default(0) # mentionable_level :integer default(0)
# messageable_level :integer default(0)
# automatic_membership_email_domains :text # automatic_membership_email_domains :text
# automatic_membership_retroactive :boolean default(FALSE) # automatic_membership_retroactive :boolean default(FALSE)
# primary_group :boolean default(FALSE), not null # primary_group :boolean default(FALSE), not null

View File

@ -4,7 +4,8 @@ class BasicGroupSerializer < ApplicationSerializer
:name, :name,
:display_name, :display_name,
:user_count, :user_count,
:alias_level, :mentionable_level,
:messageable_level,
:visibility_level, :visibility_level,
:automatic_membership_email_domains, :automatic_membership_email_domains,
:automatic_membership_retroactive, :automatic_membership_retroactive,

View File

@ -25,6 +25,10 @@ class GroupShowSerializer < BasicGroupSerializer
Group.mentionable(scope.user).exists?(id: object.id) Group.mentionable(scope.user).exists?(id: object.id)
end end
def messageable
Group.messageable(scope.user).exists?(id: object.id)
end
private private
def authenticated? def authenticated?

View File

@ -459,7 +459,8 @@ en:
staff: "Group owners and staff" staff: "Group owners and staff"
owners: "Group owners and admins" owners: "Group owners and admins"
alias_levels: alias_levels:
title: "Who can message and @mention this group?" mentionable: "Who can @mention this group?"
messageable: "Who can message this group?"
nobody: "Nobody" nobody: "Nobody"
only_admins: "Only admins" only_admins: "Only admins"
mods_and_admins: "Only moderators and Admins" mods_and_admins: "Only moderators and Admins"

View File

@ -443,6 +443,7 @@ Discourse::Application.routes.draw do
get 'messages' get 'messages'
get 'counts' get 'counts'
get 'mentionable' get 'mentionable'
get 'messageable'
get 'logs' => 'groups#histories' get 'logs' => 'groups#histories'
collection do collection do

View File

@ -0,0 +1,8 @@
class SplitAliasLevels < ActiveRecord::Migration
def change
rename_column :groups, :alias_level, :mentionable_level
add_column :groups, :messageable_level, :integer, default: 0
Group.update_all('messageable_level=mentionable_level')
end
end

View File

@ -48,7 +48,7 @@ module ImportExport
self self
end end
GROUP_ATTRS = [ :id, :name, :created_at, :alias_level, :visible, GROUP_ATTRS = [ :id, :name, :created_at, :mentionable_level, :messageable_level, :visible,
:automatic_membership_email_domains, :automatic_membership_retroactive, :automatic_membership_email_domains, :automatic_membership_retroactive,
:primary_group, :title, :grant_trust_level, :incoming_email] :primary_group, :title, :grant_trust_level, :incoming_email]

View File

@ -207,7 +207,7 @@ describe Guardian do
it "returns true if target is a staff group" do it "returns true if target is a staff group" do
Group::STAFF_GROUPS.each do |name| Group::STAFF_GROUPS.each do |name|
g = Group[name] g = Group[name]
g.alias_level = Group::ALIAS_LEVELS[:everyone] g.messageable_level = Group::ALIAS_LEVELS[:everyone]
expect(Guardian.new(user).can_send_private_message?(g)).to be_truthy expect(Guardian.new(user).can_send_private_message?(g)).to be_truthy
end end
end end

View File

@ -755,7 +755,7 @@ describe PostsController do
it "can send a message to a group" do it "can send a message to a group" do
group = Group.create(name: 'test_group', alias_level: Group::ALIAS_LEVELS[:nobody]) group = Group.create(name: 'test_group', messageable_level: Group::ALIAS_LEVELS[:nobody])
user1 = Fabricate(:user) user1 = Fabricate(:user)
group.add(user1) group.add(user1)
@ -767,7 +767,7 @@ describe PostsController do
expect(response).not_to be_success expect(response).not_to be_success
# allow pm to this group # allow pm to this group
group.update_columns(alias_level: Group::ALIAS_LEVELS[:everyone]) group.update_columns(messageable_level: Group::ALIAS_LEVELS[:everyone])
xhr :post, :create, raw: 'I can haz a test', xhr :post, :create, raw: 'I can haz a test',
title: 'I loves my test', title: 'I loves my test',

View File

@ -1026,7 +1026,7 @@ describe TopicsController do
end end
before do before do
admins.alias_level = Group::ALIAS_LEVELS[:everyone] admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
admins.save! admins.save!
end end

View File

@ -455,7 +455,7 @@ describe Topic do
it 'can add admin to allowed groups' do it 'can add admin to allowed groups' do
admins = Group[:admins] admins = Group[:admins]
admins.update!(alias_level: Group::ALIAS_LEVELS[:everyone]) admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone])
expect(topic.invite_group(topic.user, admins)).to eq(true) expect(topic.invite_group(topic.user, admins)).to eq(true)
expect(topic.allowed_groups.include?(admins)).to eq(true) expect(topic.allowed_groups.include?(admins)).to eq(true)

View File

@ -69,7 +69,7 @@ describe GroupsController do
response_body = JSON.parse(response.body) response_body = JSON.parse(response.body)
expect(response_body["mentionable"]).to eq(false) expect(response_body["mentionable"]).to eq(false)
group.update_attributes!(alias_level: Group::ALIAS_LEVELS[:everyone]) group.update_attributes!(mentionable_level: Group::ALIAS_LEVELS[:everyone])
get "/groups/test/mentionable.json", name: group.name get "/groups/test/mentionable.json", name: group.name
expect(response).to be_success expect(response).to be_success

View File

@ -30,7 +30,7 @@ describe GroupShowSerializer do
end end
describe '#mentionable' do describe '#mentionable' do
let(:group) { Fabricate(:group, alias_level: Group::ALIAS_LEVELS[:everyone]) } let(:group) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
it 'should return the right value' do it 'should return the right value' do
json = GroupShowSerializer.new(group, scope: Guardian.new).as_json json = GroupShowSerializer.new(group, scope: Guardian.new).as_json

View File

@ -270,7 +270,7 @@ describe PostAlerter do
context '@group mentions' do context '@group mentions' do
let(:group) { Fabricate(:group, name: 'group', alias_level: Group::ALIAS_LEVELS[:everyone]) } let(:group) { Fabricate(:group, name: 'group', mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
let(:post) { create_post_with_alerts(raw: "Hello @group how are you?") } let(:post) { create_post_with_alerts(raw: "Hello @group how are you?") }
before { group.add(evil_trout) } before { group.add(evil_trout) }
@ -281,7 +281,7 @@ describe PostAlerter do
expect(GroupMention.count).to eq(1) expect(GroupMention.count).to eq(1)
Fabricate(:group, name: 'group-alt', alias_level: Group::ALIAS_LEVELS[:everyone]) Fabricate(:group, name: 'group-alt', mentionable_level: Group::ALIAS_LEVELS[:everyone])
expect { expect {
create_post_with_alerts(raw: "Hello, @group-alt should not trigger a notification?") create_post_with_alerts(raw: "Hello, @group-alt should not trigger a notification?")
@ -289,7 +289,7 @@ describe PostAlerter do
expect(GroupMention.count).to eq(2) expect(GroupMention.count).to eq(2)
group.update_columns(alias_level: Group::ALIAS_LEVELS[:members_mods_and_admins]) group.update_columns(mentionable_level: Group::ALIAS_LEVELS[:members_mods_and_admins])
expect { expect {
create_post_with_alerts(raw: "Hello @group you are not mentionable") create_post_with_alerts(raw: "Hello @group you are not mentionable")
}.to change(evil_trout.notifications, :count).by(0) }.to change(evil_trout.notifications, :count).by(0)