diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index b6ea3f4d019..fa241cb5435 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -467,7 +467,7 @@ class TopicsController < ApplicationController topic = Topic.find_by(id: params[:topic_id]) if topic.private_message? - guardian.ensure_can_send_private_message!(group) + guardian.ensure_can_invite_group_to_private_message!(group, topic) topic.invite_group(current_user, group) render_json_dump BasicGroupSerializer.new(group, scope: guardian, root: 'group') else diff --git a/app/models/group.rb b/app/models/group.rb index 731500a41b6..0fadbf9c7ca 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -248,7 +248,12 @@ class Group < ActiveRecord::Base unless group = self.lookup_group(name) group = Group.new(name: name.to_s, automatic: true) - group.default_notification_level = 2 if AUTO_GROUPS[:moderators] == id + + if AUTO_GROUPS[:moderators] == id + group.default_notification_level = 2 + group.messageable_level = ALIAS_LEVELS[:everyone] + end + group.id = id group.save! end diff --git a/lib/guardian.rb b/lib/guardian.rb index e36316a851d..9792f468f3c 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -285,6 +285,11 @@ class Guardian is_admin? || (authenticated? && @user.id == user_id) end + def can_invite_group_to_private_message?(group, topic) + can_see_topic?(topic) && + can_send_private_message?(group) + end + def can_send_private_message?(target) is_user = target.is_a?(User) is_group = target.is_a?(Group) @@ -300,6 +305,8 @@ class Guardian (is_staff? || SiteSetting.enable_private_messages) && # Can't send PMs to suspended users (is_staff? || is_group || !target.suspended?) && + # Check group messageable level + (is_staff? || is_user || Group.messageable(@user).where(id: target.id).exists?) && # Silenced users can only send PM to staff (!is_silenced? || target.staff?) end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 16bd0378f5c..35ba2310827 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -211,12 +211,30 @@ describe Guardian do it "returns true if target is a staff group" do Group::STAFF_GROUPS.each do |name| g = Group[name] - g.messageable_level = Group::ALIAS_LEVELS[:everyone] + g.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) expect(Guardian.new(user).can_send_private_message?(g)).to be_truthy end end end + it "respects the group's messageable_level" do + group = Fabricate(:group) + + Group::ALIAS_LEVELS.each do |level, _| + group.update!(messageable_level: Group::ALIAS_LEVELS[level]) + output = level == :everyone ? true : false + + expect(Guardian.new(user).can_send_private_message?(group)).to eq(output) + end + + admin = Fabricate(:admin) + + Group::ALIAS_LEVELS.each do |level, _| + group.update!(messageable_level: Group::ALIAS_LEVELS[level]) + expect(Guardian.new(admin).can_send_private_message?(group)).to eq(true) + end + end + context 'target user has private message disabled' do before do another_user.user_option.update!(allow_private_messages: false) diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 20e8503133c..57ab779b7ab 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -765,7 +765,7 @@ describe PostCreator do let(:target_user1) { Fabricate(:coding_horror) } let(:target_user2) { Fabricate(:moderator) } let(:group) do - g = Fabricate.build(:group) + g = Fabricate.build(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) g.add(target_user1) g.add(target_user2) g.save @@ -773,10 +773,12 @@ describe PostCreator do end let(:unrelated) { Fabricate(:user) } let(:post) do - PostCreator.create(user, title: 'hi there welcome to my topic', - raw: "this is my awesome message @#{unrelated.username_lower}", - archetype: Archetype.private_message, - target_group_names: group.name) + PostCreator.create!(user, + title: 'hi there welcome to my topic', + raw: "this is my awesome message @#{unrelated.username_lower}", + archetype: Archetype.private_message, + target_group_names: group.name + ) end it 'can post to a group correctly' do diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index bdf13e01d91..155fee81d9e 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -158,4 +158,59 @@ RSpec.describe TopicsController do end end end + + describe 'invite_group' do + let(:admins) { Group[:admins] } + let(:pm) { Fabricate(:private_message_topic) } + + def invite_group(topic, expected_status) + post "/t/#{topic.id}/invite-group.json", params: { group: admins.name } + expect(response.status).to eq(expected_status) + end + + before do + admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) + end + + describe 'as an anon user' do + it 'should be forbidden' do + invite_group(pm, 403) + end + end + + describe 'as a normal user' do + let!(:user) { sign_in(Fabricate(:user)) } + + describe 'when user does not have permission to view the topic' do + it 'should be forbidden' do + invite_group(pm, 403) + end + end + + describe 'when user has permission to view the topic' do + before do + pm.allowed_users << user + end + + it 'should allow user to invite group to topic' do + invite_group(pm, 200) + expect(pm.allowed_groups.first.id).to eq(admins.id) + end + end + end + + describe 'as an admin user' do + let!(:admin) { sign_in(Fabricate(:admin)) } + + it "disallows inviting a group to a topic" do + topic = Fabricate(:topic) + invite_group(topic, 422) + end + + it "allows inviting a group to a PM" do + invite_group(pm, 200) + expect(pm.allowed_groups.first.id).to eq(admins.id) + end + end + end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 5040be8e6c8..4e2813e35e7 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -278,7 +278,7 @@ RSpec.describe UsersController do } expect(response).to be_success - expect(JSON.parse(response.body)["groups"].first['name']).to eq(messageable_group.name) + expect(JSON.parse(response.body)["groups"].last['name']).to eq(messageable_group.name) end it 'searches for mentionable groups' do diff --git a/spec/support/integration_helpers.rb b/spec/support/integration_helpers.rb index aebb6c6b1ff..ceea9935559 100644 --- a/spec/support/integration_helpers.rb +++ b/spec/support/integration_helpers.rb @@ -29,5 +29,6 @@ module IntegrationHelpers Fabricate(:email_token, confirmed: true, user: user) post "/session.json", params: { login: user.username, password: password } expect(response).to be_success + user end end