Merge branch '333916-service-desk-no-suffix' into 'master'

Allow custom support email without configuring custom project suffix

See merge request gitlab-org/gitlab!71940

app/models/project.rb

@@ -2582,18 +2582,21 @@ class Project < ApplicationRecord
     config = Gitlab.config.incoming_email
     wildcard = Gitlab::IncomingEmail::WILDCARD_PLACEHOLDER
 
-    config.address&.gsub(wildcard, "#{full_path_slug}-#{id}-issue-")
+    config.address&.gsub(wildcard, "#{full_path_slug}-#{default_service_desk_suffix}")
   end
 
   def service_desk_custom_address
     return unless Gitlab::ServiceDeskEmail.enabled?
 
-    key = service_desk_setting&.project_key
-    return unless key.present?
+    key = service_desk_setting&.project_key || default_service_desk_suffix
 
     Gitlab::ServiceDeskEmail.address_for_key("#{full_path_slug}-#{key}")
   end
 
+  def default_service_desk_suffix
+    "#{id}-issue-"
+  end
+
   def root_namespace
     if namespace.has_parent?
       namespace.root_ancestor

doc/user/project/service_desk.md

@@ -166,13 +166,13 @@ To edit the custom email display name:
 > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/284656) in GitLab 13.8.
 
 It is possible to customize the email address used by Service Desk. To do this, you must configure
