diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 51aeff6700c..a2e2459760f 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -202,9 +202,15 @@ class TopicsController < ApplicationController username_or_email = params[:user] ? fetch_username : fetch_email topic = Topic.find_by(id: params[:topic_id]) - guardian.ensure_can_invite_to!(topic) - if topic.invite(current_user, username_or_email) + if group_ids = params[:group_ids] + group_ids = group_ids.split(",").map(&:to_i) + group_ids = Group.where(id: group_ids).pluck(:id) + end + + guardian.ensure_can_invite_to!(topic,group_ids) + + if topic.invite(current_user, username_or_email, group_ids) user = User.find_by_username_or_email(username_or_email) if user render_json_dump BasicUserSerializer.new(user, scope: guardian, root: 'user') diff --git a/app/models/invite.rb b/app/models/invite.rb index 2736dd4a387..c42405c5a00 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -54,31 +54,45 @@ class Invite < ActiveRecord::Base # Create an invite for a user, supplying an optional topic # # Return the previously existing invite if already exists. Returns nil if the invite can't be created. - def self.invite_by_email(email, invited_by, topic=nil) + def self.invite_by_email(email, invited_by, topic=nil, group_ids=nil) lower_email = Email.downcase(email) + user = User.find_by(email: lower_email) + + if user + topic.grant_permission_to_user(lower_email) if topic && topic.private_message? + return nil + end + invite = Invite.with_deleted - .where('invited_by_id = ? and email = ?', invited_by.id, lower_email) + .where(email: lower_email, invited_by_id: invited_by.id) .order('created_at DESC') .first - if invite && invite.expired? + if invite && (invite.expired? || invite.deleted_at) invite.destroy invite = nil end - if invite.blank? - invite = Invite.create(invited_by: invited_by, email: lower_email) - unless invite.valid? - topic.grant_permission_to_user(lower_email) if topic.present? && topic.email_already_exists_for?(invite) - return + if !invite + invite = Invite.create!(invited_by: invited_by, email: lower_email) + end + + if topic && !invite.topic_invites.pluck(:topic_id).include?(topic.id) + invite.topic_invites.create!(invite_id: invite.id, topic_id: topic.id) + # to correct association + topic.reload + end + + if group_ids.present? + group_ids = group_ids - invite.invited_groups.pluck(:group_id) + group_ids.each do |group_id| + invite.invited_groups.create!(group_id: group_id) end end - # Recover deleted invites if we invite them again - invite.recover! if invite.deleted_at.present? - - topic.topic_invites.create(invite_id: invite.id) if topic.present? Jobs.enqueue(:invite_email, invite_id: invite.id) + + invite.reload invite end diff --git a/app/models/topic.rb b/app/models/topic.rb index 99fd3e16eeb..fdb723be9bc 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -523,7 +523,7 @@ class Topic < ActiveRecord::Base end # Invite a user to the topic by username or email. Returns success/failure - def invite(invited_by, username_or_email) + def invite(invited_by, username_or_email, group_ids=nil) if private_message? # If the user exists, add them to the topic. user = User.find_by_username_or_email(username_or_email) @@ -541,14 +541,14 @@ class Topic < ActiveRecord::Base if username_or_email =~ /^.+@.+$/ # NOTE callers expect an invite object if an invite was sent via email - invite_by_email(invited_by, username_or_email) + invite_by_email(invited_by, username_or_email, group_ids) else false end end - def invite_by_email(invited_by, email) - Invite.invite_by_email(email, invited_by, self) + def invite_by_email(invited_by, email, group_ids=nil) + Invite.invite_by_email(email, invited_by, self, group_ids) end def email_already_exists_for?(invite) diff --git a/lib/guardian.rb b/lib/guardian.rb index 9ee8cb40952..a12d7d56a08 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -196,8 +196,10 @@ class Guardian ) end - def can_invite_to?(object) - can_see?(object) && can_invite_to_forum? + def can_invite_to?(object, group_ids=nil) + can_see?(object) && + can_invite_to_forum? && + ( group_ids.blank? || is_admin? ) end def can_see_private_messages?(user_id) diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 2f1eae5b6ba..235b255dafa 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -748,6 +748,24 @@ describe TopicsController do end describe 'invite' do + + describe "group invites" do + it "works correctly" do + group = Fabricate(:group) + topic = Fabricate(:topic) + admin = log_in(:admin) + + xhr :post, :invite, topic_id: topic.id, email: 'hiro@from.heros', group_ids: "#{group.id}" + + response.should be_success + + invite = Invite.find_by(email: 'hiro@from.heros') + groups = invite.groups.to_a + groups.count.should == 1 + groups[0].id.should == group.id + end + end + it "won't allow us to invite toa topic when we're not logged in" do lambda { xhr :post, :invite, topic_id: 1, email: 'jake@adventuretime.ooo' }.should raise_error(Discourse::NotLoggedIn) end @@ -763,7 +781,6 @@ describe TopicsController do describe 'without permission' do it "raises an exception when the user doesn't have permission to invite to the topic" do - Guardian.any_instance.expects(:can_invite_to?).with(@topic).returns(false) xhr :post, :invite, topic_id: @topic.id, user: 'jake@adventuretime.ooo' response.should be_forbidden end @@ -771,46 +788,25 @@ describe TopicsController do describe 'with permission' do - before do - Guardian.any_instance.expects(:can_invite_to?).with(@topic).returns(true) + let!(:admin) do + log_in :admin end - context 'when it returns an invite' do - before do - Topic.any_instance.expects(:invite_by_email).with(@topic.user, 'jake@adventuretime.ooo').returns(Invite.new) - xhr :post, :invite, topic_id: @topic.id, user: 'jake@adventuretime.ooo' - end - - it 'should succeed' do - response.should be_success - end - - it 'returns success JSON' do - ::JSON.parse(response.body).should == {'success' => 'OK'} - end + it 'should work as expected' do + xhr :post, :invite, topic_id: @topic.id, user: 'jake@adventuretime.ooo' + response.should be_success + ::JSON.parse(response.body).should == {'success' => 'OK'} + Invite.where(invited_by_id: admin.id).count.should == 1 end - context 'when it fails and returns nil' do - - before do - Topic.any_instance.expects(:invite_by_email).with(@topic.user, 'jake@adventuretime.ooo').returns(nil) - xhr :post, :invite, topic_id: @topic.id, user: 'jake@adventuretime.ooo' - end - - it 'should succeed' do - response.should_not be_success - end - - it 'returns success JSON' do - ::JSON.parse(response.body).should == {'failed' => 'FAILED'} - end - + it 'should fail on shoddy email' do + xhr :post, :invite, topic_id: @topic.id, user: 'i_am_not_an_email' + response.should_not be_success + ::JSON.parse(response.body).should == {'failed' => 'FAILED'} end end - - end end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 9d52af523f9..600a24032e5 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -209,18 +209,19 @@ describe Invite do context 'invited to topics' do let!(:topic) { Fabricate(:private_message_topic) } - let!(:invite) { topic.invite(topic.user, 'jake@adventuretime.ooo')} + let!(:invite) { + topic.invite(topic.user, 'jake@adventuretime.ooo') + } context 'redeem topic invite' do - let!(:user) { invite.redeem } it 'adds the user to the topic_users' do + user = invite.redeem + topic.reload topic.allowed_users.include?(user).should be_true - end - - it 'can see the private topic' do Guardian.new(user).can_see?(topic).should be_true end + end context 'invited by another user to the same topic' do @@ -229,16 +230,18 @@ describe Invite do let!(:user) { invite.redeem } it 'adds the user to the topic_users' do + topic.reload topic.allowed_users.include?(user).should be_true end end context 'invited by another user to a different topic' do - let(:coding_horror) { User.find_by(username: "CodingHorror") } - let(:another_topic) { Fabricate(:topic, archetype: "private_message", user: coding_horror) } let!(:another_invite) { another_topic.invite(coding_horror, 'jake@adventuretime.ooo') } let!(:user) { invite.redeem } + let(:coding_horror) { User.find_by(username: "CodingHorror") } + let(:another_topic) { Fabricate(:topic, archetype: "private_message", user: coding_horror) } + it 'adds the user to the topic_users of the first topic' do topic.allowed_users.include?(user).should be_true another_topic.allowed_users.include?(user).should be_true @@ -248,12 +251,9 @@ describe Invite do context 'if they redeem the other invite afterwards' do - before do - @result = another_invite.redeem - end - it 'returns the same user' do - @result.should == user + result = another_invite.redeem + result.should == user another_invite.reload another_invite.should be_redeemed end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 38aa5390570..5cec3aedbb5 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -299,11 +299,6 @@ describe Topic do context 'invite' do - it "delegates to topic.invite_by_email when the user doesn't exist, but it's an email" do - topic.expects(:invite_by_email).with(topic.user, 'jake@adventuretime.ooo') - topic.invite(topic.user, 'jake@adventuretime.ooo') - end - context 'existing user' do let(:walter) { Fabricate(:walter_white) } @@ -324,19 +319,14 @@ describe Topic do end context 'by email' do - it 'returns true' do - topic.invite(topic.user, walter.email).should be_true - end - it 'adds walter to the allowed users' do - topic.invite(topic.user, walter.email) + it 'adds user correctly' do + lambda { + topic.invite(topic.user, walter.email).should be_true + }.should change(Notification, :count) topic.allowed_users.include?(walter).should be_true end - it 'creates a notification' do - lambda { topic.invite(topic.user, walter.email) }.should change(Notification, :count) - end - end end