From a1dcdffd523e45a30f13a846c28ff89f8c37da4f Mon Sep 17 00:00:00 2001 From: Niklas Date: Mon, 2 Oct 2023 17:47:09 +0000 Subject: [PATCH 1/2] Use attachment_color for embed colors in Discord integration Changelog: changed --- .../chat_message/alert_message.rb | 4 ++-- .../chat_message/deployment_message.rb | 24 +++++++++---------- .../chat_message/issue_message.rb | 6 ++++- .../chat_message/pipeline_message.rb | 18 +++++++------- .../integrations/chat_message/push_message.rb | 8 +++---- app/models/integrations/discord.rb | 24 ++++++++++++++++++- spec/models/integrations/discord_spec.rb | 2 +- 7 files changed, 56 insertions(+), 30 deletions(-) diff --git a/app/models/integrations/chat_message/alert_message.rb b/app/models/integrations/chat_message/alert_message.rb index e2c689f9435376..6c7ea9aed7c8f1 100644 --- a/app/models/integrations/chat_message/alert_message.rb +++ b/app/models/integrations/chat_message/alert_message.rb @@ -34,12 +34,12 @@ def message "Alert firing in #{strip_markup(project_name)}" end - private - def attachment_color "#C95823" end + private + def attachment_fields [ { diff --git a/app/models/integrations/chat_message/deployment_message.rb b/app/models/integrations/chat_message/deployment_message.rb index 0367459dfcbff5..4d3e962d8852d5 100644 --- a/app/models/integrations/chat_message/deployment_message.rb +++ b/app/models/integrations/chat_message/deployment_message.rb @@ -30,7 +30,7 @@ def attachments [{ text: format(description_message), - color: color + color: attachment_color }] end @@ -38,17 +38,7 @@ def activity {} end - private - - def message - if running? - "Starting deploy to #{strip_markup(environment)}" - else - "Deploy to #{strip_markup(environment)} #{humanized_status}" - end - end - - def color + def attachment_color case status when 'success' 'good' @@ -61,6 +51,16 @@ def color end end + private + + def message + if running? + "Starting deploy to #{strip_markup(environment)}" + else + "Deploy to #{strip_markup(environment)} #{humanized_status}" + end + end + def project_link link(project_name, project_url) end diff --git a/app/models/integrations/chat_message/issue_message.rb b/app/models/integrations/chat_message/issue_message.rb index dd51636249125d..4c144bc2f68fea 100644 --- a/app/models/integrations/chat_message/issue_message.rb +++ b/app/models/integrations/chat_message/issue_message.rb @@ -41,6 +41,10 @@ def activity } end + def attachment_color + '#C95823' + end + private def message @@ -56,7 +60,7 @@ def description_message title: issue_title, title_link: issue_url, text: format(SlackMarkdownSanitizer.sanitize_slack_link(description)), - color: '#C95823' + color: attachment_color }] end diff --git a/app/models/integrations/chat_message/pipeline_message.rb b/app/models/integrations/chat_message/pipeline_message.rb index f8a634be3367f9..2abe4a6e9c7889 100644 --- a/app/models/integrations/chat_message/pipeline_message.rb +++ b/app/models/integrations/chat_message/pipeline_message.rb @@ -89,6 +89,15 @@ def activity } end + def attachment_color + case status + when 'success' + detailed_status == 'passed with warnings' ? 'warning' : 'good' + else + 'danger' + end + end + private def actually_failed_jobs(builds) @@ -180,15 +189,6 @@ def humanized_status end end - def attachment_color - case status - when 'success' - detailed_status == 'passed with warnings' ? 'warning' : 'good' - else - 'danger' - end - end - def ref_url if ref_type == 'tag' "#{project_url}/-/tags/#{ref}" diff --git a/app/models/integrations/chat_message/push_message.rb b/app/models/integrations/chat_message/push_message.rb index b17e28bb6c6c78..ee44fc98791dba 100644 --- a/app/models/integrations/chat_message/push_message.rb +++ b/app/models/integrations/chat_message/push_message.rb @@ -35,6 +35,10 @@ def activity } end + def attachment_color + '#345' + end + private def humanized_action(short: false) @@ -111,10 +115,6 @@ def compose_action_details ['pushed to', ref_link, "of #{project_link} (#{compare_link})"] end end - - def attachment_color - '#345' - end end end end diff --git a/app/models/integrations/discord.rb b/app/models/integrations/discord.rb index 815e3669d78000..d71fefe4f0bcd3 100644 --- a/app/models/integrations/discord.rb +++ b/app/models/integrations/discord.rb @@ -68,7 +68,7 @@ def notify(message, opts) builder.add_embed do |embed| embed.author = Discordrb::Webhooks::EmbedAuthor.new(name: message.user_name, icon_url: message.user_avatar) embed.description = (message.pretext + "\n" + Array.wrap(message.attachments).join("\n")).gsub(ATTACHMENT_REGEX, " \\k - \\k\n") - embed.colour = 16543014 # The hex "fc6d26" as an Integer + embed.colour = embed_color(message) embed.timestamp = Time.now.utc end end @@ -77,6 +77,28 @@ def notify(message, opts) false end + COLOR_OVERRIDES = { + 'good' => '#0d532a', + 'warning' => '#703800', + 'danger' => '#8d1300' + }.freeze + + def embed_color(message) + return 'fc6d26'.hex unless message.respond_to?(:attachment_color) + + color = message.attachment_color + + color = COLOR_OVERRIDES[color] if COLOR_OVERRIDES.key?(color) + + color = color.delete_prefix('#') + + if color.length == 3 + ((color[0, 1] * 2) + (color[1, 1] * 2) + (color[2, 1] * 2)).hex + else + color.hex + end + end + def custom_data(data) super(data).merge(markdown: true) end diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index 7ab7308ac1c3dd..76b5e89d04ff82 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -77,7 +77,7 @@ icon_url: start_with('https://www.gravatar.com/avatar/'), name: user.name ), - color: 16543014, + color: 3359829, timestamp: Time.now.utc.iso8601 ) end -- GitLab From e340b93038b23fbfa5ddcc13bde1ddc184852163 Mon Sep 17 00:00:00 2001 From: Niklas Date: Tue, 3 Oct 2023 12:14:13 +0000 Subject: [PATCH 2/2] Apply reviewer feedback --- app/models/integrations/discord.rb | 15 +++-- .../chat_message/alert_message_spec.rb | 6 ++ .../chat_message/deployment_message_spec.rb | 65 ++++++++++++------- .../chat_message/issue_message_spec.rb | 6 ++ .../chat_message/pipeline_message_spec.rb | 27 ++++++++ .../chat_message/push_message_spec.rb | 6 ++ 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/app/models/integrations/discord.rb b/app/models/integrations/discord.rb index d71fefe4f0bcd3..6aca9fc99219d0 100644 --- a/app/models/integrations/discord.rb +++ b/app/models/integrations/discord.rb @@ -92,11 +92,16 @@ def embed_color(message) color = color.delete_prefix('#') - if color.length == 3 - ((color[0, 1] * 2) + (color[1, 1] * 2) + (color[2, 1] * 2)).hex - else - color.hex - end + normalize_color(color).hex + end + + # Expands the short notation to the full colorcode notation + # 123456 -> 123456 + # 123 -> 112233 + def normalize_color(color) + return (color[0, 1] * 2) + (color[1, 1] * 2) + (color[2, 1] * 2) if color.length == 3 + + color end def custom_data(data) diff --git a/spec/models/integrations/chat_message/alert_message_spec.rb b/spec/models/integrations/chat_message/alert_message_spec.rb index 162df1a774c685..a9db9e14883353 100644 --- a/spec/models/integrations/chat_message/alert_message_spec.rb +++ b/spec/models/integrations/chat_message/alert_message_spec.rb @@ -57,4 +57,10 @@ expect(time_item[:value]).to eq(expected_time) end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#C95823') + end + end end diff --git a/spec/models/integrations/chat_message/deployment_message_spec.rb b/spec/models/integrations/chat_message/deployment_message_spec.rb index 630ae9023314c2..afbf1d1c0d15de 100644 --- a/spec/models/integrations/chat_message/deployment_message_spec.rb +++ b/spec/models/integrations/chat_message/deployment_message_spec.rb @@ -19,6 +19,29 @@ it_behaves_like Integrations::ChatMessage + def deployment_data(params) + { + object_kind: "deployment", + status: "success", + deployable_id: 3, + deployable_url: "deployable_url", + environment: "sandbox", + project: { + name: "greatproject", + web_url: "project_web_url", + path_with_namespace: "project_path_with_namespace" + }, + user: { + name: "Jane Person", + username: "jane" + }, + user_url: "user_url", + short_sha: "12345678", + commit_url: "commit_url", + commit_title: "commit title text" + }.merge(params) + end + describe '#pretext' do it 'returns a message with the data returned by the deployment data builder' do expect(subject.pretext).to eq("Deploy to myenvironment succeeded") @@ -80,29 +103,6 @@ end describe '#attachments' do - def deployment_data(params) - { - object_kind: "deployment", - status: "success", - deployable_id: 3, - deployable_url: "deployable_url", - environment: "sandbox", - project: { - name: "greatproject", - web_url: "project_web_url", - path_with_namespace: "project_path_with_namespace" - }, - user: { - name: "Jane Person", - username: "jane" - }, - user_url: "user_url", - short_sha: "12345678", - commit_url: "commit_url", - commit_title: "commit title text" - }.merge(params) - end - context 'without markdown' do it 'returns attachments with the data returned by the deployment data builder' do job_url = Gitlab::Routing.url_helpers.project_job_url(project, ci_build) @@ -165,4 +165,23 @@ def deployment_data(params) }]) end end + + describe '#attachment_color' do + using RSpec::Parameterized::TableSyntax + where(:status, :expected_color) do + 'success' | 'good' + 'canceled' | 'warning' + 'failed' | 'danger' + 'blub' | '#334455' + end + + with_them do + it 'returns the correct color' do + data = deployment_data(status: status) + message = described_class.new(data) + + expect(message.attachment_color).to eq(expected_color) + end + end + end end diff --git a/spec/models/integrations/chat_message/issue_message_spec.rb b/spec/models/integrations/chat_message/issue_message_spec.rb index 14451427a5a654..7b09b5d08b058b 100644 --- a/spec/models/integrations/chat_message/issue_message_spec.rb +++ b/spec/models/integrations/chat_message/issue_message_spec.rb @@ -125,4 +125,10 @@ end end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#C95823') + end + end end diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index 4d371ca0899577..5eb3915018e179 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -388,4 +388,31 @@ ) end end + + describe '#attachment_color' do + context 'when success' do + before do + args[:object_attributes][:status] = 'success' + end + + it { expect(subject.attachment_color).to eq('good') } + end + + context 'when passed with warnings' do + before do + args[:object_attributes][:status] = 'success' + args[:object_attributes][:detailed_status] = 'passed with warnings' + end + + it { expect(subject.attachment_color).to eq('warning') } + end + + context 'when failed' do + before do + args[:object_attributes][:status] = 'failed' + end + + it { expect(subject.attachment_color).to eq('danger') } + end + end end diff --git a/spec/models/integrations/chat_message/push_message_spec.rb b/spec/models/integrations/chat_message/push_message_spec.rb index 5c9c5c64d7ed0b..a9d0f80140670d 100644 --- a/spec/models/integrations/chat_message/push_message_spec.rb +++ b/spec/models/integrations/chat_message/push_message_spec.rb @@ -214,4 +214,10 @@ end end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#345') + end + end end -- GitLab