Merge branch '18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment' into 'master'

Resolve "Forward an E-Mail to an existing issue as a new comment"

See merge request gitlab-org/gitlab!49050

app/graphql/types/issue_type.rb

@@ -121,6 +121,9 @@ module Types
     field :moved_to, Types::IssueType, null: true,
           description: 'Updated Issue after it got moved to another project'
 
+    field :create_note_email, GraphQL::STRING_TYPE, null: true,
+          description: 'User specific email address for the issue'
+
     def author
       Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
     end
@@ -140,6 +143,10 @@ module Types
     def discussion_locked
       !!object.discussion_locked
     end
+
+    def create_note_email
+      object.creatable_note_email_address(context[:current_user])
+    end
   end
 end
 

app/helpers/issuables_helper.rb

@@ -442,7 +442,8 @@ module IssuablesHelper
       fullPath: issuable[:project_full_path],
       iid: issuable[:iid],
       severity: issuable[:severity],
-      timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours
+      timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours,
+      createNoteEmail: issuable[:create_note_email]
     }
   end
 

app/models/concerns/noteable.rb

@@ -19,6 +19,11 @@ module Noteable
     def resolvable_types
       %w(MergeRequest DesignManagement::Design)
     end
+
+    # `Noteable` class names that support creating/forwarding individual notes.
+    def email_creatable_types
+      %w(Issue)
+    end
   end
 
   # The timestamp of the note (e.g. the :created_at or :updated_at attribute if provided via
@@ -55,6 +60,10 @@ module Noteable
     supports_discussions? && self.class.replyable_types.include?(base_class_name)
   end
 
+  def supports_creating_notes_by_email?
+    self.class.email_creatable_types.include?(base_class_name)
+  end
+
   def supports_suggestion?
     false
   end
@@ -158,6 +167,18 @@ module Noteable
   def after_note_destroyed(_note)
     # no-op
   end
+
+  # Email address that an authorized user can send/forward an email to be added directly
+  # to an issue or merge request.
+  # example: incoming+h5bp-html5-boilerplate-8-1234567890abcdef123456789-issue-34@localhost.com
+  def creatable_note_email_address(author)
+    return unless supports_creating_notes_by_email?
+
+    project_email = project.new_issuable_address(author, self.class.name.underscore)
+    return unless project_email
+
+    project_email.sub('@', "-#{iid}@")
+  end
 end
 
 Noteable.extend(Noteable::ClassMethods)

app/serializers/issuable_sidebar_basic_entity.rb

@@ -103,6 +103,10 @@ class IssuableSidebarBasicEntity < Grape::Entity
     issuable.project.emails_disabled?
   end
 
+  expose :create_note_email do |issuable|
+    issuable.creatable_note_email_address(current_user)
+  end
+
   expose :supports_time_tracking?, as: :supports_time_tracking
   expose :supports_milestone?, as: :supports_milestone
   expose :supports_severity?, as: :supports_severity

changelogs/unreleased/18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment.yml

+---
+title: New user/issue specific email address for creating/forwarding to an issue
+merge_request: 49050
+author:
+type: added

doc/api/graphql/reference/gitlab_schema.graphql

