EE Refactoring and review comments

targeting the service desk email handler

ee/app/models/ee/project.rb

@@ -213,7 +213,7 @@ module EE
       config = ::Gitlab.config.incoming_email
       wildcard = ::Gitlab::IncomingEmail::WILDCARD_PLACEHOLDER
 
-      config.address&.gsub(wildcard, "#{full_path_slug}-#{id}-")
+      config.address&.gsub(wildcard, "#{full_path_slug}-#{id}-issue-")
     end
 
     override :add_import_job

ee/lib/gitlab/email/handler/ee/service_desk_handler.rb

 # frozen_string_literal: true
 
 # handles service desk issue creation emails with these formats:
-#   incoming+gitlab-org-gitlab-ce-20-@incoming.gitlab.com
+#   incoming+gitlab-org-gitlab-ce-20-issue-@incoming.gitlab.com
 #   incoming+gitlab-org/gitlab-ce@incoming.gitlab.com (legacy)
 module Gitlab
   module Email
@@ -10,14 +10,15 @@ module Gitlab
         class ServiceDeskHandler < BaseHandler
           include ReplyProcessing
 
-          HANDLER_REGEX        = /\A.+-(?<project_id>.+)-\z/.freeze
+          HANDLER_REGEX        = /\A#{::Gitlab::Email::Handler::BaseHandler::HANDLER_ACTION_BASE_REGEX}-issue-\z/.freeze
           HANDLER_REGEX_LEGACY = /\A(?<project_path>[^\+]*)\z/.freeze
 
           def initialize(mail, mail_key)
             super(mail, mail_key)
 
-            if matched = HANDLER_REGEX.match(mail_key.to_s)
-              @project_id = matched[:project_id]
+            if !mail_key&.include?('/') && (matched = HANDLER_REGEX.match(mail_key.to_s))
+              @project_slug = matched[:project_slug]
+              @project_id   = matched[:project_id]&.to_i
             elsif matched = HANDLER_REGEX_LEGACY.match(mail_key.to_s)
               @project_path = matched[:project_path]
             end
@@ -42,20 +43,12 @@ module Gitlab
 
           attr_reader :project_id, :project_path
 
-          # rubocop: disable CodeReuse/ActiveRecord
           def project
-            return @project if instance_variable_defined?(:@project)
+            super
 
-            found_project = Project.where(service_desk_enabled: true)
-            found_project = if project_id
-                              found_project.find_by_id(project_id)
-                            else
-                              found_project.find_by_full_path(project_path)
-                            end
-
-            @project = found_project&.service_desk_enabled? ? found_project : nil
+            @project = nil unless @project&.service_desk_enabled?
+            @project
           end
-          # rubocop: enable CodeReuse/ActiveRecord
 
           def create_issue!
             # NB: the support bot is specifically forbidden

ee/spec/fixtures/emails/service_desk.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-@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+email-test-project_id-issue-@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+email-test-project_id-@appmail.adventuretime.ooo
+To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 Subject: The message subject! @all
 Mime-Version: 1.0

ee/spec/fixtures/emails/service_desk_forwarded.eml

-Delivered-To: incoming+email-test-project_id-@appmail.adventuretime.ooo
+Delivered-To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo
 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-@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+email-test-project_id-issue-@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>

ee/spec/fixtures/emails/service_desk_sender_and_from.eml

-Delivered-To: incoming+email-test-project_id-@appmail.adventuretime.ooo
+Delivered-To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo
 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-@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+email-test-project_id-issue-@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: Finn the Human <finn@adventuretime.ooo>

ee/spec/lib/gitlab/email/handler/ee/service_desk_handler_spec.rb

@@ -55,7 +55,7 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
       let(:mail) { Mail::Message.new(email_raw) }
 
       it "matches the new format" do
-        handler = described_class.new(mail, "h5bp-html5-boilerplate-#{project.project_id}-")
+        handler = described_class.new(mail, "h5bp-html5-boilerplate-#{project.project_id}-issue-")
 
         expect(handler.instance_variable_get(:@project_id).to_i).to eq project.project_id
         expect(handler.can_handle?).to be_truthy

ee/spec/models/project_spec.rb

@@ -702,7 +702,7 @@ describe Project do
     end
 
     it 'uses project full path as service desk address key' do
-      expect(project.service_desk_address).to eq("test+#{project.full_path_slug}-#{project.project_id}-@mail.com")
+      expect(project.service_desk_address).to eq("test+#{project.full_path_slug}-#{project.project_id}-issue-@mail.com")
     end
   end
 

lib/gitlab/email/handler.rb

@@ -5,8 +5,6 @@ module Gitlab
     module Handler
       prepend ::EE::Gitlab::Email::Handler
 
-      HANDLER_ACTION_BASE_REGEX = /(?<project_slug>.+)-(?<project_id>\d+)-(?<incoming_email_token>.+)/.freeze
-
       def self.handlers
         @handlers ||= load_handlers
       end

lib/gitlab/email/handler/base_handler.rb

@@ -6,6 +6,8 @@ module Gitlab
       class BaseHandler
         attr_reader :mail, :mail_key
 
+        HANDLER_ACTION_BASE_REGEX = /(?<project_slug>.+)-(?<project_id>\d+)/.freeze
+
         def initialize(mail, mail_key)
           @mail = mail
           @mail_key = mail_key

lib/gitlab/email/handler/create_issue_handler.rb

@@ -11,7 +11,7 @@ module Gitlab
       class CreateIssueHandler < BaseHandler
         include ReplyProcessing
 
-        HANDLER_REGEX        = /\A#{Gitlab::Email::Handler::HANDLER_ACTION_BASE_REGEX}-issue\z/.freeze
+        HANDLER_REGEX        = /\A#{Gitlab::Email::Handler::BaseHandler::HANDLER_ACTION_BASE_REGEX}-(?<incoming_email_token>.+)-issue\z/.freeze
         HANDLER_REGEX_LEGACY = /\A(?<project_path>[^\+]*)\+(?<incoming_email_token>.*)\z/.freeze
 
         def initialize(mail, mail_key)

lib/gitlab/email/handler/create_merge_request_handler.rb

@@ -12,7 +12,7 @@ module Gitlab
       class CreateMergeRequestHandler < BaseHandler
         include ReplyProcessing
 
-        HANDLER_REGEX        = /\A#{Gitlab::Email::Handler::HANDLER_ACTION_BASE_REGEX}-merge-request\z/.freeze
+        HANDLER_REGEX        = /\A#{Gitlab::Email::Handler::BaseHandler::HANDLER_ACTION_BASE_REGEX}-(?<incoming_email_token>.+)-merge-request\z/.freeze
         HANDLER_REGEX_LEGACY = /\A(?<project_path>[^\+]*)\+merge-request\+(?<incoming_email_token>.*)/.freeze
 
         def initialize(mail, mail_key)