Commit cf9290ca authored by Felipe Artur's avatar Felipe Artur Committed by Mikołaj Wawrzyniak

Process emails for projects with not unique service desk keys

Prevent errors with emails sent to projects
with not unique service desk keys.

Changelog: fixed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62139
parent b40d1ce7
...@@ -636,6 +636,12 @@ class Project < ApplicationRecord ...@@ -636,6 +636,12 @@ class Project < ApplicationRecord
scope :with_tracing_enabled, -> { joins(:tracing_setting) } scope :with_tracing_enabled, -> { joins(:tracing_setting) }
scope :with_enabled_error_tracking, -> { joins(:error_tracking_setting).where(project_error_tracking_settings: { enabled: true }) } scope :with_enabled_error_tracking, -> { joins(:error_tracking_setting).where(project_error_tracking_settings: { enabled: true }) }
scope :with_service_desk_key, -> (key) do
# project_key is not indexed for now
# see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24063#note_282435524 for details
joins(:service_desk_setting).where('service_desk_settings.project_key' => key)
end
enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
chronic_duration_attr :build_timeout_human_readable, :build_timeout, chronic_duration_attr :build_timeout_human_readable, :build_timeout,
...@@ -837,12 +843,6 @@ class Project < ApplicationRecord ...@@ -837,12 +843,6 @@ class Project < ApplicationRecord
from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id) from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id)
end end
def find_by_service_desk_project_key(key)
# project_key is not indexed for now
# see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24063#note_282435524 for details
joins(:service_desk_setting).find_by('service_desk_settings.project_key' => key)
end
end end
def initialize(attributes = nil) def initialize(attributes = nil)
......
...@@ -65,10 +65,9 @@ module Gitlab ...@@ -65,10 +65,9 @@ module Gitlab
def project_from_key def project_from_key
return unless match = service_desk_key.match(PROJECT_KEY_PATTERN) return unless match = service_desk_key.match(PROJECT_KEY_PATTERN)
project = Project.find_by_service_desk_project_key(match[:key]) Project.with_service_desk_key(match[:key]).find do |project|
return unless valid_project_key?(project, match[:slug]) valid_project_key?(project, match[:slug])
end
project
end end
def valid_project_key?(project, slug) def valid_project_key?(project, slug)
......
...@@ -168,7 +168,7 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do ...@@ -168,7 +168,7 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
end end
context 'when using service desk key' do context 'when using service desk key' do
let_it_be(:service_desk_settings) { create(:service_desk_setting, project: project, project_key: 'mykey') } let_it_be(:service_desk_key) { 'mykey' }
let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml') } let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml') }
let(:receiver) { Gitlab::Email::ServiceDeskReceiver.new(email_raw) } let(:receiver) { Gitlab::Email::ServiceDeskReceiver.new(email_raw) }
...@@ -176,6 +176,10 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do ...@@ -176,6 +176,10 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com') stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com')
end end
before_all do
create(:service_desk_setting, project: project, project_key: service_desk_key)
end
it_behaves_like 'a new issue request' it_behaves_like 'a new issue request'
context 'when there is no project with the key' do context 'when there is no project with the key' do
...@@ -193,6 +197,20 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do ...@@ -193,6 +197,20 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound) expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
end 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) }
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 end
end end
......
...@@ -1584,19 +1584,20 @@ RSpec.describe Project, factory_default: :keep do ...@@ -1584,19 +1584,20 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '.find_by_service_desk_project_key' do describe '.with_service_desk_key' do
it 'returns the correct project' do it 'returns projects with given key' do
project1 = create(:project) project1 = create(:project)
project2 = create(:project) project2 = create(:project)
create(:service_desk_setting, project: project1, project_key: 'key1') create(:service_desk_setting, project: project1, project_key: 'key1')
create(:service_desk_setting, project: project2, project_key: 'key2') create(:service_desk_setting, project: project2, project_key: 'key1')
create(:service_desk_setting, project_key: 'key2')
create(:service_desk_setting)
expect(Project.find_by_service_desk_project_key('key1')).to eq(project1) expect(Project.with_service_desk_key('key1')).to contain_exactly(project1, project2)
expect(Project.find_by_service_desk_project_key('key2')).to eq(project2)
end end
it 'returns nil if there is no project with the key' do it 'returns empty if there is no project with the key' do
expect(Project.find_by_service_desk_project_key('some_key')).to be_nil expect(Project.with_service_desk_key('key1')).to be_empty
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment