diff options
author | Sean McGivern <sean@gitlab.com> | 2019-01-04 11:58:55 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-01-04 11:58:55 +0300 |
commit | 8600043bc71120399c7e97ff74cadc8d0ca1d4b1 (patch) | |
tree | e0d6b3232cab1c84c0316cdbf158558a802f1e18 /spec/lib | |
parent | 626f3d03676cb7d714d9c5e9d78c76bdfb2ddda0 (diff) | |
parent | 4a0801b9c04d671b09eaaa2fd5edadfe6ee56122 (diff) |
Merge branch '29951-issue-creation-by-email-without-subaddressing' into 'master'
Support new issue creation by email without subaddressing
Closes #29951
See merge request gitlab-org/gitlab-ce!23523
Diffstat (limited to 'spec/lib')
5 files changed, 148 insertions, 35 deletions
diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 1d75e8cb5da..48139c2f9dc 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do stub_config_setting(host: 'localhost') end - let(:email_raw) { fixture_file('emails/valid_new_issue.eml') } + let(:email_raw) { email_fixture('emails/valid_new_issue.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } let!(:project) { create(:project, :public, namespace: namespace, path: 'gitlabhq') } @@ -23,21 +23,58 @@ describe Gitlab::Email::Handler::CreateIssueHandler do ) end + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "gitlabhq-gitlabhq-#{project.project_id}-#{user.incoming_email_token}-issue") + + expect(handler.instance_variable_get(:@project_id)).to eq project.project_id + expect(handler.instance_variable_get(:@project_slug)).to eq project.full_path_slug + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "h5bp/html5-boilerplate+#{user.incoming_email_token}") + + expect(handler.instance_variable_get(:@project_path)).to eq 'h5bp/html5-boilerplate' + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "h5bp-html5-boilerplate+something+invalid") + + expect(handler.can_handle?).to be_falsey + end + end + context "when everything is fine" do - it "creates a new issue" do - setup_attachment + shared_examples "a new issue" do + it "creates a new issue" do + setup_attachment - expect { receiver.execute }.to change { project.issues.count }.by(1) - issue = project.issues.last + expect { receiver.execute }.to change { project.issues.count }.by(1) + issue = project.issues.last + + expect(issue.author).to eq(user) + expect(issue.title).to eq('New Issue by email') + expect(issue.description).to include('reply by email') + expect(issue.description).to include(markdown) + end + end + + it_behaves_like "a new issue" - expect(issue.author).to eq(user) - expect(issue.title).to eq('New Issue by email') - expect(issue.description).to include('reply by email') - expect(issue.description).to include(markdown) + context "creates a new issue with legacy email address" do + let(:email_raw) { fixture_file('emails/valid_new_issue_legacy.eml') } + + it_behaves_like "a new issue" end context "when the reply is blank" do - let(:email_raw) { fixture_file("emails/valid_new_issue_empty.eml") } + let(:email_raw) { email_fixture("emails/valid_new_issue_empty.eml") } it "creates a new issue" do expect { receiver.execute }.to change { project.issues.count }.by(1) @@ -50,7 +87,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end context "when there are quotes in email" do - let(:email_raw) { fixture_file("emails/valid_new_issue_with_quote.eml") } + let(:email_raw) { email_fixture("emails/valid_new_issue_with_quote.eml") } it "creates a new issue" do expect { receiver.execute }.to change { project.issues.count }.by(1) @@ -76,7 +113,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end context "when we can't find the incoming_email_token" do - let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + let(:email_raw) { email_fixture("emails/wrong_issue_incoming_email_token.eml") } it "raises an UserNotFoundError" do expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) @@ -91,4 +128,8 @@ describe Gitlab::Email::Handler::CreateIssueHandler do end end end + + def email_fixture(path) + fixture_file(path).gsub('project_id', project.project_id.to_s) + end end diff --git a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb index f276f1a8ddf..2fa86b2b46f 100644 --- a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do TestEnv.clean_test_path end - let(:email_raw) { fixture_file('emails/valid_new_merge_request.eml') } + let(:email_raw) { email_fixture('emails/valid_new_merge_request.eml') } let(:namespace) { create(:namespace, path: 'gitlabhq') } let!(:project) { create(:project, :public, :repository, namespace: namespace, path: 'gitlabhq') } @@ -27,6 +27,33 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do ) end + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "gitlabhq-gitlabhq-#{project.project_id}-#{user.incoming_email_token}-merge-request") + + expect(handler.instance_variable_get(:@project_id)).to eq project.project_id + expect(handler.instance_variable_get(:@project_slug)).to eq project.full_path_slug + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "h5bp/html5-boilerplate+merge-request+#{user.incoming_email_token}") + + expect(handler.instance_variable_get(:@project_path)).to eq 'h5bp/html5-boilerplate' + expect(handler.instance_variable_get(:@incoming_email_token)).to eq user.incoming_email_token + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "h5bp-html5-boilerplate+merge-request") + + expect(handler.can_handle?).to be_falsey + end + end + context "as a non-developer" do before do project.add_guest(user) @@ -43,15 +70,25 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when everything is fine" do - it "creates a new merge request" do - expect { receiver.execute }.to change { project.merge_requests.count }.by(1) - merge_request = project.merge_requests.last - - expect(merge_request.author).to eq(user) - expect(merge_request.source_branch).to eq('feature') - expect(merge_request.title).to eq('Feature added') - expect(merge_request.description).to eq('Merge request description') - expect(merge_request.target_branch).to eq(project.default_branch) + shared_examples "a new merge request" do + it "creates a new merge request" do + expect { receiver.execute }.to change { project.merge_requests.count }.by(1) + merge_request = project.merge_requests.last + + expect(merge_request.author).to eq(user) + expect(merge_request.source_branch).to eq('feature') + expect(merge_request.title).to eq('Feature added') + expect(merge_request.description).to eq('Merge request description') + expect(merge_request.target_branch).to eq(project.default_branch) + end + end + + it_behaves_like "a new merge request" + + context "creates a new merge request with legacy email address" do + let(:email_raw) { fixture_file('emails/valid_new_merge_request_legacy.eml') } + + it_behaves_like "a new merge request" end end @@ -67,7 +104,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when we can't find the incoming_email_token" do - let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + let(:email_raw) { email_fixture("emails/wrong_merge_request_incoming_email_token.eml") } it "raises an UserNotFoundError" do expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) @@ -75,7 +112,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when the subject is blank" do - let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_subject.eml") } + let(:email_raw) { email_fixture("emails/valid_new_merge_request_no_subject.eml") } it "raises an InvalidMergeRequestError" do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) @@ -83,7 +120,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context "when the message body is blank" do - let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_description.eml") } + let(:email_raw) { email_fixture("emails/valid_new_merge_request_no_description.eml") } it "creates a new merge request with description set from the last commit" do expect { receiver.execute }.to change { project.merge_requests.count }.by(1) @@ -95,7 +132,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when the email contains patch attachments' do - let(:email_raw) { fixture_file("emails/valid_merge_request_with_patch.eml") } + let(:email_raw) { email_fixture("emails/valid_merge_request_with_patch.eml") } it 'creates the source branch and applies the patches' do receiver.execute @@ -120,7 +157,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when the patch could not be applied' do - let(:email_raw) { fixture_file("emails/merge_request_with_conflicting_patch.eml") } + let(:email_raw) { email_fixture("emails/merge_request_with_conflicting_patch.eml") } it 'raises an error' do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidAttachment) @@ -128,7 +165,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end context 'when specifying the target branch using quick actions' do - let(:email_raw) { fixture_file('emails/merge_request_with_patch_and_target_branch.eml') } + let(:email_raw) { email_fixture('emails/merge_request_with_patch_and_target_branch.eml') } it 'creates the merge request with the correct target branch' do receiver.execute @@ -150,7 +187,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do end describe '#patch_attachments' do - let(:email_raw) { fixture_file('emails/merge_request_multiple_patches.eml') } + let(:email_raw) { email_fixture('emails/merge_request_multiple_patches.eml') } let(:mail) { Mail::Message.new(email_raw) } subject(:handler) { described_class.new(mail, mail_key) } @@ -163,4 +200,8 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do expect(attachments).to eq(expected_filenames) end end + + def email_fixture(path) + fixture_file(path).gsub('project_id', project.project_id.to_s) + end end diff --git a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb index b8660b133ec..dcddd00df59 100644 --- a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb @@ -10,13 +10,35 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do stub_config_setting(host: 'localhost') end - let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}+unsubscribe") } - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - let(:noteable) { create(:issue, project: project) } + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") } + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:noteable) { create(:issue, project: project) } let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) } + context "when email key" do + let(:mail) { Mail::Message.new(email_raw) } + + it "matches the new format" do + handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") + + expect(handler.can_handle?).to be_truthy + end + + it "matches the legacy format" do + handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}") + + expect(handler.can_handle?).to be_truthy + end + + it "doesn't match either format" do + handler = described_class.new(mail, "+#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") + + expect(handler.can_handle?).to be_falsey + end + end + context 'when notification concerns a commit' do let(:commit) { create(:commit, project: project) } let!(:sent_notification) { SentNotification.record(commit, user.id, mail_key) } @@ -40,6 +62,14 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do it 'unsubscribes user from notable' do expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false) end + + context 'when using old style unsubscribe link' do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}") } + + it 'unsubscribes user from notable' do + expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false) + end + end end context 'when the noteable could not be found' do diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb index c651765dc0f..d2920b08956 100644 --- a/spec/lib/gitlab/email/handler_spec.rb +++ b/spec/lib/gitlab/email/handler_spec.rb @@ -19,7 +19,8 @@ describe Gitlab::Email::Handler do describe 'regexps are set properly' do let(:addresses) do - %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path/to/project+merge-request+user_email_token path/to/project+user_email_token) + %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path-to-project-123-user_email_token-merge-request path-to-project-123-user_email_token-issue) + + %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY} sent_notification_key path/to/project+merge-request+user_email_token path/to/project+user_email_token) end it 'picks each handler at least once' do diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index 4c0c3fcbcc7..2db62ab983a 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/incoming_email_spec.rb @@ -61,7 +61,7 @@ describe Gitlab::IncomingEmail do end it 'returns the address with interpolated reply key and unsubscribe suffix' do - expect(described_class.unsubscribe_address('key')).to eq('replies+key+unsubscribe@example.com') + expect(described_class.unsubscribe_address('key')).to eq("replies+key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}@example.com") end end |