From a96cd7cb2bd3cfcafc30a349bdd9289f9db055a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= Date: Thu, 13 Dec 2018 11:15:48 +0000 Subject: Add List-Id to notification emails --- app/helpers/emails_helper.rb | 25 ++++++++++ app/mailers/notify.rb | 2 + .../unreleased/53493-list-id-email-header.yml | 5 ++ doc/workflow/notifications.md | 1 + spec/helpers/emails_helper_spec.rb | 55 ++++++++++++++++++++++ .../shared_examples/notify_shared_examples.rb | 4 +- 6 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/53493-list-id-email-header.yml diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index 2d2e89a2a50..e4c46ceeaa2 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -98,4 +98,29 @@ module EmailsHelper "#{string} on #{Gitlab.config.gitlab.host}" end + + def create_list_id_string(project, list_id_max_length = 255) + project_path_as_domain = project.full_path.downcase + .split('/').reverse.join('/') + .gsub(%r{[^a-z0-9\/]}, '-') + .gsub(%r{\/+}, '.') + .gsub(/(\A\.+|\.+\z)/, '') + + max_domain_length = list_id_max_length - Gitlab.config.gitlab.host.length - project.id.to_s.length - 2 + + if max_domain_length < 3 + return project.id.to_s + "..." + Gitlab.config.gitlab.host + end + + if project_path_as_domain.length > max_domain_length + project_path_as_domain = project_path_as_domain.slice(0, max_domain_length) + + last_dot_index = project_path_as_domain[0..-2].rindex(".") + last_dot_index ||= max_domain_length - 2 + + project_path_as_domain = project_path_as_domain.slice(0, last_dot_index).concat("..") + end + + project.id.to_s + "." + project_path_as_domain + "." + Gitlab.config.gitlab.host + end end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 49a3920e43f..15710bee4d4 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -3,6 +3,7 @@ class Notify < BaseMailer include ActionDispatch::Routing::PolymorphicRoutes include GitlabRoutingHelper + include EmailsHelper include Emails::Issues include Emails::MergeRequests @@ -194,6 +195,7 @@ class Notify < BaseMailer headers['X-GitLab-Project'] = @project.name headers['X-GitLab-Project-Id'] = @project.id headers['X-GitLab-Project-Path'] = @project.full_path + headers['List-Id'] = "#{@project.full_path} <#{create_list_id_string(@project)}>" end def add_unsubscription_headers_and_links diff --git a/changelogs/unreleased/53493-list-id-email-header.yml b/changelogs/unreleased/53493-list-id-email-header.yml new file mode 100644 index 00000000000..09a0639f6f5 --- /dev/null +++ b/changelogs/unreleased/53493-list-id-email-header.yml @@ -0,0 +1,5 @@ +--- +title: Add project identifier as List-Id email Header to ease filtering +merge_request: 22817 +author: Olivier Crête +type: added diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md index 020aa73f809..6ce789998a4 100644 --- a/doc/workflow/notifications.md +++ b/doc/workflow/notifications.md @@ -135,6 +135,7 @@ Notification emails include headers that provide extra content about the notific | X-GitLab-Pipeline-Id | Only in pipeline emails, the ID of the pipeline the notification is for | | X-GitLab-Reply-Key | A unique token to support reply by email | | X-GitLab-NotificationReason | The reason for being notified. "mentioned", "assigned", etc | +| List-Id | The path of the project in a RFC 2919 mailing list identifier useful for email organization, for example, with GMail filters | #### X-GitLab-NotificationReason diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index 139387e0b24..3820cf5cb9d 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -73,4 +73,59 @@ describe EmailsHelper do end end end + + describe '#create_list_id_string' do + using RSpec::Parameterized::TableSyntax + + where(:full_path, :list_id_path) do + "01234" | "01234" + "5/0123" | "012.." + "45/012" | "012.." + "012" | "012" + "23/01" | "01.23" + "2/01" | "01.2" + "234/01" | "01.." + "4/2/0" | "0.2.4" + "45/2/0" | "0.2.." + "5/23/0" | "0.." + "0-2/5" | "5.0-2" + "0_2/5" | "5.0-2" + "0.2/5" | "5.0-2" + end + + with_them do + it 'ellipcizes different variants' do + project = double("project") + allow(project).to receive(:full_path).and_return(full_path) + allow(project).to receive(:id).and_return(12345) + # Set a max length that gives only 5 chars for the project full path + max_length = "12345..#{Gitlab.config.gitlab.host}".length + 5 + list_id = create_list_id_string(project, max_length) + + expect(list_id).to eq("12345.#{list_id_path}.#{Gitlab.config.gitlab.host}") + expect(list_id).to satisfy { |s| s.length <= max_length } + end + end + end + + describe 'Create realistic List-Id identifier' do + using RSpec::Parameterized::TableSyntax + + where(:full_path, :list_id_path) do + "gitlab-org/gitlab-ce" | "gitlab-ce.gitlab-org" + "project-name/subproject_name/my.project" | "my-project.subproject-name.project-name" + end + + with_them do + it 'Produces the right List-Id' do + project = double("project") + allow(project).to receive(:full_path).and_return(full_path) + allow(project).to receive(:id).and_return(12345) + list_id = create_list_id_string(project) + + expect(list_id).to eq("12345.#{list_id_path}.#{Gitlab.config.gitlab.host}") + expect(list_id).to satisfy { |s| s.length <= 255 } + end + end + end end diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index 66536e80db2..a38354060cf 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -1,5 +1,5 @@ shared_context 'gitlab email notification' do - set(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository, name: 'a-known-name') } set(:recipient) { create(:user, email: 'recipient@example.com') } let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name } @@ -62,9 +62,11 @@ end shared_examples 'an email with X-GitLab headers containing project details' do it 'has X-GitLab-Project headers' do aggregate_failures do + full_path_as_domain = "#{project.name}.#{project.namespace.path}" is_expected.to have_header('X-GitLab-Project', /#{project.name}/) is_expected.to have_header('X-GitLab-Project-Id', /#{project.id}/) is_expected.to have_header('X-GitLab-Project-Path', /#{project.full_path}/) + is_expected.to have_header('List-Id', "#{project.full_path} <#{project.id}.#{full_path_as_domain}.#{Gitlab.config.gitlab.host}>") end end end -- cgit v1.2.1