diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 76e6c879e11..e5aa3b033c9 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -477,9 +477,19 @@ class TopicsController < ApplicationController end def invite - username_or_email = params[:user] ? fetch_username : fetch_email + unless guardian.is_staff? + RateLimiter.new( + current_user, + "topic-invitations-per-day", + SiteSetting.max_topic_invitations_per_day, + 1.day.to_i + ).performed! + end topic = Topic.find_by(id: params[:topic_id]) + raise Discourse::InvalidParameters.new unless topic + + username_or_email = params[:user] ? fetch_username : fetch_email groups = Group.lookup_groups( group_ids: params[:group_ids], @@ -492,6 +502,7 @@ class TopicsController < ApplicationController begin if topic.invite(current_user, username_or_email, group_ids, params[:custom_message]) user = User.find_by_username_or_email(username_or_email) + if user render_json_dump BasicUserSerializer.new(user, scope: guardian, root: 'user') else diff --git a/app/models/topic.rb b/app/models/topic.rb index b5244d73756..d48825e0cd1 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -792,62 +792,42 @@ SQL true end - # Invite a user to the topic by username or email. Returns success/failure def invite(invited_by, username_or_email, group_ids = nil, custom_message = nil) user = User.find_by_username_or_email(username_or_email) - if private_message? - # If the user exists, add them to the message. - raise UserExists.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists? - - if user && topic_allowed_users.create!(user_id: user.id) - # Create a small action message - add_small_action(invited_by, "invited_user", user.username) - - # Notify the user they've been invited - user.notifications.create(notification_type: Notification.types[:invited_to_private_message], - topic_id: id, - post_number: 1, - data: { topic_title: title, - display_username: invited_by.username }.to_json) - return true - end + if user && topic_allowed_users.where(user_id: user.id).exists? + raise UserExists.new(I18n.t("topic_invite.user_exists")) end - if username_or_email =~ /^.+@.+$/ && Guardian.new(invited_by).can_invite_via_email?(self) - RateLimiter.new(invited_by, "topic-invitations-per-day", SiteSetting.max_topic_invitations_per_day, 1.day.to_i).performed! + if user && private_message? && topic_allowed_users.create!(user_id: user.id) + add_small_action(invited_by, "invited_user", user.username) - if user.present? - # add existing users + create_invite_notification!( + Notification.types[:invited_to_private_message], + invited_by.username + ) + + true + elsif username_or_email =~ /^.+@.+$/ && Guardian.new(invited_by).can_invite_via_email?(self) + if user Invite.extend_permissions(self, user, invited_by) - # Notify the user they've been invited - user.notifications.create(notification_type: Notification.types[:invited_to_topic], - topic_id: id, - post_number: 1, - data: { topic_title: title, - display_username: invited_by.username }.to_json) - return true + create_invite_notification!( + Notification.types[:invited_to_topic], + invited_by.username + ) else - # NOTE callers expect an invite object if an invite was sent via email invite_by_email(invited_by, username_or_email, group_ids, custom_message) end - else - raise UserExists.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists? - if user && topic_allowed_users.create!(user_id: user.id) - RateLimiter.new(invited_by, "topic-invitations-per-day", SiteSetting.max_topic_invitations_per_day, 1.day.to_i).performed! + true + elsif user && topic_allowed_users.create!(user_id: user.id) + create_invite_notification!( + Notification.types[:invited_to_topic], + invited_by.username + ) - # Notify the user they've been invited - user.notifications.create(notification_type: Notification.types[:invited_to_topic], - topic_id: id, - post_number: 1, - data: { topic_title: title, - display_username: invited_by.username }.to_json) - return true - else - false - end + true end end @@ -1310,6 +1290,17 @@ SQL RateLimiter.new(user, "#{key}-per-day", SiteSetting.send(method_name), 1.day.to_i) end + def create_invite_notification!(notification_type, username) + user.notifications.create!( + notification_type: notification_type, + topic_id: self.id, + post_number: 1, + data: { + topic_title: self.title, + display_username: username + }.to_json + ) + end end # == Schema Information diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 01637a4b09c..0b6722e63eb 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -1260,103 +1260,6 @@ describe TopicsController do end end - describe 'invite' do - - describe "group invites" do - it "works correctly" do - group = Fabricate(:group) - topic = Fabricate(:topic) - _admin = log_in(:admin) - - post :invite, params: { - topic_id: topic.id, email: 'hiro@from.heros', group_ids: "#{group.id}" - }, format: :json - - expect(response).to be_success - - invite = Invite.find_by(email: 'hiro@from.heros') - groups = invite.groups.to_a - expect(groups.count).to eq(1) - expect(groups[0].id).to eq(group.id) - end - end - - it "won't allow us to invite toa topic when we're not logged in" do - post :invite, params: { - topic_id: 1, email: 'jake@adventuretime.ooo' - }, format: :json - expect(response.status).to eq(403) - end - - describe 'when logged in as group manager' do - let(:group_manager) { log_in } - let(:group) { Fabricate(:group).tap { |g| g.add_owner(group_manager) } } - let(:private_category) { Fabricate(:private_category, group: group) } - let(:group_private_topic) { Fabricate(:topic, category: private_category, user: group_manager) } - let(:recipient) { 'jake@adventuretime.ooo' } - - it "should attach group to the invite" do - post :invite, params: { - topic_id: group_private_topic.id, user: recipient - }, format: :json - - expect(response).to be_success - expect(Invite.find_by(email: recipient).groups).to eq([group]) - end - end - - describe 'when logged in' do - before do - @topic = Fabricate(:topic, user: log_in) - end - - it 'requires an email parameter' do - expect do - post :invite, params: { topic_id: @topic.id }, format: :json - end.to raise_error(ActionController::ParameterMissing) - end - - describe 'without permission' do - it "raises an exception when the user doesn't have permission to invite to the topic" do - post :invite, params: { - topic_id: @topic.id, user: 'jake@adventuretime.ooo' - }, format: :json - - expect(response).to be_forbidden - end - end - - describe 'with admin permission' do - - let!(:admin) do - log_in :admin - end - - it 'should work as expected' do - post :invite, params: { - topic_id: @topic.id, user: 'jake@adventuretime.ooo' - }, format: :json - - expect(response).to be_success - expect(::JSON.parse(response.body)).to eq('success' => 'OK') - expect(Invite.where(invited_by_id: admin.id).count).to eq(1) - end - - it 'should fail on shoddy email' do - post :invite, params: { - topic_id: @topic.id, user: 'i_am_not_an_email' - }, format: :json - - expect(response).not_to be_success - expect(::JSON.parse(response.body)).to eq('failed' => 'FAILED') - end - - end - - end - - end - describe 'make_banner' do it 'needs you to be a staff member' do diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index a5f61c591d9..2e69779555a 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -493,8 +493,7 @@ describe Topic do expect(Guardian.new(coding_horror).can_see?(topic)).to eq(true) expect(TopicQuery.new(evil_trout).list_latest.topics).not_to include(topic) - # invites - expect(topic.invite(topic.user, 'duhhhhh')).to eq(false) + expect(topic.invite(topic.user, 'duhhhhh')).to eq(nil) end context 'invite' do @@ -567,7 +566,8 @@ describe Topic do it 'adds user correctly' do expect { expect(topic.invite(topic.user, walter.email)).to eq(true) - }.to change(Notification, :count) + }.to change(Notification, :count).by(1) + expect(topic.allowed_users.include?(walter)).to eq(true) end @@ -591,35 +591,6 @@ describe Topic do end - context 'rate limits' do - - it "rate limits topic invitations" do - SiteSetting.max_topic_invitations_per_day = 2 - RateLimiter.enable - RateLimiter.clear_all! - - start = Time.now.tomorrow.beginning_of_day - freeze_time(start) - - user = Fabricate(:user) - trust_level_2 = Fabricate(:user, trust_level: 2) - topic = Fabricate(:topic, user: trust_level_2) - - freeze_time(start + 10.minutes) - topic.invite(topic.user, user.username) - - freeze_time(start + 20.minutes) - topic.invite(topic.user, "walter@white.com") - - freeze_time(start + 30.minutes) - - expect { - topic.invite(topic.user, "user@example.com") - }.to raise_error(RateLimiter::LimitExceeded) - end - - end - context 'bumping topics' do before do @@ -1824,8 +1795,8 @@ describe Topic do let(:randolph) { 'randolph@duke.ooo' } it "should attach group to the invite" do - invite = group_private_topic.invite(group_manager, randolph) - expect(invite.groups).to eq([group]) + expect(group_private_topic.invite(group_manager, randolph)).to eq(true) + expect(Invite.last.groups).to eq([group]) end end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 72f00371cf7..a6bfdf56c92 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -156,6 +156,111 @@ RSpec.describe TopicsController do end end + describe '#invite' do + describe 'when not logged in' do + it "should return the right response" do + post "/t/#{topic.id}/invite.json", params: { + email: 'jake@adventuretime.ooo' + } + + expect(response.status).to eq(403) + end + end + + describe 'when logged in' do + before do + sign_in(user) + end + + describe 'as a valid user' do + let(:topic) { Fabricate(:topic, user: user) } + + it 'should return the right response' do + user.update!(trust_level: TrustLevel[2]) + + expect do + post "/t/#{topic.id}/invite.json", params: { + email: 'someguy@email.com' + } + end.to change { Invite.where(invited_by_id: user.id).count }.by(1) + + expect(response.status).to eq(200) + end + end + + describe 'when user is a group manager' do + let(:group) { Fabricate(:group).tap { |g| g.add_owner(user) } } + let(:private_category) { Fabricate(:private_category, group: group) } + + let(:group_private_topic) do + Fabricate(:topic, category: private_category, user: user) + end + + let(:recipient) { 'jake@adventuretime.ooo' } + + it "should attach group to the invite" do + + post "/t/#{group_private_topic.id}/invite.json", params: { + user: recipient + } + + expect(response.status).to eq(200) + expect(Invite.find_by(email: recipient).groups).to eq([group]) + end + end + + describe 'when topic id is invalid' do + it 'should return the right response' do + post "/t/999/invite.json", params: { + email: Fabricate(:user).email + } + + expect(response.status).to eq(400) + end + end + + it 'requires an email parameter' do + post "/t/#{topic.id}/invite.json" + + expect(response.status).to eq(400) + end + + describe 'when user does not have permission to invite to the topic' do + let(:topic) { Fabricate(:private_message_topic) } + + it "should return the right response" do + post "/t/#{topic.id}/invite.json", params: { + user: user.username + } + + expect(response.status).to eq(403) + end + end + end + + describe "when inviting a group to a topic" do + let(:group) { Fabricate(:group) } + + before do + sign_in(Fabricate(:admin)) + end + + it "should work correctly" do + email = 'hiro@from.heros' + + post "/t/#{topic.id}/invite.json", params: { + email: email, group_ids: group.id + } + + expect(response.status).to eq(200) + + groups = Invite.find_by(email: email).groups + expect(groups.count).to eq(1) + expect(groups.first.id).to eq(group.id) + end + end + end + describe 'invite_group' do let(:admins) { Group[:admins] } let(:pm) { Fabricate(:private_message_topic) }