-both a [custom mailbox](#configuring-a-custom-mailbox) and a
+a [custom mailbox](#configuring-a-custom-mailbox). If you want you can also configure a
 [custom suffix](#configuring-a-custom-email-address-suffix).
 
 #### Configuring a custom mailbox
 
 NOTE:
-On GitLab.com a custom mailbox is already configured with `contact-project+%{key}@incoming.gitlab.com` as the email address, so you only have to configure the
+On GitLab.com a custom mailbox is already configured with `contact-project+%{key}@incoming.gitlab.com` as the email address, you can still configure the
 [custom suffix](#configuring-a-custom-email-address-suffix) in project settings.
 
 Using the `service_desk_email` configuration, you can customize the mailbox
@@ -271,6 +271,8 @@ For example, suppose the `mygroup/myproject` project Service Desk settings has t
 The Service Desk email address for this project is: `contact+mygroup-myproject-support@example.com`.
 The [incoming email](../../administration/incoming_email.md) address still works.
 
+If you don't configure the custom suffix, the default project identification will be used for identifying the project. You can see that email address in the project settings.
+
 ## Using Service Desk
 
 You can use Service Desk to [create an issue](#as-an-end-user-issue-creator) or [respond to one](#as-a-responder-to-the-issue).

lib/gitlab/email/handler/service_desk_handler.rb

@@ -15,16 +15,14 @@ module Gitlab
         PROJECT_KEY_PATTERN  = /\A(?<slug>.+)-(?<key>[a-z0-9_]+)\z/.freeze
 
         def initialize(mail, mail_key, service_desk_key: nil)
-          super(mail, mail_key)
-
-          if service_desk_key.present?
+          if service_desk_key
+            mail_key ||= service_desk_key
             @service_desk_key = service_desk_key
-          elsif !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
+
+          super(mail, mail_key)
+
+          match_project_slug || match_legacy_project_slug
         end
 
         def can_handle?
@@ -42,15 +40,29 @@ module Gitlab
           end
         end
 
+        def match_project_slug
+          return if mail_key&.include?('/')
+          return unless matched = HANDLER_REGEX.match(mail_key.to_s)
+
+          @project_slug = matched[:project_slug]
+          @project_id   = matched[:project_id]&.to_i
+        end
+
+        def match_legacy_project_slug
+          return unless matched = HANDLER_REGEX_LEGACY.match(mail_key.to_s)
+
+          @project_path = matched[:project_path]
+        end
+
         def metrics_event
           :receive_email_service_desk
         end
 
         def project
           strong_memoize(:project) do
-            @project = service_desk_key ? project_from_key : super
-            @project = nil unless @project&.service_desk_enabled?
-            @project
+            project_record = super
+            project_record ||= project_from_key if service_desk_key
+            project_record&.service_desk_enabled? ? project_record : nil
           end
         end
 

spec/features/projects/settings/service_desk_setting_spec.rb

@@ -54,7 +54,7 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache do
       wait_for_requests
 
       project.reload
-      expect(find('[data-testid="incoming-email"]').value).to eq(project.service_desk_incoming_address)
+      expect(find('[data-testid="incoming-email"]').value).to eq(project.service_desk_custom_address)
 
       page.within '#js-service-desk' do
         fill_in('service-desk-project-suffix', with: 'foo')

spec/fixtures/emails/service_desk_custom_address_no_key.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: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <support+project_slug-project_key@example.com>; 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: support+email-test-project_id-issue-@example.com
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+Subject: The message subject! @all
+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
+
+Service desk stuff!
+
+```
+a = b
+```
+
+/label ~label1
+/assign @user1
+/close

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

@@ -196,51 +196,64 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
         end
       end
 
-      context 'when using service desk key' do
-        let_it_be(:service_desk_key) { 'mykey' }
-
-        let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml') }
+      context 'when using custom service desk address' do
         let(:receiver) { Gitlab::Email::ServiceDeskReceiver.new(email_raw) }
 
         before do
           stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com')
         end
 
-        before_all do
-          create(:service_desk_setting, project: project, project_key: service_desk_key)
-        end
+        context 'when using project key' do
+          let_it_be(:service_desk_key) { 'mykey' }
 
-        it_behaves_like 'a new issue request'
+          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml') }
+
+          before_all do
+            create(:service_desk_setting, project: project, project_key: service_desk_key)
+          end
 
-        context 'when there is no project with the key' do
-          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', key: 'some_key') }
+          it_behaves_like 'a new issue request'
 
-          it 'bounces the email' do
-            expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+          context 'when there is no project with the key' do
+            let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', key: 'some_key') }
+
+            it 'bounces the email' do
+              expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+            end
           end
-        end
 
-        context 'when the project slug does not match' do
-          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', slug: 'some-slug') }
+          context 'when the project slug does not match' do
+            let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', slug: 'some-slug') }
+
+            it 'bounces the email' do
+              expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+            end
+          end
+
+          context 'when there are multiple projects with same key' do
+            let_it_be(:project_with_same_key) { create(:project, group: group, service_desk_enabled: true) }
 
-          it 'bounces the email' do
-            expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+            let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', slug: project_with_same_key.full_path_slug.to_s) }
+
+            before do
+              create(:service_desk_setting, project: project_with_same_key, project_key: service_desk_key)
+            end
+
+            it 'process email for project with matching slug' do
+              expect { receiver.execute }.to change { Issue.count }.by(1)
+              expect(Issue.last.project).to eq(project_with_same_key)
+            end
           end
         end
 
-        context 'when there are multiple projects with same key' do
-          let_it_be(:project_with_same_key) { create(:project, group: group, service_desk_enabled: true) }
-
-          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', slug: project_with_same_key.full_path_slug.to_s) }
+        context 'when project key is not set' do
+          let(:email_raw) { email_fixture('emails/service_desk_custom_address_no_key.eml') }
 
           before do
-            create(:service_desk_setting, project: project_with_same_key, project_key: service_desk_key)
+            stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com')
           end
 
-          it 'process email for project with matching slug' do
-            expect { receiver.execute }.to change { Issue.count }.by(1)
-            expect(Issue.last.project).to eq(project_with_same_key)
-          end
+          it_behaves_like 'a new issue request'
         end
       end
 

spec/models/project_spec.rb

@@ -1715,13 +1715,19 @@ RSpec.describe Project, factory_default: :keep do
         allow(::Gitlab::ServiceDeskEmail).to receive(:config).and_return(config)
       end
 
-      it 'returns custom address when project_key is set' do
-        create(:service_desk_setting, project: project, project_key: 'key1')
+      context 'when project_key is set' do
+        it 'returns custom address including the project_key' do
+          create(:service_desk_setting, project: project, project_key: 'key1')
 
-        expect(subject).to eq("foo+#{project.full_path_slug}-key1@bar.com")
+          expect(subject).to eq("foo+#{project.full_path_slug}-key1@bar.com")
+        end
       end
 
-      it_behaves_like 'with incoming email address'
+      context 'when project_key is not set' do
+        it 'returns custom address including the project full path' do
+          expect(subject).to eq("foo+#{project.full_path_slug}-#{project.project_id}-issue-@bar.com")
+        end
+      end
     end
   end