diff --git a/app/assets/javascripts/discourse/app/components/invite-panel.js b/app/assets/javascripts/discourse/app/components/invite-panel.js index dd55ebeed29..17ea67436e0 100644 --- a/app/assets/javascripts/discourse/app/components/invite-panel.js +++ b/app/assets/javascripts/discourse/app/components/invite-panel.js @@ -351,12 +351,11 @@ export default Component.extend({ if (this.isInviteeGroup) { return this.inviteModel .createGroupInvite(this.invitee.trim()) - .then((data) => { + .then(() => { model.setProperties({ saving: false, finished: true }); - this.get("inviteModel.details.allowed_groups").pushObject( - EmberObject.create(data.group) - ); - this.appEvents.trigger("post-stream:refresh"); + this.inviteModel.reload().then(() => { + this.appEvents.trigger("post-stream:refresh"); + }); }) .catch(onerror); } else { diff --git a/app/models/topic.rb b/app/models/topic.rb index 5f807a7dfef..81d0d56ad82 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -971,16 +971,38 @@ class Topic < ActiveRecord::Base end def invite_group(user, group) - TopicAllowedGroup.create!(topic_id: id, group_id: group.id) - allowed_groups.reload + TopicAllowedGroup.create!(topic_id: self.id, group_id: group.id) + self.allowed_groups.reload - last_post = posts.order('post_number desc').where('not hidden AND posts.deleted_at IS NULL').first + last_post = self.posts.order('post_number desc').where('not hidden AND posts.deleted_at IS NULL').first if last_post Jobs.enqueue(:post_alert, post_id: last_post.id) add_small_action(user, "invited_group", group.name) Jobs.enqueue(:group_pm_alert, user_id: user.id, group_id: group.id, post_id: last_post.id) end + # If the group invited includes the OP of the topic as one of is members, + # we cannot strip the topic_allowed_user record since it will be more + # complicated to recover the topic_allowed_user record for the OP if the + # group is removed. + allowed_user_where_clause = <<~SQL + users.id IN ( + SELECT topic_allowed_users.user_id + FROM topic_allowed_users + INNER JOIN group_users ON group_users.user_id = topic_allowed_users.user_id + INNER JOIN topic_allowed_groups ON topic_allowed_groups.group_id = group_users.group_id + WHERE topic_allowed_groups.group_id = :group_id AND + topic_allowed_users.topic_id = :topic_id AND + topic_allowed_users.user_id != :op_user_id + ) + SQL + User.where([ + allowed_user_where_clause, + { group_id: group.id, topic_id: self.id, op_user_id: self.user_id } + ]).find_each do |allowed_user| + remove_allowed_user(Discourse.system_user, allowed_user) + end + true end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 64d599f0196..40d96125789 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1021,6 +1021,51 @@ describe Topic do .to eq(Notification.types[:group_message_summary]) end + + it "removes users in topic_allowed_users who are part of the added group" do + admins = Group[:admins] + admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) + + # clear up the state so we can be more explicit with the test + TopicAllowedUser.where(topic: topic).delete_all + user0 = topic.user + user1 = Fabricate(:user) + user2 = Fabricate(:user) + user3 = Fabricate(:user) + Fabricate(:topic_allowed_user, topic: topic, user: user0) + Fabricate(:topic_allowed_user, topic: topic, user: user1) + Fabricate(:topic_allowed_user, topic: topic, user: user2) + Fabricate(:topic_allowed_user, topic: topic, user: user3) + + admins.add(user1) + admins.add(user2) + + other_topic = Fabricate(:topic) + Fabricate(:topic_allowed_user, user: user1, topic: other_topic) + + expect(topic.invite_group(topic.user, admins)).to eq(true) + expect(topic.posts.last.action_code).to eq("removed_user") + expect(topic.allowed_users).to match_array([user0, user3, Discourse.system_user]) + expect(other_topic.allowed_users).to match_array([user1]) + end + + it "does not remove the OP from topic_allowed_users if they are part of an added group" do + admins = Group[:admins] + admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) + + # clear up the state so we can be more explicit with the test + TopicAllowedUser.where(topic: topic).delete_all + user0 = topic.user + user1 = Fabricate(:user) + Fabricate(:topic_allowed_user, topic: topic, user: user0) + Fabricate(:topic_allowed_user, topic: topic, user: user1) + + admins.add(topic.user) + admins.add(user1) + + expect(topic.invite_group(topic.user, admins)).to eq(true) + expect(topic.allowed_users).to match_array([topic.user, Discourse.system_user]) + end end end end