From ab99ac9d4e5be31bdd6ced03ea4a89ea3e73eac5 Mon Sep 17 00:00:00 2001 From: Markus Koller <mkoller@gitlab.com> Date: Tue, 28 Jun 2022 07:25:16 +0000 Subject: [PATCH] Refactor more integrations to use the DSL (1/2) --- .../concerns/notification_branch_selection.rb | 16 +++-- app/models/integrations/asana.rb | 37 ++++------- app/models/integrations/assembla.rb | 29 +++----- .../integrations/base_chat_notification.rb | 6 +- app/models/integrations/confluence.rb | 19 ++---- app/models/integrations/discord.rb | 2 +- app/models/integrations/emails_on_push.rb | 51 +++++++------- app/models/integrations/external_wiki.rb | 20 ++---- app/models/integrations/flowdock.rb | 23 +++---- app/models/integrations/hangouts_chat.rb | 2 +- app/models/integrations/microsoft_teams.rb | 2 +- app/models/integrations/pipelines_email.rb | 2 +- app/models/integrations/pivotaltracker.rb | 35 ++++------ app/models/integrations/prometheus.rb | 66 +++++++------------ app/models/integrations/unify_circuit.rb | 2 +- app/models/integrations/webex_teams.rb | 2 +- app/serializers/integrations/event_entity.rb | 2 +- locale/gitlab.pot | 6 ++ .../base_chat_notification_spec.rb | 6 +- spec/models/integrations/prometheus_spec.rb | 43 ------------ 20 files changed, 135 insertions(+), 236 deletions(-) diff --git a/app/models/concerns/notification_branch_selection.rb b/app/models/concerns/notification_branch_selection.rb index 18ec996c3df75..f2df7579a653a 100644 --- a/app/models/concerns/notification_branch_selection.rb +++ b/app/models/concerns/notification_branch_selection.rb @@ -6,13 +6,15 @@ module NotificationBranchSelection extend ActiveSupport::Concern - def branch_choices - [ - [_('All branches'), 'all'].freeze, - [_('Default branch'), 'default'].freeze, - [_('Protected branches'), 'protected'].freeze, - [_('Default branch and protected branches'), 'default_and_protected'].freeze - ].freeze + class_methods do + def branch_choices + [ + [_('All branches'), 'all'].freeze, + [_('Default branch'), 'default'].freeze, + [_('Protected branches'), 'protected'].freeze, + [_('Default branch and protected branches'), 'default_and_protected'].freeze + ].freeze + end end def notify_for_branch?(data) diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index d25bf8b1b1e79..2cfd71c9eb2a0 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -4,9 +4,22 @@ module Integrations class Asana < Integration - prop_accessor :api_key, :restrict_to_branch validates :api_key, presence: true, if: :activated? + field :api_key, + type: 'password', + title: 'API key', + help: -> { s_('AsanaService|User Personal Access Token. User must have access to the task. All comments are attributed to this user.') }, + non_empty_password_title: -> { s_('ProjectService|Enter new API key') }, + non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current API key.') }, + # Example Personal Access Token from Asana docs + placeholder: '0/68a9e79b868c6789e79a124c30b0', + required: true + + field :restrict_to_branch, + title: -> { s_('Integrations|Restrict to branch (optional)') }, + help: -> { s_('AsanaService|Comma-separated list of branches to be automatically inspected. Leave blank to include all branches.') } + def title 'Asana' end @@ -24,28 +37,6 @@ def self.to_param 'asana' end - def fields - [ - { - type: 'password', - name: 'api_key', - title: 'API key', - help: s_('AsanaService|User Personal Access Token. User must have access to the task. All comments are attributed to this user.'), - non_empty_password_title: s_('ProjectService|Enter new API key'), - non_empty_password_help: s_('ProjectService|Leave blank to use your current API key.'), - # Example Personal Access Token from Asana docs - placeholder: '0/68a9e79b868c6789e79a124c30b0', - required: true - }, - { - type: 'text', - name: 'restrict_to_branch', - title: 'Restrict to branch (optional)', - help: s_('AsanaService|Comma-separated list of branches to be automatically inspected. Leave blank to include all branches.') - } - ] - end - def self.supported_events %w(push) end diff --git a/app/models/integrations/assembla.rb b/app/models/integrations/assembla.rb index ccd24c1fb2c20..88dbf2915ef7d 100644 --- a/app/models/integrations/assembla.rb +++ b/app/models/integrations/assembla.rb @@ -2,9 +2,18 @@ module Integrations class Assembla < Integration - prop_accessor :token, :subdomain validates :token, presence: true, if: :activated? + field :token, + type: 'password', + non_empty_password_title: -> { s_('ProjectService|Enter new token') }, + non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') }, + placeholder: '', + required: true + + field :subdomain, + placeholder: '' + def title 'Assembla' end @@ -17,24 +26,6 @@ def self.to_param 'assembla' end - def fields - [ - { - type: 'password', - name: 'token', - non_empty_password_title: s_('ProjectService|Enter new token'), - non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), - placeholder: '', - required: true - }, - { - type: 'text', - name: 'subdomain', - placeholder: '' - } - ] - end - def self.supported_events %w(push) end diff --git a/app/models/integrations/base_chat_notification.rb b/app/models/integrations/base_chat_notification.rb index 0cfdcdbaf5f2e..51f238d5eb902 100644 --- a/app/models/integrations/base_chat_notification.rb +++ b/app/models/integrations/base_chat_notification.rb @@ -78,7 +78,7 @@ def default_fields type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices }.freeze, { type: 'text', @@ -118,7 +118,7 @@ def execute(data) event_type = data[:event_type] || object_kind - channel_names = get_channel_field(event_type).presence || channel.presence + channel_names = event_channel_value(event_type).presence || channel.presence channels = channel_names&.split(',')&.map(&:strip) opts = {} @@ -161,7 +161,7 @@ def event_channel_name(event) EVENT_CHANNEL[event] end - def get_channel_field(event) + def event_channel_value(event) field_name = event_channel_name(event) self.public_send(field_name) # rubocop:disable GitlabSecurity/PublicSend end diff --git a/app/models/integrations/confluence.rb b/app/models/integrations/confluence.rb index 4e1d1993d026e..c1c43af99bf2c 100644 --- a/app/models/integrations/confluence.rb +++ b/app/models/integrations/confluence.rb @@ -6,11 +6,14 @@ class Confluence < BaseThirdPartyWiki VALID_HOST_MATCH = %r{\A.+\.atlassian\.net\Z}.freeze VALID_PATH_MATCH = %r{\A/wiki(/|\Z)}.freeze - prop_accessor :confluence_url - validates :confluence_url, presence: true, if: :activated? validate :validate_confluence_url_is_cloud, if: :activated? + field :confluence_url, + title: -> { s_('Confluence Cloud Workspace URL') }, + placeholder: 'https://example.atlassian.net/wiki', + required: true + def self.to_param 'confluence' end @@ -38,18 +41,6 @@ def help end end - def fields - [ - { - type: 'text', - name: 'confluence_url', - title: s_('Confluence Cloud Workspace URL'), - placeholder: 'https://example.atlassian.net/wiki', - required: true - } - ] - end - def testable? false end diff --git a/app/models/integrations/discord.rb b/app/models/integrations/discord.rb index 231d1c623402e..ecabf23c90bb1 100644 --- a/app/models/integrations/discord.rb +++ b/app/models/integrations/discord.rb @@ -39,7 +39,7 @@ def default_fields type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices } ] end diff --git a/app/models/integrations/emails_on_push.rb b/app/models/integrations/emails_on_push.rb index ab458bb2c2702..ed12a3a8d6337 100644 --- a/app/models/integrations/emails_on_push.rb +++ b/app/models/integrations/emails_on_push.rb @@ -6,12 +6,35 @@ class EmailsOnPush < Integration RECIPIENTS_LIMIT = 750 - boolean_accessor :send_from_committer_email - boolean_accessor :disable_diffs - prop_accessor :recipients, :branches_to_be_notified validates :recipients, presence: true, if: :validate_recipients? validate :number_of_recipients_within_limit, if: :validate_recipients? + field :send_from_committer_email, + type: 'checkbox', + title: -> { s_("EmailsOnPushService|Send from committer") }, + help: -> do + @help ||= begin + domains = Notify.allowed_email_domains.map { |domain| "user@#{domain}" }.join(", ") + + s_("EmailsOnPushService|Send notifications from the committer's email address if the domain matches the domain used by your GitLab instance (such as %{domains}).") % { domains: domains } + end + end + + field :disable_diffs, + type: 'checkbox', + title: -> { s_("EmailsOnPushService|Disable code diffs") }, + help: -> { s_("EmailsOnPushService|Don't include possibly sensitive code diffs in notification body.") } + + field :branches_to_be_notified, + type: 'select', + title: -> { s_('Integrations|Branches for which notifications are to be sent') }, + choices: branch_choices + + field :recipients, + type: 'textarea', + placeholder: -> { s_('EmailsOnPushService|tanuki@example.com gitlab@example.com') }, + help: -> { s_('EmailsOnPushService|Emails separated by whitespace.') } + def self.valid_recipients(recipients) recipients.split.grep(Devise.email_regexp).uniq(&:downcase) end @@ -67,28 +90,6 @@ def disable_diffs? Gitlab::Utils.to_boolean(self.disable_diffs) end - def fields - domains = Notify.allowed_email_domains.map { |domain| "user@#{domain}" }.join(", ") - [ - { type: 'checkbox', name: 'send_from_committer_email', title: s_("EmailsOnPushService|Send from committer"), - help: s_("EmailsOnPushService|Send notifications from the committer's email address if the domain matches the domain used by your GitLab instance (such as %{domains}).") % { domains: domains } }, - { type: 'checkbox', name: 'disable_diffs', title: s_("EmailsOnPushService|Disable code diffs"), - help: s_("EmailsOnPushService|Don't include possibly sensitive code diffs in notification body.") }, - { - type: 'select', - name: 'branches_to_be_notified', - title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices - }, - { - type: 'textarea', - name: 'recipients', - placeholder: s_('EmailsOnPushService|tanuki@example.com gitlab@example.com'), - help: s_('EmailsOnPushService|Emails separated by whitespace.') - } - ] - end - private def number_of_recipients_within_limit diff --git a/app/models/integrations/external_wiki.rb b/app/models/integrations/external_wiki.rb index 18c48411e30f2..c16ae9926f1de 100644 --- a/app/models/integrations/external_wiki.rb +++ b/app/models/integrations/external_wiki.rb @@ -2,9 +2,14 @@ module Integrations class ExternalWiki < Integration - prop_accessor :external_wiki_url validates :external_wiki_url, presence: true, public_url: true, if: :activated? + field :external_wiki_url, + title: -> { s_('ExternalWikiService|External wiki URL') }, + placeholder: -> { s_('ExternalWikiService|https://example.com/xxx/wiki/...') }, + help: -> { s_('ExternalWikiService|Enter the URL to the external wiki.') }, + required: true + def title s_('ExternalWikiService|External wiki') end @@ -17,19 +22,6 @@ def self.to_param 'external_wiki' end - def fields - [ - { - type: 'text', - name: 'external_wiki_url', - title: s_('ExternalWikiService|External wiki URL'), - placeholder: s_('ExternalWikiService|https://example.com/xxx/wiki/...'), - help: 'Enter the URL to the external wiki.', - required: true - } - ] - end - def help docs_link = ActionController::Base.helpers.link_to _('Learn more.'), Rails.application.routes.url_helpers.help_page_url('user/project/wiki/index', anchor: 'link-an-external-wiki'), target: '_blank', rel: 'noopener noreferrer' diff --git a/app/models/integrations/flowdock.rb b/app/models/integrations/flowdock.rb index 703d8013babd9..52efb29f2c13e 100644 --- a/app/models/integrations/flowdock.rb +++ b/app/models/integrations/flowdock.rb @@ -2,9 +2,16 @@ module Integrations class Flowdock < Integration - prop_accessor :token validates :token, presence: true, if: :activated? + field :token, + type: 'password', + help: -> { s_('FlowdockService|Enter your Flowdock token.') }, + non_empty_password_title: -> { s_('ProjectService|Enter new token') }, + non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') }, + placeholder: '1b609b52537...', + required: true + def title 'Flowdock' end @@ -22,20 +29,6 @@ def self.to_param 'flowdock' end - def fields - [ - { - type: 'password', - name: 'token', - help: s_('FlowdockService|Enter your Flowdock token.'), - non_empty_password_title: s_('ProjectService|Enter new token'), - non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), - placeholder: '1b609b52537...', - required: true - } - ] - end - def self.supported_events %w(push) end diff --git a/app/models/integrations/hangouts_chat.rb b/app/models/integrations/hangouts_chat.rb index 4667bbffa1b9f..df112ad6ca85e 100644 --- a/app/models/integrations/hangouts_chat.rb +++ b/app/models/integrations/hangouts_chat.rb @@ -39,7 +39,7 @@ def default_fields type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices } ] end diff --git a/app/models/integrations/microsoft_teams.rb b/app/models/integrations/microsoft_teams.rb index 949b24ca03f94..69863f164cdd3 100644 --- a/app/models/integrations/microsoft_teams.rb +++ b/app/models/integrations/microsoft_teams.rb @@ -44,7 +44,7 @@ def default_fields section: SECTION_TYPE_CONFIGURATION, name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices } ] end diff --git a/app/models/integrations/pipelines_email.rb b/app/models/integrations/pipelines_email.rb index f15482dc2e18f..c08d1fe7f4af3 100644 --- a/app/models/integrations/pipelines_email.rb +++ b/app/models/integrations/pipelines_email.rb @@ -76,7 +76,7 @@ def fields { type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices } + choices: self.class.branch_choices } ] end diff --git a/app/models/integrations/pivotaltracker.rb b/app/models/integrations/pivotaltracker.rb index 931ccf46655d5..d32fb97433922 100644 --- a/app/models/integrations/pivotaltracker.rb +++ b/app/models/integrations/pivotaltracker.rb @@ -4,9 +4,22 @@ module Integrations class Pivotaltracker < Integration API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits' - prop_accessor :token, :restrict_to_branch validates :token, presence: true, if: :activated? + field :token, + type: 'password', + help: -> { s_('PivotalTrackerService|Pivotal Tracker API token. User must have access to the story. All comments are attributed to this user.') }, + non_empty_password_title: -> { s_('ProjectService|Enter new token') }, + non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') }, + required: true + + field :restrict_to_branch, + title: -> { s_('Integrations|Restrict to branch (optional)') }, + help: -> do + s_('PivotalTrackerService|Comma-separated list of branches to ' \ + 'automatically inspect. Leave blank to include all branches.') + end + def title 'Pivotal Tracker' end @@ -24,26 +37,6 @@ def self.to_param 'pivotaltracker' end - def fields - [ - { - type: 'password', - name: 'token', - help: s_('PivotalTrackerService|Pivotal Tracker API token. User must have access to the story. All comments are attributed to this user.'), - non_empty_password_title: s_('ProjectService|Enter new token'), - non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), - required: true - }, - { - type: 'text', - name: 'restrict_to_branch', - title: 'Restrict to branch (optional)', - help: s_('PivotalTrackerService|Comma-separated list of branches to ' \ - 'automatically inspect. Leave blank to include all branches.') - } - ] - end - def self.supported_events %w(push) end diff --git a/app/models/integrations/prometheus.rb b/app/models/integrations/prometheus.rb index 360605653172f..e672a98581055 100644 --- a/app/models/integrations/prometheus.rb +++ b/app/models/integrations/prometheus.rb @@ -4,11 +4,30 @@ module Integrations class Prometheus < BaseMonitoring include PrometheusAdapter - # Access to prometheus is directly through the API - prop_accessor :api_url - prop_accessor :google_iap_service_account_json - prop_accessor :google_iap_audience_client_id - boolean_accessor :manual_configuration + field :manual_configuration, + type: 'checkbox', + title: -> { s_('PrometheusService|Active') }, + help: -> { s_('PrometheusService|Select this checkbox to override the auto configuration settings with your own settings.') }, + required: true + + field :api_url, + title: 'API URL', + placeholder: -> { s_('PrometheusService|https://prometheus.example.com/') }, + help: -> { s_('PrometheusService|The Prometheus API base URL.') }, + required: true + + field :google_iap_audience_client_id, + title: 'Google IAP Audience Client ID', + placeholder: -> { s_('PrometheusService|IAP_CLIENT_ID.apps.googleusercontent.com') }, + help: -> { s_('PrometheusService|The ID of the IAP-secured resource.') }, + required: false + + field :google_iap_service_account_json, + type: 'textarea', + title: 'Google IAP Service Account JSON', + placeholder: -> { s_('PrometheusService|{ "type": "service_account", "project_id": ... }') }, + help: -> { s_('PrometheusService|The contents of the credentials.json file of your service account.') }, + required: false # We need to allow the self-monitoring project to connect to the internal # Prometheus instance. @@ -45,43 +64,6 @@ def self.to_param 'prometheus' end - def fields - [ - { - type: 'checkbox', - name: 'manual_configuration', - title: s_('PrometheusService|Active'), - help: s_('PrometheusService|Select this checkbox to override the auto configuration settings with your own settings.'), - required: true - }, - { - type: 'text', - name: 'api_url', - title: 'API URL', - placeholder: s_('PrometheusService|https://prometheus.example.com/'), - help: s_('PrometheusService|The Prometheus API base URL.'), - required: true - }, - { - type: 'text', - name: 'google_iap_audience_client_id', - title: 'Google IAP Audience Client ID', - placeholder: s_('PrometheusService|IAP_CLIENT_ID.apps.googleusercontent.com'), - help: s_('PrometheusService|The ID of the IAP-secured resource.'), - autocomplete: 'off', - required: false - }, - { - type: 'textarea', - name: 'google_iap_service_account_json', - title: 'Google IAP Service Account JSON', - placeholder: s_('PrometheusService|{ "type": "service_account", "project_id": ... }'), - help: s_('PrometheusService|The contents of the credentials.json file of your service account.'), - required: false - } - ] - end - # Check we can connect to the Prometheus API def test(*args) return { success: false, result: 'Prometheus configuration error' } unless prometheus_client diff --git a/app/models/integrations/unify_circuit.rb b/app/models/integrations/unify_circuit.rb index 40570c098c282..646c2e75b03fd 100644 --- a/app/models/integrations/unify_circuit.rb +++ b/app/models/integrations/unify_circuit.rb @@ -35,7 +35,7 @@ def default_fields type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices } ] end diff --git a/app/models/integrations/webex_teams.rb b/app/models/integrations/webex_teams.rb index 7bf902592670d..54d6f51ee1740 100644 --- a/app/models/integrations/webex_teams.rb +++ b/app/models/integrations/webex_teams.rb @@ -35,7 +35,7 @@ def default_fields type: 'select', name: 'branches_to_be_notified', title: s_('Integrations|Branches for which notifications are to be sent'), - choices: branch_choices + choices: self.class.branch_choices } ] end diff --git a/app/serializers/integrations/event_entity.rb b/app/serializers/integrations/event_entity.rb index f603c0e035e00..91bd91dd94165 100644 --- a/app/serializers/integrations/event_entity.rb +++ b/app/serializers/integrations/event_entity.rb @@ -23,7 +23,7 @@ class EventEntity < Grape::Entity integration.event_channel_name(event) end expose :value do |event| - integration.get_channel_field(event) + integration.event_channel_value(event) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cad7aeeaf6b35..802221e5668c5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15487,6 +15487,9 @@ msgstr "" msgid "ExternalIssueIntegration|This issue is synchronized with %{trackerName}" msgstr "" +msgid "ExternalWikiService|Enter the URL to the external wiki." +msgstr "" + msgid "ExternalWikiService|External wiki" msgstr "" @@ -20737,6 +20740,9 @@ msgstr "" msgid "Integrations|Resetting this integration will clear the settings and deactivate this integration." msgstr "" +msgid "Integrations|Restrict to branch (optional)" +msgstr "" + msgid "Integrations|Return to GitLab for Jira" msgstr "" diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 8cbefbb9b47df..eb503e501d69d 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -292,15 +292,15 @@ end end - describe '#get_channel_field' do + describe '#event_channel_value' do it 'returns the channel field value for the given event' do subject.push_channel = '#pushes' - expect(subject.get_channel_field(:push)).to eq('#pushes') + expect(subject.event_channel_value(:push)).to eq('#pushes') end it 'raises an error for unsupported events' do - expect { subject.get_channel_field(:foo) }.to raise_error(NoMethodError) + expect { subject.event_channel_value(:foo) }.to raise_error(NoMethodError) end end end diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index fbeaebfd807fd..ae965ed78d13f 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -475,47 +475,4 @@ def stub_iap_request end end end - - describe '#fields' do - let(:expected_fields) do - [ - { - type: 'checkbox', - name: 'manual_configuration', - title: s_('PrometheusService|Active'), - help: s_('PrometheusService|Select this checkbox to override the auto configuration settings with your own settings.'), - required: true - }, - { - type: 'text', - name: 'api_url', - title: 'API URL', - placeholder: s_('PrometheusService|https://prometheus.example.com/'), - help: s_('PrometheusService|The Prometheus API base URL.'), - required: true - }, - { - type: 'text', - name: 'google_iap_audience_client_id', - title: 'Google IAP Audience Client ID', - placeholder: s_('PrometheusService|IAP_CLIENT_ID.apps.googleusercontent.com'), - help: s_('PrometheusService|The ID of the IAP-secured resource.'), - autocomplete: 'off', - required: false - }, - { - type: 'textarea', - name: 'google_iap_service_account_json', - title: 'Google IAP Service Account JSON', - placeholder: s_('PrometheusService|{ "type": "service_account", "project_id": ... }'), - help: s_('PrometheusService|The contents of the credentials.json file of your service account.'), - required: false - } - ] - end - - it 'returns fields' do - expect(integration.fields).to eq(expected_fields) - end - end end -- GitLab