diff options
29 files changed, 347 insertions, 70 deletions
diff --git a/app/assets/javascripts/issuable_index.js b/app/assets/javascripts/issuable_index.js index 0b123a11a3b..c3e0acdff66 100644 --- a/app/assets/javascripts/issuable_index.js +++ b/app/assets/javascripts/issuable_index.js @@ -28,7 +28,7 @@ export default class IssuableIndex { url: $('.incoming-email-token-reset').attr('href'), dataType: 'json', success(response) { - $('#issue_email').val(response.new_issue_address).focus(); + $('#issuable_email').val(response.new_address).focus(); }, beforeSend() { $('.incoming-email-token-reset').text('resetting...'); diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index d3dda2e7d25..8218326ae8f 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -164,12 +164,7 @@ ul.related-merge-requests > li { } } -.issues-footer { - padding-top: $gl-padding; - padding-bottom: 37px; -} - -.issue-email-modal-btn { +.issuable-email-modal-btn { padding: 0; color: $gl-link-color; background-color: transparent; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 4c1e6d46242..9345177a4dc 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -1210,3 +1210,8 @@ pre.light-well { border-color: $border-color; } } + +.issuable-footer { + padding-top: $gl-padding; + padding-bottom: 37px; +} diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index a784c6f402a..3882fa4791d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -133,11 +133,11 @@ class ProjectsController < Projects::ApplicationController redirect_to edit_project_path(@project), status: 302, alert: ex.message end - def new_issue_address + def new_issuable_address return render_404 unless Gitlab::IncomingEmail.supports_issue_creation? current_user.reset_incoming_email_token! - render json: { new_issue_address: @project.new_issue_address(current_user) } + render json: { new_address: @project.new_issuable_address(current_user, params[:issuable_type]) } end def archive diff --git a/app/models/project.rb b/app/models/project.rb index 0a7e7617d9b..cc530076bf7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -752,13 +752,14 @@ class Project < ActiveRecord::Base Gitlab::Routing.url_helpers.project_url(self) end - def new_issue_address(author) + def new_issuable_address(author, address_type) return unless Gitlab::IncomingEmail.supports_issue_creation? && author author.ensure_incoming_email_token! + suffix = address_type == 'merge_request' ? '+merge-request' : '' Gitlab::IncomingEmail.reply_address( - "#{full_path}+#{author.incoming_email_token}") + "#{full_path}#{suffix}+#{author.incoming_email_token}") end def build_commit_note(commit) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index c2fb01466df..9622a5c5462 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -10,8 +10,12 @@ module MergeRequests merge_request.target_branch = find_target_branch merge_request.can_be_created = branches_valid? - compare_branches if branches_present? - assign_title_and_description if merge_request.can_be_created + # compare branches only if branches are valid, otherwise + # compare_branches may raise an error + if merge_request.can_be_created + compare_branches + assign_title_and_description + end merge_request end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 820709583fa..49cf534dc0d 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -35,6 +35,12 @@ module MergeRequests super end + # expose issuable create method so it can be called from email + # handler CreateMergeRequestHandler + def create(merge_request) + super + end + private def update_merge_requests_head_pipeline(merge_request) diff --git a/app/views/projects/_issuable_by_email.html.haml b/app/views/projects/_issuable_by_email.html.haml new file mode 100644 index 00000000000..749e273b2e2 --- /dev/null +++ b/app/views/projects/_issuable_by_email.html.haml @@ -0,0 +1,30 @@ +- name = issuable_type == 'issue' ? 'issue' : 'merge request' + +.issuable-footer.text-center + %button.issuable-email-modal-btn{ type: "button", data: { toggle: "modal", target: "#issuable-email-modal" } } + Email a new #{name} to this project + +#issuable-email-modal.modal.fade{ tabindex: "-1", role: "dialog" } + .modal-dialog{ role: "document" } + .modal-content + .modal-header + %button.close{ type: "button", data: { dismiss: "modal" }, aria: { label: "close" } } + %span{ aria: { hidden: "true" } }= icon("times") + %h4.modal-title + Create new #{name} by email + .modal-body + %p + You can create a new #{name} inside this project by sending an email to the following email address: + .email-modal-input-group.input-group + = text_field_tag :issuable_email, email, class: "monospace js-select-on-focus form-control", readonly: true + .input-group-btn + = clipboard_button(target: '#issuable_email') + %p + = render 'by_email_description' + %p + This is a private email address, generated just for you. + + Anyone who gets ahold of it can create issues or merge requests as if they were you. + You should + = link_to 'reset it', new_issuable_address_project_path(@project, issuable_type: issuable_type), class: 'incoming-email-token-reset' + if that ever happens. diff --git a/app/views/projects/issues/_by_email_description.html.haml b/app/views/projects/issues/_by_email_description.html.haml new file mode 100644 index 00000000000..f2d58534903 --- /dev/null +++ b/app/views/projects/issues/_by_email_description.html.haml @@ -0,0 +1,6 @@ +The subject will be used as the title of the new issue, and the message will be the description. + += link_to 'Quick actions', help_page_path('user/project/quick_actions'), target: '_blank', tabindex: -1 +and styling with += link_to 'Markdown', help_page_path('user/markdown'), target: '_blank', tabindex: -1 +are supported. diff --git a/app/views/projects/issues/_issue_by_email.html.haml b/app/views/projects/issues/_issue_by_email.html.haml deleted file mode 100644 index 264032a3a31..00000000000 --- a/app/views/projects/issues/_issue_by_email.html.haml +++ /dev/null @@ -1,34 +0,0 @@ -.issues-footer.text-center - %button.issue-email-modal-btn{ type: "button", data: { toggle: "modal", target: "#issue-email-modal" } } - Email a new issue to this project - -#issue-email-modal.modal.fade{ tabindex: "-1", role: "dialog" } - .modal-dialog{ role: "document" } - .modal-content - .modal-header - %button.close{ type: "button", data: { dismiss: "modal" }, aria: { label: "close" } } - %span{ aria: { hidden: "true" } }= icon("times") - %h4.modal-title - Create new issue by email - .modal-body - %p - You can create a new issue inside this project by sending an email to the following email address: - .email-modal-input-group.input-group - = text_field_tag :issue_email, email, class: "monospace js-select-on-focus form-control", readonly: true - .input-group-btn - = clipboard_button(target: '#issue_email') - %p - The subject will be used as the title of the new issue, and the message will be the description. - - = link_to 'Quick actions', help_page_path('user/project/quick_actions'), target: '_blank', tabindex: -1 - and styling with - = link_to 'Markdown', help_page_path('user/markdown'), target: '_blank', tabindex: -1 - are supported. - - %p - This is a private email address, generated just for you. - - Anyone who gets ahold of it can create issues as if they were you. - You should - = link_to 'reset it', new_issue_address_project_path(@project), class: 'incoming-email-token-reset' - if that ever happens. diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index bfaf024428d..193111b4cee 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -2,7 +2,7 @@ - @can_bulk_update = can?(current_user, :admin_issue, @project) - page_title "Issues" -- new_issue_email = @project.new_issue_address(current_user) +- new_issue_email = @project.new_issuable_address(current_user, 'issue') - content_for :page_specific_javascripts do = webpack_bundle_tag 'common_vue' @@ -25,6 +25,6 @@ .issues-holder = render 'issues' - if new_issue_email - = render 'issue_by_email', email: new_issue_email + = render 'projects/issuable_by_email', email: new_issue_email, issuable_type: 'issue' - else = render 'shared/empty_states/issues', button_path: new_project_issue_path(@project) diff --git a/app/views/projects/merge_requests/_by_email_description.html.haml b/app/views/projects/merge_requests/_by_email_description.html.haml new file mode 100644 index 00000000000..8ba251749b8 --- /dev/null +++ b/app/views/projects/merge_requests/_by_email_description.html.haml @@ -0,0 +1 @@ +The subject will be used as the source branch name for the new merge request and the target branch will be the default branch for the project. diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index 8da2243adef..2ded7484151 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/app/views/projects/merge_requests/index.html.haml @@ -4,6 +4,7 @@ - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project - page_title "Merge Requests" +- new_merge_request_email = @project.new_issuable_address(current_user, 'merge_request') - content_for :page_specific_javascripts do = webpack_bundle_tag 'common_vue' @@ -25,5 +26,7 @@ .merge-requests-holder = render 'merge_requests' + - if new_merge_request_email + = render 'projects/issuable_by_email', email: new_merge_request_email, issuable_type: 'merge_request' - else = render 'shared/empty_states/merge_requests', button_path: new_merge_request_path diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index 1afa24c8e2a..05cb116a623 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -39,8 +39,7 @@ class EmailReceiverWorker "You are not allowed to perform this action. If you believe this is in error, contact a staff member." when Gitlab::Email::NoteableNotFoundError "The thread you are replying to no longer exists, perhaps it was deleted? If you believe this is in error, contact a staff member." - when Gitlab::Email::InvalidNoteError, - Gitlab::Email::InvalidIssueError + when Gitlab::Email::InvalidRecordError can_retry = true e.message end diff --git a/changelogs/unreleased/32878-merge-request-from-email.yml b/changelogs/unreleased/32878-merge-request-from-email.yml new file mode 100644 index 00000000000..2df148d5a81 --- /dev/null +++ b/changelogs/unreleased/32878-merge-request-from-email.yml @@ -0,0 +1,5 @@ +--- +title: Allow creation of merge request from email +merge_request: 13817 +author: janp +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index 45474034822..093da10f57f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -435,7 +435,7 @@ constraints(ProjectUrlConstrainer.new) do get :download_export get :activity get :refs - put :new_issue_address + put :new_issuable_address end end end diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 4b2e042251b..d76ea259301 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -27,7 +27,7 @@ With GitLab merge requests, you can: - [Resolve merge conflicts from the UI](#resolve-conflicts) - Enable [fast-forward merge requests](#fast-forward-merge-requests) - Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch - +- [Create new merge requests by email](#create_by_email) With **[GitLab Enterprise Edition][ee]**, you can also: @@ -132,6 +132,14 @@ those conflicts in the GitLab UI. [Learn more about resolving merge conflicts in the UI.](resolve_conflicts.md) +## Create new merge requests by email + +You can create a new merge request by sending an email to a user-specific email +address. The address can be obtained on the merge requests page by clicking on +a **Email a new merge request to this project** button. The subject will be +used as the source branch name for the new merge request and the target branch +will be the default branch for the project. + ## Revert changes GitLab implements Git's powerful feature to revert any commit with introducing diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb index b07c68d1498..e08b5be8984 100644 --- a/lib/gitlab/email/handler.rb +++ b/lib/gitlab/email/handler.rb @@ -1,3 +1,4 @@ +require 'gitlab/email/handler/create_merge_request_handler' require 'gitlab/email/handler/create_note_handler' require 'gitlab/email/handler/create_issue_handler' require 'gitlab/email/handler/unsubscribe_handler' @@ -8,6 +9,7 @@ module Gitlab HANDLERS = [ UnsubscribeHandler, CreateNoteHandler, + CreateMergeRequestHandler, CreateIssueHandler ].freeze diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb new file mode 100644 index 00000000000..c63666b98c1 --- /dev/null +++ b/lib/gitlab/email/handler/create_merge_request_handler.rb @@ -0,0 +1,67 @@ +require 'gitlab/email/handler/base_handler' +require 'gitlab/email/handler/reply_processing' + +module Gitlab + module Email + module Handler + class CreateMergeRequestHandler < BaseHandler + include ReplyProcessing + attr_reader :project_path, :incoming_email_token + + def initialize(mail, mail_key) + super(mail, mail_key) + if m = /\A([^\+]*)\+merge-request\+(.*)/.match(mail_key.to_s) + @project_path, @incoming_email_token = m.captures + end + end + + def can_handle? + @project_path && @incoming_email_token + end + + def execute + raise ProjectNotFound unless project + + validate_permission!(:create_merge_request) + + verify_record!( + record: create_merge_request, + invalid_exception: InvalidMergeRequestError, + record_name: 'merge_request') + end + + def author + @author ||= User.find_by(incoming_email_token: incoming_email_token) + end + + def project + @project ||= Project.find_by_full_path(project_path) + end + + def metrics_params + super.merge(project: project&.full_path) + end + + private + + def create_merge_request + merge_request = MergeRequests::BuildService.new(project, author, merge_request_params).execute + + if merge_request.errors.any? + merge_request + else + MergeRequests::CreateService.new(project, author).create(merge_request) + end + end + + def merge_request_params + { + source_project_id: project.id, + source_branch: mail.subject, + target_project_id: project.id + } + end + end + end + end +end diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index c8f4591d060..d8c594ad0e7 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -13,8 +13,10 @@ module Gitlab UserBlockedError = Class.new(ProcessingError) UserNotAuthorizedError = Class.new(ProcessingError) NoteableNotFoundError = Class.new(ProcessingError) - InvalidNoteError = Class.new(ProcessingError) - InvalidIssueError = Class.new(ProcessingError) + InvalidRecordError = Class.new(ProcessingError) + InvalidNoteError = Class.new(InvalidRecordError) + InvalidIssueError = Class.new(InvalidRecordError) + InvalidMergeRequestError = Class.new(InvalidRecordError) UnknownIncomingEmail = Class.new(ProcessingError) class Receiver diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e4b2bbb7c51..e61187fb518 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -426,11 +426,12 @@ describe ProjectsController do end end - describe 'PUT #new_issue_address' do + describe 'PUT #new_issuable_address for issue' do subject do - put :new_issue_address, + put :new_issuable_address, namespace_id: project.namespace, - id: project + id: project, + issuable_type: 'issue' user.reload end @@ -449,7 +450,35 @@ describe ProjectsController do end it 'changes projects new issue address' do - expect { subject }.to change { project.new_issue_address(user) } + expect { subject }.to change { project.new_issuable_address(user, 'issue') } + end + end + + describe 'PUT #new_issuable_address for merge request' do + subject do + put :new_issuable_address, + namespace_id: project.namespace, + id: project, + issuable_type: 'merge_request' + user.reload + end + + before do + sign_in(user) + project.team << [user, :developer] + allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true) + end + + it 'has http status 200' do + expect(response).to have_http_status(200) + end + + it 'changes the user incoming email token' do + expect { subject }.to change { user.incoming_email_token } + end + + it 'changes projects new merge request address' do + expect { subject }.to change { project.new_issuable_address(user, 'merge_request') } end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index b9af77f918a..852d9e368aa 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -365,16 +365,16 @@ describe 'Issues' do end it 'changes incoming email address token', :js do - find('.issue-email-modal-btn').click - previous_token = find('input#issue_email').value + find('.issuable-email-modal-btn').click + previous_token = find('input#issuable_email').value find('.incoming-email-token-reset').click wait_for_requests - expect(page).to have_no_field('issue_email', with: previous_token) - new_token = project1.new_issue_address(user.reload) + expect(page).to have_no_field('issuable_email', with: previous_token) + new_token = project1.new_issuable_address(user.reload, 'issue') expect(page).to have_field( - 'issue_email', + 'issuable_email', with: new_token ) end @@ -630,8 +630,8 @@ describe 'Issues' do end it 'click the button to show modal for the new email' do - page.within '#issue-email-modal' do - email = project.new_issue_address(user) + page.within '#issuable-email-modal' do + email = project.new_issuable_address(user, 'issue') expect(page).to have_selector("input[value='#{email}']") end diff --git a/spec/fixtures/emails/valid_new_merge_request.eml b/spec/fixtures/emails/valid_new_merge_request.eml new file mode 100644 index 00000000000..480675a6d7e --- /dev/null +++ b/spec/fixtures/emails/valid_new_merge_request.eml @@ -0,0 +1,18 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+merge-request+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: feature +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 diff --git a/spec/fixtures/emails/valid_new_merge_request_no_subject.eml b/spec/fixtures/emails/valid_new_merge_request_no_subject.eml new file mode 100644 index 00000000000..27eb1b7d922 --- /dev/null +++ b/spec/fixtures/emails/valid_new_merge_request_no_subject.eml @@ -0,0 +1,18 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+merge-request+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 diff --git a/spec/javascripts/issuable_spec.js b/spec/javascripts/issuable_spec.js index ceee08d47c5..5a9112716f4 100644 --- a/spec/javascripts/issuable_spec.js +++ b/spec/javascripts/issuable_spec.js @@ -26,7 +26,7 @@ describe('Issuable', () => { document.body.appendChild(element); const input = document.createElement('input'); - input.setAttribute('id', 'issue_email'); + input.setAttribute('id', 'issuable_email'); document.body.appendChild(input); Issuable = new IssuableIndex('issue_'); 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 new file mode 100644 index 00000000000..e361d1a7393 --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateMergeRequestHandler do + include_context :email_shared_context + it_behaves_like :reply_processing_shared_examples + + before do + stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo") + stub_config_setting(host: 'localhost') + end + + let(:email_raw) { fixture_file('emails/valid_new_merge_request.eml') } + let(:namespace) { create(:namespace, path: 'gitlabhq') } + + # project's git repository is not deleted when project is deleted + # between tests. Then tests fail because re-creation of the project with + # the same name fails on existing git repository -> skip_disk_validation + # ignores repository existence on disk + let!(:project) { create(:project, :public, :repository, skip_disk_validation: true, namespace: namespace, path: 'gitlabhq') } + let!(:user) do + create( + :user, + email: 'jake@adventuretime.ooo', + incoming_email_token: 'auth_token' + ) + end + + context "as a non-developer" do + before do + project.add_guest(user) + end + + it "raises UserNotAuthorizedError if the user is not a member" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError) + end + end + + context "as a developer" do + before do + project.add_developer(user) + 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.target_branch).to eq(project.default_branch) + end + end + + context "something is wrong" do + context "when the merge request could not be saved" do + before do + allow_any_instance_of(MergeRequest).to receive(:save).and_return(false) + end + + it "raises an InvalidMergeRequestError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) + end + end + + context "when we can't find the incoming_email_token" do + let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") } + + it "raises an UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + end + end + + context "when the subject is blank" do + let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_subject.eml") } + + it "raises an InvalidMergeRequestError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) + end + end + end + end +end diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb new file mode 100644 index 00000000000..650b01c4df4 --- /dev/null +++ b/spec/lib/gitlab/email/handler_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Gitlab::Email::Handler do + describe '.for' do + it 'picks issue handler if there is not merge request prefix' do + expect(described_class.for('email', 'project+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateIssueHandler) + end + + it 'picks merge request handler if there is merge request key' do + expect(described_class.for('email', 'project+merge-request+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateMergeRequestHandler) + end + + it 'returns nil if no handler is found' do + expect(described_class.for('email', '')).to be_nil + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 000d5b7126d..6bda1eb15a8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -451,7 +451,7 @@ describe Project do end end - describe "#new_issue_address" do + describe "#new_issuable_address" do let(:project) { create(:project, path: "somewhere") } let(:user) { create(:user) } @@ -463,7 +463,13 @@ describe Project do it 'returns the address to create a new issue' do address = "p+#{project.full_path}+#{user.incoming_email_token}@gl.ab" - expect(project.new_issue_address(user)).to eq(address) + expect(project.new_issuable_address(user, 'issue')).to eq(address) + end + + it 'returns the address to create a new merge request' do + address = "p+#{project.full_path}+merge-request+#{user.incoming_email_token}@gl.ab" + + expect(project.new_issuable_address(user, 'merge_request')).to eq(address) end end @@ -473,7 +479,11 @@ describe Project do end it 'returns nil' do - expect(project.new_issue_address(user)).to be_nil + expect(project.new_issuable_address(user, 'issue')).to be_nil + end + + it 'returns nil' do + expect(project.new_issuable_address(user, 'merge_request')).to be_nil end end end diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index fee293760f5..b5c92e681fb 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -39,6 +39,7 @@ describe MergeRequests::BuildService do describe '#execute' do it 'calls the compare service with the correct arguments' do + allow_any_instance_of(described_class).to receive(:branches_valid?).and_return(true) expect(CompareService).to receive(:new) .with(project, Gitlab::Git::BRANCH_REF_PREFIX + source_branch) .and_call_original |