diff options
25 files changed, 686 insertions, 264 deletions
diff --git a/CHANGELOG b/CHANGELOG index 833a55e44f9..8759bb04dec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,6 +13,7 @@ v 8.11.0 (unreleased) - Retrieve rendered HTML from cache in one request - Fix renaming repository when name contains invalid chararacters under project settings - Nokogiri's various parsing methods are now instrumented + - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 - Add build event color in HipChat messages (David Eisner) - Make fork counter always clickable. !5463 (winniehell) - All created issues, API or WebUI, can be submitted to Akismet for spam check !5333 diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index ee3b2d2b801..dfe1e3075da 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -99,3 +99,33 @@ form.edit-issue { .issue-form .select2-container { width: 250px !important; } + +.issues-footer { + padding-top: $gl-padding; + padding-bottom: 37px; +} + +.issue-email-modal-btn { + padding: 0; + color: $gl-link-color; + background-color: transparent; + border: 0; + outline: 0; + + &:hover { + text-decoration: underline; + } +} + +.email-modal-input-group { + margin-bottom: 10px; + + .form-control { + background-color: $white-light; + } + + .btn { + background-color: $background-color; + border: 1px solid $border-gray-light; + } +} diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index cbfa14e81f1..aac78d75f57 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -331,7 +331,7 @@ module Ci end def valid_token?(token) - project.valid_runners_token? token + project.valid_runners_token?(token) end def has_tags? diff --git a/app/models/project.rb b/app/models/project.rb index b3703d71e72..e2bf5f3a3fb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -605,6 +605,13 @@ class Project < ActiveRecord::Base web_url.split('://')[1] end + def new_issue_address(author) + if Gitlab::IncomingEmail.enabled? && author + Gitlab::IncomingEmail.reply_address( + "#{path_with_namespace}+#{author.authentication_token}") + end + end + def build_commit_note(commit) notes.new(commit_id: commit.id, noteable_type: 'Commit') end diff --git a/app/views/projects/issues/_issue_by_email.html.haml b/app/views/projects/issues/_issue_by_email.html.haml new file mode 100644 index 00000000000..72669372497 --- /dev/null +++ b/app/views/projects/issues/_issue_by_email.html.haml @@ -0,0 +1,27 @@ +.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 + Write an email to the below email address. (This is a private email address, so keep it secret.) + .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(clipboard_target: '#issue_email') + %p + Send an email to this address to create an issue. + %p + Use the subject line as the title of your issue. + %p + Use the message as the body of your issue (feel free to include some nice + = succeed ")." do + = link_to "Markdown", help_page_path('markdown', 'markdown') diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index 7612fe3719a..d0edd2f22ec 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -1,5 +1,6 @@ - @no_container = true - page_title "Issues" +- new_issue_email = @project.new_issue_address(current_user) = render "projects/issues/head" = content_for :meta_tags do @@ -23,7 +24,9 @@ = render 'shared/issuable/filter', type: :issues .issues-holder - = render "issues" + = render 'issues' + - if new_issue_email + = render 'issue_by_email', email: new_issue_email - else .blank-state.blank-state-welcome %h2.blank-state-title.blank-state-welcome-title @@ -40,3 +43,5 @@ - if can? current_user, :create_issue, @project = link_to new_namespace_project_issue_path(@project.namespace, @project), class: "btn btn-new", title: "New Issue", id: "new_issue_link" do New Issue + - if new_issue_email + = render 'issue_by_email', email: new_issue_email diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index f2649e38eb3..842eebdea9e 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -21,31 +21,35 @@ class EmailReceiverWorker return unless raw.present? can_retry = false - reason = nil - - 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::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." - when Gitlab::Email::Receiver::AutoGeneratedEmailError - reason = "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface." - when Gitlab::Email::Receiver::UserNotFoundError - reason = "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." - when Gitlab::Email::Receiver::UserBlockedError - reason = "Your account has been blocked. If you believe this is in error, contact a staff member." - when Gitlab::Email::Receiver::UserNotAuthorizedError - reason = "You are not allowed to respond to the thread you are replying to. If you believe this is in error, contact a staff member." - when Gitlab::Email::Receiver::NoteableNotFoundError - reason = "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::Receiver::InvalidNoteError - can_retry = true - reason = e.message - else - return + reason = + case e + when Gitlab::Email::UnknownIncomingEmail + "We couldn't figure out what the email is for. Please create your issue or comment through the web interface." + when Gitlab::Email::SentNotificationNotFoundError + "We couldn't figure out what the email is in reply to. Please create your comment through the web interface." + when Gitlab::Email::ProjectNotFound + "We couldn't find the project. Please check if there's any typo." + when Gitlab::Email::EmptyEmailError + can_retry = true + "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies." + when Gitlab::Email::AutoGeneratedEmailError + "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface." + when Gitlab::Email::UserNotFoundError + "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." + when Gitlab::Email::UserBlockedError + "Your account has been blocked. If you believe this is in error, contact a staff member." + when Gitlab::Email::UserNotAuthorizedError + "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 + can_retry = true + e.message + end + + if reason + EmailRejectionMailer.rejection(reason, raw, can_retry).deliver_later end - - EmailRejectionMailer.rejection(reason, raw, can_retry).deliver_later end end diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb new file mode 100644 index 00000000000..bd3267e2a80 --- /dev/null +++ b/lib/gitlab/email/handler.rb @@ -0,0 +1,17 @@ +require 'gitlab/email/handler/create_note_handler' +require 'gitlab/email/handler/create_issue_handler' + +module Gitlab + module Email + module Handler + HANDLERS = [CreateNoteHandler, CreateIssueHandler] + + def self.for(mail, mail_key) + HANDLERS.find do |klass| + handler = klass.new(mail, mail_key) + break handler if handler.can_handle? + end + end + end + end +end diff --git a/lib/gitlab/email/handler/base_handler.rb b/lib/gitlab/email/handler/base_handler.rb new file mode 100644 index 00000000000..b7ed11cb638 --- /dev/null +++ b/lib/gitlab/email/handler/base_handler.rb @@ -0,0 +1,60 @@ +module Gitlab + module Email + module Handler + class BaseHandler + attr_reader :mail, :mail_key + + def initialize(mail, mail_key) + @mail = mail + @mail_key = mail_key + end + + def message + @message ||= process_message + end + + def author + raise NotImplementedError + end + + def project + raise NotImplementedError + end + + private + + def validate_permission!(permission) + raise UserNotFoundError unless author + raise UserBlockedError if author.blocked? + raise ProjectNotFound unless author.can?(:read_project, project) + raise UserNotAuthorizedError unless author.can?(permission, project) + end + + def process_message + message = ReplyParser.new(mail).execute.strip + add_attachments(message) + end + + def add_attachments(reply) + attachments = Email::AttachmentUploader.new(mail).execute(project) + + reply + attachments.map do |link| + "\n\n#{link[:markdown]}" + end.join + end + + def verify_record!(record:, invalid_exception:, record_name:) + return if record.persisted? + + error_title = "The #{record_name} could not be created for the following reasons:" + + msg = error_title + record.errors.full_messages.map do |error| + "\n\n- #{error}" + end.join + + raise invalid_exception, msg + end + end + end + end +end diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb new file mode 100644 index 00000000000..4e6566af8ab --- /dev/null +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -0,0 +1,52 @@ + +require 'gitlab/email/handler/base_handler' + +module Gitlab + module Email + module Handler + class CreateIssueHandler < BaseHandler + attr_reader :project_path, :authentication_token + + def initialize(mail, mail_key) + super(mail, mail_key) + @project_path, @authentication_token = + mail_key && mail_key.split('+', 2) + end + + def can_handle? + !authentication_token.nil? + end + + def execute + raise ProjectNotFound unless project + + validate_permission!(:create_issue) + + verify_record!( + record: create_issue, + invalid_exception: InvalidIssueError, + record_name: 'issue') + end + + def author + @author ||= User.find_by(authentication_token: authentication_token) + end + + def project + @project ||= Project.find_with_namespace(project_path) + end + + private + + def create_issue + Issues::CreateService.new( + project, + author, + title: mail.subject, + description: message + ).execute + end + end + end + end +end diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb new file mode 100644 index 00000000000..06dae31cc27 --- /dev/null +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -0,0 +1,55 @@ + +require 'gitlab/email/handler/base_handler' + +module Gitlab + module Email + module Handler + class CreateNoteHandler < BaseHandler + def can_handle? + mail_key =~ /\A\w+\z/ + end + + def execute + raise SentNotificationNotFoundError unless sent_notification + raise AutoGeneratedEmailError if mail.header.to_s =~ /auto-(generated|replied)/ + + validate_permission!(:create_note) + + raise NoteableNotFoundError unless sent_notification.noteable + raise EmptyEmailError if message.blank? + + verify_record!( + record: create_note, + invalid_exception: InvalidNoteError, + record_name: 'comment') + end + + def author + sent_notification.recipient + end + + def project + sent_notification.project + end + + def sent_notification + @sent_notification ||= SentNotification.for(mail_key) + end + + private + + def create_note + Notes::CreateService.new( + project, + author, + note: message, + noteable_type: sent_notification.noteable_type, + noteable_id: sent_notification.noteable_id, + commit_id: sent_notification.commit_id, + line_code: sent_notification.line_code + ).execute + end + end + end + end +end diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 1c671a7487b..9213cfb51e8 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -1,18 +1,24 @@ + +require 'gitlab/email/handler' + # Inspired in great part by Discourse's Email::Receiver module Gitlab module Email - class Receiver - class ProcessingError < StandardError; end - class EmailUnparsableError < ProcessingError; end - class SentNotificationNotFoundError < ProcessingError; end - class EmptyEmailError < ProcessingError; end - class AutoGeneratedEmailError < ProcessingError; end - class UserNotFoundError < ProcessingError; end - class UserBlockedError < ProcessingError; end - class UserNotAuthorizedError < ProcessingError; end - class NoteableNotFoundError < ProcessingError; end - class InvalidNoteError < ProcessingError; end + 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 + class UserBlockedError < ProcessingError; end + class UserNotAuthorizedError < ProcessingError; end + class NoteableNotFoundError < ProcessingError; end + class InvalidNoteError < ProcessingError; end + class InvalidIssueError < ProcessingError; end + class UnknownIncomingEmail < ProcessingError; end + class Receiver def initialize(raw) @raw = raw end @@ -20,91 +26,38 @@ module Gitlab def execute raise EmptyEmailError if @raw.blank? - raise SentNotificationNotFoundError unless sent_notification - - raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/ - - author = sent_notification.recipient - - raise UserNotFoundError unless author - - raise UserBlockedError if author.blocked? - - project = sent_notification.project - - raise UserNotAuthorizedError unless project && author.can?(:create_note, project) - - raise NoteableNotFoundError unless sent_notification.noteable - - reply = ReplyParser.new(message).execute.strip - - raise EmptyEmailError if reply.blank? - - reply = add_attachments(reply) - - note = create_note(reply) + mail = build_mail + mail_key = extract_mail_key(mail) + handler = Handler.for(mail, mail_key) - unless note.persisted? - msg = "The comment could not be created for the following reasons:" - note.errors.full_messages.each do |error| - msg << "\n\n- #{error}" - end + raise UnknownIncomingEmail unless handler - raise InvalidNoteError, msg - end + handler.execute end - private - - def message - @message ||= Mail::Message.new(@raw) - rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e + def build_mail + Mail::Message.new(@raw) + rescue Encoding::UndefinedConversionError, + Encoding::InvalidByteSequenceError => e raise EmailUnparsableError, e end - def reply_key - key_from_to_header || key_from_additional_headers + def extract_mail_key(mail) + key_from_to_header(mail) || key_from_additional_headers(mail) end - def key_from_to_header - key = nil - message.to.each do |address| + def key_from_to_header(mail) + mail.to.find do |address| key = Gitlab::IncomingEmail.key_from_address(address) - break if key + break key if key end - - key end - def key_from_additional_headers - reply_key = nil - - Array(message.references).each do |message_id| - reply_key = Gitlab::IncomingEmail.key_from_fallback_reply_message_id(message_id) - break if reply_key + def key_from_additional_headers(mail) + Array(mail.references).find do |mail_id| + key = Gitlab::IncomingEmail.key_from_fallback_message_id(mail_id) + break key if key end - - reply_key - end - - def sent_notification - return nil unless reply_key - - SentNotification.for(reply_key) - end - - def add_attachments(reply) - attachments = Email::AttachmentUploader.new(message).execute(sent_notification.project) - - attachments.each do |link| - reply << "\n\n#{link[:markdown]}" - end - - reply - end - - def create_note(reply) - sent_notification.create_note(reply) end end end diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb index 8ce9d32abe0..d7be50bd437 100644 --- a/lib/gitlab/incoming_email.rb +++ b/lib/gitlab/incoming_email.rb @@ -1,7 +1,7 @@ module Gitlab module IncomingEmail class << self - FALLBACK_REPLY_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze + FALLBACK_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze def enabled? config.enabled && config.address @@ -21,8 +21,8 @@ module Gitlab match[1] end - def key_from_fallback_reply_message_id(message_id) - match = message_id.match(FALLBACK_REPLY_MESSAGE_ID_REGEX) + def key_from_fallback_message_id(mail_id) + match = mail_id.match(FALLBACK_MESSAGE_ID_REGEX) return unless match match[1] diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index d51c9abea19..93dcb2ec3fc 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -524,6 +524,35 @@ describe 'Issues', feature: true do end end + describe 'new issue by email' do + shared_examples 'show the email in the modal' do + before do + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + + visit namespace_project_issues_path(project.namespace, project) + click_button('Email a new issue') + 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) + + expect(page).to have_selector("input[value='#{email}']") + end + end + end + + context 'with existing issues' do + let!(:issue) { create(:issue, project: project, author: @user) } + + it_behaves_like 'show the email in the modal' + end + + context 'without existing issues' do + it_behaves_like 'show the email in the modal' + end + end + describe 'due date' do context 'update due on issue#show', js: true do let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } diff --git a/spec/fixtures/emails/valid_new_issue.eml b/spec/fixtures/emails/valid_new_issue.eml new file mode 100644 index 00000000000..3cf53a656a5 --- /dev/null +++ b/spec/fixtures/emails/valid_new_issue.eml @@ -0,0 +1,23 @@ +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+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +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 + +The reply by email functionality should be extended to allow creating a new issue by email. + +* Allow an admin to specify which project the issue should be created under by checking the sender domain. +* Possibly allow the use of regular expression matches within the subject/body to specify which project the issue should be created under. diff --git a/spec/fixtures/emails/valid_new_issue_empty.eml b/spec/fixtures/emails/valid_new_issue_empty.eml new file mode 100644 index 00000000000..fc1d52a3f42 --- /dev/null +++ b/spec/fixtures/emails/valid_new_issue_empty.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+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +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/wrong_authentication_token.eml b/spec/fixtures/emails/wrong_authentication_token.eml new file mode 100644 index 00000000000..0994c2f7775 --- /dev/null +++ b/spec/fixtures/emails/wrong_authentication_token.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+bad_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +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/wrong_reply_key.eml b/spec/fixtures/emails/wrong_mail_key.eml index 491e078fb5b..491e078fb5b 100644 --- a/spec/fixtures/emails/wrong_reply_key.eml +++ b/spec/fixtures/emails/wrong_mail_key.eml diff --git a/spec/lib/gitlab/email/email_shared_blocks.rb b/spec/lib/gitlab/email/email_shared_blocks.rb new file mode 100644 index 00000000000..19298e261e3 --- /dev/null +++ b/spec/lib/gitlab/email/email_shared_blocks.rb @@ -0,0 +1,41 @@ +require 'gitlab/email/receiver' + +shared_context :email_shared_context do + let(:mail_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } + let(:receiver) { Gitlab::Email::Receiver.new(email_raw) } + let(:markdown) { "" } + + def setup_attachment + allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( + [ + { + url: "uploads/image.png", + alt: "image", + markdown: markdown + } + ] + ) + end +end + +shared_examples :email_shared_examples do + context "when the user could not be found" do + before do + user.destroy + end + + it "raises a UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + end + end + + context "when the user is not authorized to the project" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + it "raises a ProjectNotFound" do + expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound) + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb new file mode 100644 index 00000000000..e1153154778 --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateIssueHandler, lib: true do + include_context :email_shared_context + it_behaves_like :email_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_issue.eml') } + let(:namespace) { create(:namespace, path: 'gitlabhq') } + + let!(:project) { create(:project, :public, namespace: namespace) } + let!(:user) do + create( + :user, + email: 'jake@adventuretime.ooo', + authentication_token: 'auth_token' + ) + end + + context "when everything is fine" do + it "creates a new issue" do + setup_attachment + + 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 + + context "when the reply is blank" do + let(:email_raw) { fixture_file("emails/valid_new_issue_empty.eml") } + + it "creates a new issue" do + 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 eq('') + end + end + end + + context "something is wrong" do + context "when the issue could not be saved" do + before do + allow_any_instance_of(Issue).to receive(:persisted?).and_return(false) + end + + it "raises an InvalidIssueError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidIssueError) + end + end + + context "when we can't find the authentication_token" do + let(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") } + + it "raises an UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + 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::ProjectNotFound) + end + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb new file mode 100644 index 00000000000..a2119b0dadf --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do + include_context :email_shared_context + it_behaves_like :email_shared_examples + + before do + stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo") + stub_config_setting(host: 'localhost') + end + + let(:email_raw) { fixture_file('emails/valid_reply.eml') } + let(:project) { create(:project, :public) } + let(:noteable) { create(:issue, project: project) } + let(:user) { create(:user) } + + let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) } + + context "when the recipient address doesn't include a mail key" do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") } + + it "raises a UnknownIncomingEmail" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) + end + end + + context "when no sent notification for the mail key could be found" do + let(:email_raw) { fixture_file('emails/wrong_mail_key.eml') } + + it "raises a SentNotificationNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError) + end + end + + context "when the email was auto generated" do + let!(:mail_key) { '636ca428858779856c226bb145ef4fad' } + let!(:email_raw) { fixture_file("emails/auto_reply.eml") } + + it "raises an AutoGeneratedEmailError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError) + end + end + + context "when the noteable could not be found" do + before do + noteable.destroy + end + + it "raises a NoteableNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::NoteableNotFoundError) + end + end + + context "when the note could not be saved" do + before do + allow_any_instance_of(Note).to receive(:persisted?).and_return(false) + end + + it "raises an InvalidNoteError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError) + end + end + + context "when the reply is blank" do + let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } + + it "raises an EmptyEmailError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) + end + end + + context "when everything is fine" do + before do + setup_attachment + end + + it "creates a comment" do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + note = noteable.notes.last + + expect(note.author).to eq(sent_notification.recipient) + expect(note.note).to include("I could not disagree more.") + end + + it "adds all attachments" do + receiver.execute + + note = noteable.notes.last + + expect(note.note).to include(markdown) + end + + context 'when sub-addressing is not supported' do + before do + stub_incoming_email_setting(enabled: true, address: nil) + end + + shared_examples 'an email that contains a mail key' do |header| + it "fetches the mail key from the #{header} header and creates a comment" do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + note = noteable.notes.last + + expect(note.author).to eq(sent_notification.recipient) + expect(note.note).to include('I could not disagree more.') + end + end + + context 'mail key is in the References header' do + let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') } + + it_behaves_like 'an email that contains a mail key', 'References' + end + end + end +end diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 84d2584a791..2a86b427806 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -1,34 +1,14 @@ -require "spec_helper" +require 'spec_helper' +require_relative 'email_shared_blocks' describe Gitlab::Email::Receiver, lib: true do - before do - stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo") - stub_config_setting(host: 'localhost') - end - - let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } - let(:email_raw) { fixture_file('emails/valid_reply.eml') } - - let(:project) { create(:project, :public) } - let(:noteable) { create(:issue, project: project) } - let(:user) { create(:user) } - let!(:sent_notification) { SentNotification.record(noteable, user.id, reply_key) } - - let(:receiver) { described_class.new(email_raw) } - - context "when the recipient address doesn't include a reply key" do - let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(reply_key, "") } - - it "raises a SentNotificationNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::SentNotificationNotFoundError) - end - end + include_context :email_shared_context - context "when no sent notificiation for the reply key could be found" do - let(:email_raw) { fixture_file('emails/wrong_reply_key.eml') } + context "when we cannot find a capable handler" do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") } - it "raises a SentNotificationNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::SentNotificationNotFoundError) + it "raises a UnknownIncomingEmail" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) end end @@ -36,128 +16,7 @@ describe Gitlab::Email::Receiver, lib: true do let(:email_raw) { "" } it "raises an EmptyEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) - end - end - - context "when the email was auto generated" do - let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } - let!(:email_raw) { fixture_file("emails/auto_reply.eml") } - - it "raises an AutoGeneratedEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError) - end - end - - context "when the user could not be found" do - before do - user.destroy - end - - it "raises a UserNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotFoundError) - end - end - - context "when the user has been blocked" do - before do - user.block - end - - it "raises a UserBlockedError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserBlockedError) - end - end - - context "when the user is not authorized to create a note" do - before do - project.update_attribute(:visibility_level, Project::PRIVATE) - end - - it "raises a UserNotAuthorizedError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError) - end - end - - context "when the noteable could not be found" do - before do - noteable.destroy - end - - it "raises a NoteableNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::NoteableNotFoundError) - end - end - - context "when the reply is blank" do - let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } - - it "raises an EmptyEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) - end - end - - context "when the note could not be saved" do - before do - allow_any_instance_of(Note).to receive(:persisted?).and_return(false) - end - - it "raises an InvalidNoteError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::InvalidNoteError) - end - end - - context "when everything is fine" do - let(:markdown) { "" } - - before do - allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( - [ - { - url: "uploads/image.png", - alt: "image", - markdown: markdown - } - ] - ) - end - - it "creates a comment" do - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last - - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include("I could not disagree more.") - end - - it "adds all attachments" do - receiver.execute - - note = noteable.notes.last - - expect(note.note).to include(markdown) - end - - context 'when sub-addressing is not supported' do - before do - stub_incoming_email_setting(enabled: true, address: nil) - end - - shared_examples 'an email that contains a reply key' do |header| - it "fetches the reply key from the #{header} header and creates a comment" do - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last - - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include('I could not disagree more.') - end - end - - context 'reply key is in the References header' do - let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') } - - it_behaves_like 'an email that contains a reply key', 'References' - end + expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) end end end diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index afb3e26f8fb..1dcf2c0668b 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/incoming_email_spec.rb @@ -43,9 +43,9 @@ describe Gitlab::IncomingEmail, lib: true do end end - context 'self.key_from_fallback_reply_message_id' do + context 'self.key_from_fallback_message_id' do it 'returns reply key' do - expect(described_class.key_from_fallback_reply_message_id('reply-key@localhost')).to eq('key') + expect(described_class.key_from_fallback_message_id('reply-key@localhost')).to eq('key') end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index be5c53c44be..72b8a4e25bd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -245,6 +245,34 @@ describe Project, models: true do end end + describe "#new_issue_address" do + let(:project) { create(:empty_project, path: "somewhere") } + let(:user) { create(:user) } + + context 'incoming email enabled' do + before do + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + end + + it 'returns the address to create a new issue' do + token = user.authentication_token + address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab" + + expect(project.new_issue_address(user)).to eq(address) + end + end + + context 'incoming email disabled' do + before do + stub_incoming_email_setting(enabled: false) + end + + it 'returns nil' do + expect(project.new_issue_address(user)).to be_nil + end + end + end + describe 'last_activity methods' do let(:project) { create(:project) } let(:last_event) { double(created_at: Time.now) } diff --git a/spec/workers/email_receiver_worker_spec.rb b/spec/workers/email_receiver_worker_spec.rb index de40a6f78af..fe70501eeac 100644 --- a/spec/workers/email_receiver_worker_spec.rb +++ b/spec/workers/email_receiver_worker_spec.rb @@ -17,7 +17,7 @@ describe EmailReceiverWorker do context "when an error occurs" do before do - allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::Receiver::EmptyEmailError) + allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::EmptyEmailError) end it "sends out a rejection email" do |