From 8156475ea501221c3ba1bf3280e8368b354839bc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 7 Apr 2016 05:27:28 +0800
Subject: [PATCH] Report better errors. TODO: Enable skipped test
---
app/workers/email_receiver_worker.rb | 2 ++
lib/gitlab/email/receiver.rb | 11 +++++++++--
spec/lib/gitlab/email/receiver_spec.rb | 25 ++++++++++++++++++++-----
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb
index 64105dbc01d..544b23da85e 100644
--- a/app/workers/email_receiver_worker.rb
+++ b/app/workers/email_receiver_worker.rb
@@ -26,6 +26,8 @@ class EmailReceiverWorker
case e
when Gitlab::Email::Receiver::SentNotificationNotFoundError
reason = "We couldn't figure out what the email is in reply to. Please create your comment through the web interface."
+ when Gitlab::Email::Receiver::ProjectNotFound
+ reason = "We couldn't find the project. Please check if there's any typo."
when Gitlab::Email::Receiver::EmptyEmailError
can_retry = true
reason = "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies."
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index 471d10a11a6..50c16da4ef6 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -5,6 +5,7 @@ module Gitlab
class ProcessingError < StandardError; end
class EmailUnparsableError < ProcessingError; end
class SentNotificationNotFoundError < ProcessingError; end
+ class ProjectNotFound < ProcessingError; end
class EmptyEmailError < ProcessingError; end
class AutoGeneratedEmailError < ProcessingError; end
class UserNotFoundError < ProcessingError; end
@@ -25,10 +26,16 @@ module Gitlab
process_create_note
elsif message_project
- process_create_issue
+ if message_sender.can?(:read_project, message_project)
+ process_create_issue
+ else # Must be private project without access
+ raise ProjectNotFound
+ end
+ elsif reply_key =~ %r{/|\+}
+ # Sent Notification reply_key would not have / or +
+ raise ProjectNotFound
else
- # TODO: could also be project not found
raise SentNotificationNotFoundError
end
end
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index d1b52b9d086..c0fc18a9f83 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -210,10 +210,12 @@ describe Gitlab::Email::Receiver, lib: true do
end
context "something is wrong" do
+ before do
+ project
+ end
+
context "when the issue could not be saved" do
before do
- project
-
allow_any_instance_of(Issue).to receive(:persisted?).and_return(false)
end
@@ -225,11 +227,24 @@ describe Gitlab::Email::Receiver, lib: true do
context "when the authentication_token token didn't match" do
let!(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") }
- before do
- project
+ it "raises an UserNotAuthorizedError" do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError)
end
+ end
+
+ context "when project is private" do
+ let(:project) { create(:project, :private, namespace: namespace) }
+
+ it "raises a ProjectNotFound if the user is not a member" do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::ProjectNotFound)
+ end
+
+ it "raises a UserNotAuthorizedError if the user has no sufficient permission" do
+ skip("Find a role which can :read_project but can't :create_issue")
+
+ project.update(group: create(:group))
+ project.group.add_guest(user)
- it "raises an UserNotAuthorizedError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError)
end
end
--
GitLab