From 77a8cae0942e008ee7988d320b33614e717351c2 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 2 May 2017 15:13:33 +0530 Subject: [PATCH] FIX: rescue specific errors on invite failure --- app/controllers/invites_controller.rb | 2 +- app/controllers/topics_controller.rb | 2 +- app/models/invite.rb | 3 ++- app/models/topic.rb | 5 +++-- spec/models/invite_spec.rb | 2 +- spec/models/topic_spec.rb | 2 +- 6 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index dd151b0ba29..fc7230fec73 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -74,7 +74,7 @@ class InvitesController < ApplicationController else render json: failed_json, status: 422 end - rescue => e + rescue Invite::UserExists => e render json: {errors: [e.message]}, status: 422 end end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 33b88966a75..403dcb9c113 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -497,7 +497,7 @@ class TopicsController < ApplicationController else render json: failed_json, status: 422 end - rescue => e + rescue Topic::UserExists => e render json: {errors: [e.message]}, status: 422 end end diff --git a/app/models/invite.rb b/app/models/invite.rb index 5fd7471f007..3b76c456db3 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -1,6 +1,7 @@ require_dependency 'rate_limiter' class Invite < ActiveRecord::Base + class UserExists < StandardError; end include RateLimiter::OnCreateRecord include Trashable @@ -103,7 +104,7 @@ class Invite < ActiveRecord::Base if user extend_permissions(topic, user, invited_by) if topic - raise StandardError.new I18n.t("invite.user_exists", email: lower_email, username: user.username) + raise UserExists.new I18n.t("invite.user_exists", email: lower_email, username: user.username) end invite = Invite.with_deleted diff --git a/app/models/topic.rb b/app/models/topic.rb index 2dd0b9b8356..f6f26196118 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -10,6 +10,7 @@ require_dependency 'discourse_tagging' require_dependency 'search' class Topic < ActiveRecord::Base + class UserExists < StandardError; end include ActionView::Helpers::SanitizeHelper include RateLimiter::OnCreateRecord include HasCustomFields @@ -722,7 +723,7 @@ SQL if private_message? # If the user exists, add them to the message. user = User.find_by_username_or_email(username_or_email) - raise StandardError.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists? + 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 @@ -747,7 +748,7 @@ SQL else # invite existing member to a topic user = User.find_by_username(username_or_email) - raise StandardError.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists? + 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) # rate limit topic invite diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 7bba547df88..60c390a681c 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -162,7 +162,7 @@ describe Invite do it "works" do # doesn't create an invite - expect { topic.invite_by_email(topic.user, coding_horror.email) }.to raise_error(StandardError) + expect { topic.invite_by_email(topic.user, coding_horror.email) }.to raise_error(Invite::UserExists) # gives the user permission to access the topic expect(topic.allowed_users.include?(coding_horror)).to eq(true) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 9db33f78bd6..7d567ad3418 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1678,7 +1678,7 @@ describe Topic do it "should add user to the group" do expect(Guardian.new(walter).can_see?(group_private_topic)).to be_falsey - expect { group_private_topic.invite(group_manager, walter.email) }.to raise_error(StandardError) + expect { group_private_topic.invite(group_manager, walter.email) }.to raise_error(Invite::UserExists) expect(walter.groups).to include(group) expect(Guardian.new(walter).can_see?(group_private_topic)).to be_truthy end