@@ -8665,6 +8665,11 @@ type EpicIssue implements CurrentUserTodos & Noteable {
   """
   confidential: Boolean!
 
+  """
+  User specific email address for the issue
+  """
+  createNoteEmail: String
+
   """
   Timestamp of when the issue was created
   """
@@ -11636,6 +11641,11 @@ type Issue implements CurrentUserTodos & Noteable {
   """
   confidential: Boolean!
 
+  """
+  User specific email address for the issue
+  """
+  createNoteEmail: String
+
   """
   Timestamp of when the issue was created
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -24014,6 +24014,20 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "createNoteEmail",
+              "description": "User specific email address for the issue",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "createdAt",
               "description": "Timestamp of when the issue was created",
@@ -31845,6 +31859,20 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "createNoteEmail",
+              "description": "User specific email address for the issue",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "createdAt",
               "description": "Timestamp of when the issue was created",

doc/api/graphql/reference/index.md

@@ -1425,6 +1425,7 @@ Relationship between an epic and an issue.
 | `blockedByCount` | Int | Count of issues blocking this issue. |
 | `closedAt` | Time | Timestamp of when the issue was closed |
 | `confidential` | Boolean! | Indicates the issue is confidential |
+| `createNoteEmail` | String | User specific email address for the issue |
 | `createdAt` | Time! | Timestamp of when the issue was created |
 | `currentUserTodos` | TodoConnection! | Todos for the current user |
 | `description` | String | Description of the issue |
@@ -1768,6 +1769,7 @@ Represents a recorded measurement (object count) for the Admins.
 | `blockedByCount` | Int | Count of issues blocking this issue. |
 | `closedAt` | Time | Timestamp of when the issue was closed |
 | `confidential` | Boolean! | Indicates the issue is confidential |
+| `createNoteEmail` | String | User specific email address for the issue |
 | `createdAt` | Time! | Timestamp of when the issue was created |
 | `currentUserTodos` | TodoConnection! | Todos for the current user |
 | `description` | String | Description of the issue |

lib/gitlab/email/handler.rb

@@ -11,6 +11,7 @@ module Gitlab
         [
           CreateNoteHandler,
           CreateIssueHandler,
+          CreateNoteOnIssuableHandler,
           UnsubscribeHandler,
           CreateMergeRequestHandler,
           ServiceDeskHandler

lib/gitlab/email/handler/create_note_on_issuable_handler.rb

+# frozen_string_literal: true
+
+require 'gitlab/email/handler/base_handler'
+
+# Handles comment creation emails when sent/forwarded by an authorized
+# user. Attachments are allowed. Quoted material is _not_ stripped, just like
+# create issue emails
+# Supports these formats:
+#   incoming+gitlab-org-gitlab-ce-20-Author_Token12345678-issue-34@incoming.gitlab.com
+module Gitlab
+  module Email
+    module Handler
+      class CreateNoteOnIssuableHandler < BaseHandler
+        include ReplyProcessing
+
+        attr_reader :issuable_iid
+
+        HANDLER_REGEX = /\A#{HANDLER_ACTION_BASE_REGEX}-(?<incoming_email_token>.+)-issue-(?<issuable_iid>\d+)\z/.freeze
+
+        def initialize(mail, mail_key)
+          super(mail, mail_key)
+
+          if (matched = HANDLER_REGEX.match(mail_key.to_s))
+            @project_slug         = matched[:project_slug]
+            @project_id           = matched[:project_id]&.to_i
+            @incoming_email_token = matched[:incoming_email_token]
+            @issuable_iid         = matched[:issuable_iid]&.to_i
+          end
+        end
+
+        def can_handle?
+          incoming_email_token && project_id && issuable_iid
+        end
+
+        def execute
+          raise ProjectNotFound unless project
+
+          validate_permission!(:create_note)
+
+          raise NoteableNotFoundError unless noteable
+          raise EmptyEmailError if message_including_reply.blank?
+
+          verify_record!(
+            record: create_note,
+            invalid_exception: InvalidNoteError,
+            record_name: 'comment')
+        end
+
+        def metrics_event
+          :receive_email_create_note_issuable
+        end
+
+        def noteable
+          return unless issuable_iid
+
+          @noteable ||= project&.issues&.find_by_iid(issuable_iid)
+        end
+
+        private
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def author
+          @author ||= User.find_by(incoming_email_token: incoming_email_token)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def create_note
+          Notes::CreateService.new(project, author, note_params).execute
+        end
+
+        def note_params
+          {
+            noteable_type: noteable.class.to_s,
+            noteable_id: noteable.id,
+            note: message_including_reply
+          }
+        end
+      end
+    end
+  end
+end

spec/fixtures/emails/valid_note_on_issuable.eml

+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-project_id-auth_token-issue-issue_iid@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-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+Subject: New Issue comment 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
+
+This should create a new comment on the issue.
+
+- Jake out
+
+> This quoted content will be included in the comment.

spec/graphql/types/issue_type_spec.rb

@@ -17,7 +17,8 @@ RSpec.describe GitlabSchema.types['Issue'] do
     fields = %i[id iid title description state reference author assignees updated_by participants labels milestone due_date
                 confidential discussion_locked upvotes downvotes user_notes_count user_discussions_count web_path web_url relative_position
                 emails_disabled subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status
-                design_collection alert_management_alert severity current_user_todos moved moved_to]
+                design_collection alert_management_alert severity current_user_todos moved moved_to
+                create_note_email]
 
     fields.each do |field_name|
       expect(described_class).to have_graphql_field(field_name)

spec/lib/gitlab/email/handler/create_note_handler_spec.rb

@@ -4,146 +4,50 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
   include_context :email_shared_context
-  let!(:sent_notification) do
-    SentNotification.record_note(note, user.id, mail_key)
-  end
+
+  let_it_be(:user)      { create(:user) }
+  let_it_be(:project)   { create(:project, :public, :repository) }
 
   let(:noteable)  { note.noteable }
   let(:note)      { create(:diff_note_on_merge_request, project: project) }
-  let(:user)      { create(:user) }
-  let(:project)   { create(:project, :public, :repository) }
   let(:email_raw) { fixture_file('emails/valid_reply.eml') }
+  let!(:sent_notification) do
+    SentNotification.record_note(note, user.id, mail_key)
+  end
 
   it_behaves_like :reply_processing_shared_examples
 
+  it_behaves_like :note_handler_shared_examples do
+    let(:recipient) { sent_notification.recipient }
+
+    let(:update_commands_only) { fixture_file('emails/update_commands_only_reply.eml')}
+    let(:no_content)           { fixture_file('emails/no_content_reply.eml') }
+    let(:commands_in_reply)    { fixture_file('emails/commands_in_reply.eml') }
+    let(:with_quick_actions)   { fixture_file('emails/valid_reply_with_quick_actions.eml') }
+  end
+
   before do
     stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo")
     stub_config_setting(host: 'localhost')
   end
 
-  context "when the recipient address doesn't include a mail key" do
-    let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") }
+  context 'when the recipient address does not include a mail key' do
+    let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, '') }
 
-    it "raises a UnknownIncomingEmail" do
+    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
+  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
+    it 'raises a SentNotificationNotFoundError' do
       expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError)
     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
-
-    context 'because the note was update commands only' do
-      let!(:email_raw) { fixture_file("emails/update_commands_only_reply.eml") }
-
-      context 'and current user cannot update noteable' do
-        it 'raises a CommandsOnlyNoteError' do
-          expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
-        end
-      end
-
-      context "and current user can update noteable" do
-        before do
-          project.add_developer(user)
-        end
-
-        it 'does not raise an error' do
-          expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1)
-
-          expect(noteable.reload).to be_closed
-        end
-      end
-    end
-  end
-
-  context 'when the note contains quick actions' do
-    let!(:email_raw) { fixture_file("emails/commands_in_reply.eml") }
-
-    context 'and current user cannot update the noteable' do
-      it 'only executes the commands that the user can perform' do
-        expect { receiver.execute }
-          .to change { noteable.notes.user.count }.by(1)
-          .and change { user.todos_pending_count }.from(0).to(1)
-
-        expect(noteable.reload).to be_open
-      end
-    end
-
-    context 'and current user can update noteable' do
-      before do
-        project.add_developer(user)
-      end
-
-      it 'posts a note and updates the noteable' do
-        expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
-
-        expect { receiver.execute }
-          .to change { noteable.notes.user.count }.by(1)
-          .and change { user.todos_pending_count }.from(0).to(1)
-
-        expect(noteable.reload).to be_closed
-      end
-    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
-
-  shared_examples "checks permissions on noteable" do
-    context "when user has access" do
-      before do
-        project.add_reporter(user)
-      end
-
-      it "creates a comment" do
-        expect { receiver.execute }.to change { noteable.notes.count }.by(1)
-      end
-    end
-
-    context "when user does not have access" do
-      it "raises UserNotAuthorizedError" do
-        expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
-      end
-    end
-  end
-
-  context "when discussion is locked" do
-    before do
-      noteable.update_attribute(:discussion_locked, true)
-    end
-
-    it_behaves_like "checks permissions on noteable"
-  end
-
-  context "when issue is confidential" do
+  context 'when issue is confidential' do
     let(:issue) { create(:issue, project: project) }
     let(:note) { create(:note, noteable: issue, project: project) }
 
@@ -151,17 +55,17 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
       issue.update_attribute(:confidential, true)
     end
 
-    it_behaves_like "checks permissions on noteable"
+    it_behaves_like :checks_permissions_on_noteable_examples
   end
 
   shared_examples 'a reply to existing comment' do
-    it "creates a comment" do
+    it 'creates a comment' do
       expect { receiver.execute }.to change { noteable.notes.count }.by(1)
       new_note = noteable.notes.last
 
       expect(new_note.author).to eq(sent_notification.recipient)
       expect(new_note.position).to eq(note.position)
-      expect(new_note.note).to include("I could not disagree more.")
+      expect(new_note.note).to include('I could not disagree more.')
       expect(new_note.in_reply_to?(note)).to be_truthy
 
       if note.part_of_discussion?
@@ -172,32 +76,14 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
     end
   end
 
-  context "when everything is fine" do
+  # additional shared tests in :reply_processing_shared_examples
+  context 'when everything is fine' do
     before do
       setup_attachment
     end
 
     it_behaves_like 'a reply to existing comment'
 
-    it "adds all attachments" do
-      expect_next_instance_of(Gitlab::Email::AttachmentUploader) do |uploader|
-        expect(uploader).to receive(:execute).with(upload_parent: project, uploader_class: FileUploader).and_return(
-          [
-            {
-              url: "uploads/image.png",
-              alt: "image",
-              markdown: markdown
-            }
-          ]
-        )
-      end
-
-      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)
@@ -228,75 +114,9 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
     end
   end
 
-  context "when note is not a discussion" do
+  context 'when note is not a discussion' do
     let(:note) { create(:note_on_merge_request, project: project) }
 
     it_behaves_like 'a reply to existing comment'
   end
-
-  context 'when the service desk' do
-    let(:project) { create(:project, :public, service_desk_enabled: true) }
-    let(:support_bot) { User.support_bot }
-    let(:noteable) { create(:issue, project: project, author: support_bot, title: 'service desk issue') }
-    let(:note) { create(:note, project: project, noteable: noteable) }
-    let(:email_raw) { fixture_file('emails/valid_reply_with_quick_actions.eml') }
-
-    let!(:sent_notification) do
-      SentNotification.record_note(note, support_bot.id, mail_key)
-    end
-
-    context 'is enabled' do
-      before do
-        allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
-        project.project_feature.update!(issues_access_level: issues_access_level)
-      end
-
-      context 'when issues are enabled for everyone' do
-        let(:issues_access_level) { ProjectFeature::ENABLED }
-
-        it 'creates a comment' do
-          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
-        end
-
-        context 'when quick actions are present' do
-          it 'encloses quick actions with code span markdown' do
-            receiver.execute
-            noteable.reload
-
-            note = Note.last
-            expect(note.note).to include("Jake out\n\n`/close`\n`/title test`")
-            expect(noteable.title).to eq('service desk issue')
-            expect(noteable).to be_opened
-          end
-        end
-      end
-
-      context 'when issues are protected members only' do
-        let(:issues_access_level) { ProjectFeature::PRIVATE }
-
-        it 'creates a comment' do
-          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
-        end
-      end
-
-      context 'when issues are disabled' do
-        let(:issues_access_level) { ProjectFeature::DISABLED }
-
-        it 'does not create a comment' do
-          expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
-        end
-      end
-    end
-
-    context 'is disabled' do
-      before do
-        allow(Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
-        allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(false)
-      end
-
-      it 'does not create a comment' do
-        expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
-      end
-    end
-  end
 end

spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Email::Handler::CreateNoteOnIssuableHandler do
+  include_context :email_shared_context
+
+  let_it_be(:user)      { create(:user, email: 'jake@adventuretime.ooo', incoming_email_token: 'auth_token') }
+  let_it_be(:namespace) { create(:namespace, path: 'gitlabhq') }
+  let_it_be(:project)   { create(:project, :public, namespace: namespace, path: 'gitlabhq') }
+
+  let!(:noteable) { create(:issue, project: project) }
+  let(:email_raw) { email_fixture('emails/valid_note_on_issuable.eml') }
+
+  before do
+    stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo")
+    stub_config_setting(host: 'localhost')
+  end
+
+  it_behaves_like :reply_processing_shared_examples
+
+  it_behaves_like :note_handler_shared_examples, true do
+    let_it_be(:recipient) { user }
+
+    let(:update_commands_only) { email_reply_fixture('emails/update_commands_only_reply.eml') }
+    let(:no_content)           { email_reply_fixture('emails/no_content_reply.eml') }
+    let(:commands_in_reply)    { email_reply_fixture('emails/commands_in_reply.eml') }
+    let(:with_quick_actions)   { email_reply_fixture('emails/valid_reply_with_quick_actions.eml') }
+  end
+
+  context 'when the recipient address does not include a mail key' do
+    let(:mail_key)  { 'gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid' }
+    let(:email_raw) { fixture_file('emails/valid_note_on_issuable.eml').gsub(mail_key, '') }
+
+    it 'raises an UnknownIncomingEmail' do
+      expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
+    end
+  end
+
+  context 'when issue is confidential' do
+    before do
+      noteable.update_attribute(:confidential, true)
+    end
+
+    it_behaves_like :checks_permissions_on_noteable_examples
+  end
+
+  def email_fixture(path)
+    fixture_file(path)
+      .gsub('project_id', project.project_id.to_s)
+      .gsub('issue_iid', noteable.iid.to_s)
+  end
+
+  def email_reply_fixture(path)
+    reply_address = 'reply+59d8df8370b7e95c5a49fbf86aeb2c93'
+    note_address  = "incoming+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-issue-#{noteable.iid}"
+
+    fixture_file(path)
+      .gsub(reply_address, note_address)
+  end
+end

spec/lib/gitlab/email/handler_spec.rb

@@ -60,8 +60,9 @@ RSpec.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-123-user_email_token-merge-request) +
-        %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY} sent_notification_key path-to-project-123-user_email_token-issue) +
+      %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}) +
+        %w(sent_notification_key path-to-project-123-user_email_token-merge-request) +
+        %w(path-to-project-123-user_email_token-issue path-to-project-123-user_email_token-issue-123) +
         %w(path/to/project+user_email_token path/to/project+merge-request+user_email_token some/project)
     end
 

spec/models/concerns/noteable_spec.rb

@@ -25,8 +25,8 @@ RSpec.describe Noteable do
 
   let(:active_position2) do
     Gitlab::Diff::Position.new(
-      old_path: "files/ruby/popen.rb",
-      new_path: "files/ruby/popen.rb",
+      old_path: 'files/ruby/popen.rb',
+      new_path: 'files/ruby/popen.rb',
       old_line: 16,
       new_line: 22,
       diff_refs: subject.diff_refs
@@ -35,11 +35,11 @@ RSpec.describe Noteable do
 
   let(:outdated_position) do
     Gitlab::Diff::Position.new(
-      old_path: "files/ruby/popen.rb",
-      new_path: "files/ruby/popen.rb",
+      old_path: 'files/ruby/popen.rb',
+      new_path: 'files/ruby/popen.rb',
       old_line: nil,
       new_line: 9,
-      diff_refs: project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs
+      diff_refs: project.commit('874797c3a73b60d2187ed6e2fcabd289ff75171e').diff_refs
     )
   end
 
@@ -80,7 +80,7 @@ RSpec.describe Noteable do
   describe '#grouped_diff_discussions' do
     let(:grouped_diff_discussions) { subject.grouped_diff_discussions }
 
-    it "includes active discussions" do
+    it 'includes active discussions' do
       discussions = grouped_diff_discussions.values.flatten
 
       expect(discussions.count).to eq(2)
@@ -91,17 +91,17 @@ RSpec.describe Noteable do
       expect(discussions.last.notes).to eq([active_diff_note3])
     end
 
-    it "doesn't include outdated discussions" do
+    it 'does not include outdated discussions' do
       expect(grouped_diff_discussions.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
     end
 
-    it "groups the discussions by line code" do
+    it 'groups the discussions by line code' do
       expect(grouped_diff_discussions[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id)
       expect(grouped_diff_discussions[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id)
     end
   end
 
-  context "discussion status" do
+  context 'discussion status' do
     let(:first_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
     let(:second_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
     let(:third_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
@@ -110,56 +110,56 @@ RSpec.describe Noteable do
       allow(subject).to receive(:resolvable_discussions).and_return([first_discussion, second_discussion, third_discussion])
     end
 
-    describe "#discussions_resolvable?" do
-      context "when all discussions are unresolvable" do
+    describe '#discussions_resolvable?' do
+      context 'when all discussions are unresolvable' do
         before do
           allow(first_discussion).to receive(:resolvable?).and_return(false)
           allow(second_discussion).to receive(:resolvable?).and_return(false)
           allow(third_discussion).to receive(:resolvable?).and_return(false)
         end
 
-        it "returns false" do
+        it 'returns false' do
           expect(subject.discussions_resolvable?).to be false
         end
       end
 
-      context "when some discussions are unresolvable and some discussions are resolvable" do
+      context 'when some discussions are unresolvable and some discussions are resolvable' do
         before do
           allow(first_discussion).to receive(:resolvable?).and_return(true)
           allow(second_discussion).to receive(:resolvable?).and_return(false)
           allow(third_discussion).to receive(:resolvable?).and_return(true)
         end
 
-        it "returns true" do
+        it 'returns true' do
           expect(subject.discussions_resolvable?).to be true
         end
       end
 
-      context "when all discussions are resolvable" do
+      context 'when all discussions are resolvable' do
         before do
           allow(first_discussion).to receive(:resolvable?).and_return(true)
           allow(second_discussion).to receive(:resolvable?).and_return(true)
           allow(third_discussion).to receive(:resolvable?).and_return(true)
         end
 
-        it "returns true" do
+        it 'returns true' do
           expect(subject.discussions_resolvable?).to be true
         end
       end
     end
 
-    describe "#discussions_resolved?" do
-      context "when discussions are not resolvable" do
+    describe '#discussions_resolved?' do
+      context 'when discussions are not resolvable' do
         before do
           allow(subject).to receive(:discussions_resolvable?).and_return(false)
         end
 
-        it "returns false" do
+        it 'returns false' do
           expect(subject.discussions_resolved?).to be false
         end
       end
 
-      context "when discussions are resolvable" do
+      context 'when discussions are resolvable' do
         before do
           allow(subject).to receive(:discussions_resolvable?).and_return(true)
 
@@ -168,31 +168,31 @@ RSpec.describe Noteable do
           allow(third_discussion).to receive(:resolvable?).and_return(true)
         end
 
-        context "when all resolvable discussions are resolved" do
+        context 'when all resolvable discussions are resolved' do
           before do
             allow(first_discussion).to receive(:resolved?).and_return(true)
             allow(third_discussion).to receive(:resolved?).and_return(true)
           end
 
-          it "returns true" do
+          it 'returns true' do
             expect(subject.discussions_resolved?).to be true
           end
         end
 
-        context "when some resolvable discussions are not resolved" do
+        context 'when some resolvable discussions are not resolved' do
           before do
             allow(first_discussion).to receive(:resolved?).and_return(true)
             allow(third_discussion).to receive(:resolved?).and_return(false)
           end
 
-          it "returns false" do
+          it 'returns false' do
             expect(subject.discussions_resolved?).to be false
           end
         end
       end
     end
 
-    describe "#discussions_to_be_resolved" do
+    describe '#discussions_to_be_resolved' do
       before do
         allow(first_discussion).to receive(:to_be_resolved?).and_return(true)
         allow(second_discussion).to receive(:to_be_resolved?).and_return(false)
@@ -245,6 +245,12 @@ RSpec.describe Noteable do
     end
   end
 
+  describe '.email_creatable_types' do
+    it 'exposes the email creatable types' do
+      expect(described_class.email_creatable_types).to include('Issue')
+    end
+  end
+
   describe '#capped_notes_count' do
     context 'notes number < 10' do
       it 'the number of notes is returned' do
@@ -263,13 +269,13 @@ RSpec.describe Noteable do
     end
   end
 
-  describe "#has_any_diff_note_positions?" do
-    let(:source_branch) { "compare-with-merge-head-source" }
-    let(:target_branch) { "compare-with-merge-head-target" }
+  describe '#has_any_diff_note_positions?' do
+    let(:source_branch) { 'compare-with-merge-head-source' }
+    let(:target_branch) { 'compare-with-merge-head-target' }
     let(:merge_request) { create(:merge_request, source_branch: source_branch, target_branch: target_branch) }
 
     let!(:note) do
-      path = "files/markdown/ruby-style-guide.md"
+      path = 'files/markdown/ruby-style-guide.md'
 
       position = Gitlab::Diff::Position.new(
         old_path: path,
@@ -286,20 +292,54 @@ RSpec.describe Noteable do
       Discussions::CaptureDiffNotePositionsService.new(merge_request).execute
     end
 
-    it "returns true when it has diff note positions" do
+    it 'returns true when it has diff note positions' do
       expect(merge_request.has_any_diff_note_positions?).to be(true)
     end
 
-    it "returns false when it has notes but no diff note positions" do
+    it 'returns false when it has notes but no diff note positions' do
       DiffNotePosition.where(note: note).find_each(&:delete)
 
       expect(merge_request.has_any_diff_note_positions?).to be(false)
     end
 
-    it "returns false when it has no notes" do
+    it 'returns false when it has no notes' do
       merge_request.notes.find_each(&:destroy)
 
       expect(merge_request.has_any_diff_note_positions?).to be(false)
     end
   end
+
+  describe '#creatable_note_email_address' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:source_branch) { 'compare-with-merge-head-source' }
+
+    let(:issue) { create(:issue, project: project) }
+    let(:snippet) { build(:snippet) }
+
+    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 note' do
+        address = "p+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-issue-#{issue.iid}@gl.ab"
+
+        expect(issue.creatable_note_email_address(user)).to eq(address)
+      end
+
+      it 'returns nil for unsupported types' do
+        expect(snippet.creatable_note_email_address(user)).to be_nil
+      end
+    end
+
+    context 'incoming email disabled' do
+      before do
+        stub_incoming_email_setting(enabled: false)
+      end
+
+      it 'returns nil' do
+        expect(issue.creatable_note_email_address(user)).to be_nil
+      end
+    end
+  end
 end

spec/support/shared_contexts/email_shared_context.rb

 # frozen_string_literal: true
 
 RSpec.shared_context :email_shared_context do
-  let(:mail_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
+  let(:mail_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' }
   let(:receiver) { Gitlab::Email::Receiver.new(email_raw) }
-  let(:markdown) { "![image](uploads/image.png)" }
+  let(:markdown) { '![image](uploads/image.png)' }
 
   def setup_attachment
     allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return(
       [
         {
-          url: "uploads/image.png",
-          alt: "image",
+          url: 'uploads/image.png',
+          alt: 'image',
           markdown: markdown
         }
       ]
@@ -19,23 +19,252 @@ RSpec.shared_context :email_shared_context do
 end
 
 RSpec.shared_examples :reply_processing_shared_examples do
-  context "when the user could not be found" do
+  context 'when the user could not be found' do
     before do
       user.destroy!
     end
 
-    it "raises a UserNotFoundError" do
+    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
+  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
+    it 'raises a ProjectNotFound' do
       expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
     end
   end
 end
+
+RSpec.shared_examples :checks_permissions_on_noteable_examples do
+  context 'when user has access' do
+    before do
+      project.add_reporter(user)
+    end
+
+    it 'creates a comment' do
+      expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+    end
+  end
+
+  context 'when user does not have access' do
+    it 'raises UserNotAuthorizedError' do
+      expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
+    end
+  end
+end
+
+RSpec.shared_examples :note_handler_shared_examples do |forwardable|
+  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
+
+    context 'because the note was update commands only' do
+      let!(:email_raw) { update_commands_only }
+
+      context 'and current user cannot update noteable' do
+        it 'raises a CommandsOnlyNoteError' do
+          expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
+        end
+      end
+
+      context 'and current user can update noteable' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'does not raise an error', unless: forwardable do
+          expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1)
+
+          expect(noteable.reload).to be_closed
+        end
+
+        it 'raises an InvalidNoteError', if: forwardable do
+          expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
+        end
+      end
+    end
+  end
+
+  context 'when the note contains quick actions' do
+    let!(:email_raw) { commands_in_reply }
+
+    context 'and current user cannot update the noteable' do
+      it 'only executes the commands that the user can perform' do
+        expect { receiver.execute }
+          .to change { noteable.notes.user.count }.by(1)
+                .and change { user.todos_pending_count }.from(0).to(1)
+
+        expect(noteable.reload).to be_open
+      end
+    end
+
+    context 'and current user can update noteable' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'posts a note and updates the noteable' do
+        expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
+
+        expect { receiver.execute }
+          .to change { noteable.notes.user.count }.by(1)
+                .and change { user.todos_pending_count }.from(0).to(1)
+
+        expect(noteable.reload).to be_closed
+      end
+    end
+  end
+
+  context 'when the reply is blank' do
+    let!(:email_raw) { no_content }
+
+    it 'raises an EmptyEmailError', unless: forwardable do
+      expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
+    end
+
+    it 'allows email to only have quoted text', if: forwardable do
+      expect { receiver.execute }.not_to raise_error(Gitlab::Email::EmptyEmailError)
+    end
+  end
+
+  context 'when discussion is locked' do
+    before do
+      noteable.update_attribute(:discussion_locked, true)
+    end
+
+    it_behaves_like :checks_permissions_on_noteable_examples
+  end
+
+  context 'when everything is fine' do
+    before do
+      setup_attachment
+    end
+
+    it 'adds all attachments' do
+      expect_next_instance_of(Gitlab::Email::AttachmentUploader) do |uploader|
+        expect(uploader).to receive(:execute).with(upload_parent: project, uploader_class: FileUploader).and_return(
+          [
+            {
+              url: 'uploads/image.png',
+              alt: 'image',
+              markdown: markdown
+            }
+          ]
+        )
+      end
+
+      receiver.execute
+
+      note = noteable.notes.last
+      expect(note.note).to include(markdown)
+      expect(note.note).to include('Jake out')
+    end
+  end
+
+  context 'when the service desk' do
+    let(:project) { create(:project, :public, service_desk_enabled: true) }
+    let(:support_bot) { User.support_bot }
+    let(:noteable) { create(:issue, project: project, author: support_bot, title: 'service desk issue') }
+    let!(:note) { create(:note, project: project, noteable: noteable) }
+    let(:email_raw) { with_quick_actions }
+
+    let!(:sent_notification) do
+      SentNotification.record_note(note, support_bot.id, mail_key)
+    end
+
+    context 'is enabled' do
+      before do
+        allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
+        project.project_feature.update!(issues_access_level: issues_access_level)
+      end
+
+      context 'when issues are enabled for everyone' do
+        let(:issues_access_level) { ProjectFeature::ENABLED }
+
+        it 'creates a comment' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+        end
+
+        context 'when quick actions are present' do
+          before do
+            receiver.execute
+            noteable.reload
+          end
+
+          context 'when author is a support_bot', unless: forwardable do
+            it 'encloses quick actions with code span markdown' do
+              note = Note.last
+              expect(note.note).to include("Jake out\n\n`/close`\n`/title test`")
+              expect(noteable.title).to eq('service desk issue')
+              expect(noteable).to be_opened
+            end
+          end
+
+          context 'when author is a normal user', if: forwardable do
+            it 'extracted the quick actions' do
+              note = Note.last
+              expect(note.note).to include('Jake out')
+              expect(note.note).not_to include("`/close`\n`/title test`")
+            end
+          end
+        end
+      end
+
+      context 'when issues are protected members only' do
+        let(:issues_access_level) { ProjectFeature::PRIVATE }
+
+        before do
+          if recipient.support_bot?
+            @changed_by = 1
+          else
+            @changed_by = 2
+            project.add_developer(recipient)
+          end
+        end
+
+        it 'creates a comment' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(@changed_by)
+        end
+      end
+
+      context 'when issues are disabled' do
+        let(:issues_access_level) { ProjectFeature::DISABLED }
+
+        it 'does not create a comment' do
+          expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
+        end
+      end
+    end
+
+    context 'is disabled', unless: forwardable do
+      before do
+        allow(Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
+        allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(false)
+      end
+
+      it 'does not create a comment' do
+        expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+      end
+    end
+  end
+end