From 5121a5ebd16c3ca588613b6e31ff20390fba437b Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Thu, 31 Oct 2024 11:25:40 +0000 Subject: [PATCH 1/7] Update chat-based integrations to use shared modules In order to support the split of integrations between instance/non-instance we need to use shared functionality via modules. For this, we need to update existing integrations to use shared modules. Changelog: other EE: true --- .../integrations/gitlab_slack_application.rb | 3 +- app/models/integrations/mattermost.rb | 1 - app/models/integrations/slack.rb | 1 - .../ee/integrations/base/integration.rb | 72 +++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 ee/app/models/ee/integrations/base/integration.rb diff --git a/app/models/integrations/gitlab_slack_application.rb b/app/models/integrations/gitlab_slack_application.rb index 2ed5a38333a458..77ff9504e7210f 100644 --- a/app/models/integrations/gitlab_slack_application.rb +++ b/app/models/integrations/gitlab_slack_application.rb @@ -88,6 +88,8 @@ def upgrade_needed? slack_integration.present? && slack_integration.upgrade_needed? end + def default_channel_placeholder; end + private override :notify @@ -160,7 +162,6 @@ def test_notification_channels ) end - override :metrics_key_prefix def metrics_key_prefix 'i_integrations_gitlab_for_slack_app' end diff --git a/app/models/integrations/mattermost.rb b/app/models/integrations/mattermost.rb index ed9365f72a7437..37ea3d1a665591 100644 --- a/app/models/integrations/mattermost.rb +++ b/app/models/integrations/mattermost.rb @@ -33,7 +33,6 @@ def self.webhook_help 'http://mattermost.example.com/hooks/...' end - override :configurable_channels? def configurable_channels? true end diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index 6495d2240a1e75..0c195b09dae57b 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -24,7 +24,6 @@ def self.webhook_help private - override :metrics_key_prefix def metrics_key_prefix 'i_ecosystem_slack_service' end diff --git a/ee/app/models/ee/integrations/base/integration.rb b/ee/app/models/ee/integrations/base/integration.rb new file mode 100644 index 00000000000000..1431b0e576e0b2 --- /dev/null +++ b/ee/app/models/ee/integrations/base/integration.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module EE + module Integrations + module Base + module Integration + extend ActiveSupport::Concern + + prepended do + scope :vulnerability_hooks, -> { where(vulnerability_events: true, active: true) } + end + + EE_INTEGRATION_NAMES = %w[ + google_cloud_platform_workload_identity_federation + git_guardian + ].freeze + + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES = %w[ + github + google_cloud_platform_artifact_registry + ].freeze + + GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES = %w[ + google_cloud_platform_artifact_registry + google_cloud_platform_workload_identity_federation + ].freeze + + class_methods do + extend ::Gitlab::Utils::Override + + override :integration_names + def integration_names + names = super + EE_INTEGRATION_NAMES + + unless ::Gitlab::Saas.feature_available?(:google_cloud_support) + names.delete('google_cloud_platform_workload_identity_federation') + end + + names.delete('git_guardian') unless ::Feature.enabled?(:git_guardian_integration) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Legacy use + + names + end + + override :project_specific_integration_names + def project_specific_integration_names + names = super + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES + + unless ::Gitlab::Saas.feature_available?(:google_cloud_support) + names.delete('google_cloud_platform_artifact_registry') + end + + names + end + + # Returns the STI type for the given integration name. + # Example: "asana" => "Integrations::Asana" + override :integration_name_to_type + def integration_name_to_type(name) + name = name.to_s + + if GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES.include?(name) + name = name.delete_prefix("google_cloud_platform_") + "Integrations::GoogleCloudPlatform::#{name.camelize}" + else + super + end + end + end + end + end + end +end -- GitLab From f3f8c3dded77cc4f31dedecc1930fa1929fb9e23 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Mon, 11 Nov 2024 11:14:57 +0000 Subject: [PATCH 2/7] Update BaseChatNotification references --- doc/development/integrations/index.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index e8d67d3600040b..6e9ad58291cf56 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -23,11 +23,11 @@ if you need clarification or spot any outdated information. - For example, `Integrations::FooBar` in `app/models/integrations/foo_bar.rb`. - For certain types of integrations, you can include these base modules (or inherit from them if they are classes): - `Integrations::Base::ChatNotification` - - `Integrations::BaseCi` - - `Integrations::BaseIssueTracker` - - `Integrations::BaseMonitoring` - - `Integrations::BaseSlashCommands` - - `Integrations::BaseThirdPartyWiki` + - `Integrations::Base::Ci` + - `Integrations::Base::IssueTracker` + - `Integrations::Base::Monitoring` + - `Integrations::Base::SlashCommands` + - `Integrations::Base::ThirdPartyWiki` - For integrations that primarily trigger HTTP calls to external services, you can also use the `Integrations::HasWebHook` concern. This reuses the [webhook functionality](../../user/project/integrations/webhooks.md) in GitLab through an associated `ServiceHook` model, and automatically records request logs -- GitLab From ffea22f8d77009edd15d9d8d154a80ba276bd79e Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Thu, 14 Nov 2024 11:16:32 +0000 Subject: [PATCH 3/7] Move ee integrations module to the right location --- .../integrations/gitlab_slack_application.rb | 1 + app/models/integrations/mattermost.rb | 1 + app/models/integrations/slack.rb | 1 + .../ee/integrations/base/integration.rb | 72 ------------------- 4 files changed, 3 insertions(+), 72 deletions(-) delete mode 100644 ee/app/models/ee/integrations/base/integration.rb diff --git a/app/models/integrations/gitlab_slack_application.rb b/app/models/integrations/gitlab_slack_application.rb index 77ff9504e7210f..233accf9db5e10 100644 --- a/app/models/integrations/gitlab_slack_application.rb +++ b/app/models/integrations/gitlab_slack_application.rb @@ -162,6 +162,7 @@ def test_notification_channels ) end + override :metrics_key_prefix def metrics_key_prefix 'i_integrations_gitlab_for_slack_app' end diff --git a/app/models/integrations/mattermost.rb b/app/models/integrations/mattermost.rb index 37ea3d1a665591..ed9365f72a7437 100644 --- a/app/models/integrations/mattermost.rb +++ b/app/models/integrations/mattermost.rb @@ -33,6 +33,7 @@ def self.webhook_help 'http://mattermost.example.com/hooks/...' end + override :configurable_channels? def configurable_channels? true end diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index 0c195b09dae57b..6495d2240a1e75 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -24,6 +24,7 @@ def self.webhook_help private + override :metrics_key_prefix def metrics_key_prefix 'i_ecosystem_slack_service' end diff --git a/ee/app/models/ee/integrations/base/integration.rb b/ee/app/models/ee/integrations/base/integration.rb deleted file mode 100644 index 1431b0e576e0b2..00000000000000 --- a/ee/app/models/ee/integrations/base/integration.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module EE - module Integrations - module Base - module Integration - extend ActiveSupport::Concern - - prepended do - scope :vulnerability_hooks, -> { where(vulnerability_events: true, active: true) } - end - - EE_INTEGRATION_NAMES = %w[ - google_cloud_platform_workload_identity_federation - git_guardian - ].freeze - - EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES = %w[ - github - google_cloud_platform_artifact_registry - ].freeze - - GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES = %w[ - google_cloud_platform_artifact_registry - google_cloud_platform_workload_identity_federation - ].freeze - - class_methods do - extend ::Gitlab::Utils::Override - - override :integration_names - def integration_names - names = super + EE_INTEGRATION_NAMES - - unless ::Gitlab::Saas.feature_available?(:google_cloud_support) - names.delete('google_cloud_platform_workload_identity_federation') - end - - names.delete('git_guardian') unless ::Feature.enabled?(:git_guardian_integration) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Legacy use - - names - end - - override :project_specific_integration_names - def project_specific_integration_names - names = super + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES - - unless ::Gitlab::Saas.feature_available?(:google_cloud_support) - names.delete('google_cloud_platform_artifact_registry') - end - - names - end - - # Returns the STI type for the given integration name. - # Example: "asana" => "Integrations::Asana" - override :integration_name_to_type - def integration_name_to_type(name) - name = name.to_s - - if GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES.include?(name) - name = name.delete_prefix("google_cloud_platform_") - "Integrations::GoogleCloudPlatform::#{name.camelize}" - else - super - end - end - end - end - end - end -end -- GitLab From 07a9e59c1b03fa6542fd9b534094cdafb5ba41a1 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Thu, 14 Nov 2024 16:09:08 +0000 Subject: [PATCH 4/7] Code review feedback --- app/models/integrations/gitlab_slack_application.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/integrations/gitlab_slack_application.rb b/app/models/integrations/gitlab_slack_application.rb index 233accf9db5e10..2ed5a38333a458 100644 --- a/app/models/integrations/gitlab_slack_application.rb +++ b/app/models/integrations/gitlab_slack_application.rb @@ -88,8 +88,6 @@ def upgrade_needed? slack_integration.present? && slack_integration.upgrade_needed? end - def default_channel_placeholder; end - private override :notify -- GitLab From a85c3b05ddf4e58f46870bd0a4875f79586e99e6 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Thu, 31 Oct 2024 11:25:40 +0000 Subject: [PATCH 5/7] Update chat-based integrations to use shared modules In order to support the split of integrations between instance/non-instance we need to use shared functionality via modules. For this, we need to update existing integrations to use shared modules. Changelog: other EE: true --- .../concerns/integrations/base/integration.rb | 6 ++ .../integrations/gitlab_slack_application.rb | 3 +- app/models/integrations/mattermost.rb | 1 - app/models/integrations/slack.rb | 1 - .../ee/integrations/base/integration.rb | 72 +++++++++++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 ee/app/models/ee/integrations/base/integration.rb diff --git a/app/models/concerns/integrations/base/integration.rb b/app/models/concerns/integrations/base/integration.rb index dd6fd217ef3bc1..b73f4e33920f0d 100644 --- a/app/models/concerns/integrations/base/integration.rb +++ b/app/models/concerns/integrations/base/integration.rb @@ -817,6 +817,12 @@ def validate_belongs_to_project_or_group def validate_recipients? activated? && !importing? end + + class_methods do + def supported_events + %w[commit push tag_push issue confidential_issue merge_request wiki_page] + end + end end end end diff --git a/app/models/integrations/gitlab_slack_application.rb b/app/models/integrations/gitlab_slack_application.rb index 2ed5a38333a458..77ff9504e7210f 100644 --- a/app/models/integrations/gitlab_slack_application.rb +++ b/app/models/integrations/gitlab_slack_application.rb @@ -88,6 +88,8 @@ def upgrade_needed? slack_integration.present? && slack_integration.upgrade_needed? end + def default_channel_placeholder; end + private override :notify @@ -160,7 +162,6 @@ def test_notification_channels ) end - override :metrics_key_prefix def metrics_key_prefix 'i_integrations_gitlab_for_slack_app' end diff --git a/app/models/integrations/mattermost.rb b/app/models/integrations/mattermost.rb index ed9365f72a7437..37ea3d1a665591 100644 --- a/app/models/integrations/mattermost.rb +++ b/app/models/integrations/mattermost.rb @@ -33,7 +33,6 @@ def self.webhook_help 'http://mattermost.example.com/hooks/...' end - override :configurable_channels? def configurable_channels? true end diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index 6495d2240a1e75..0c195b09dae57b 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -24,7 +24,6 @@ def self.webhook_help private - override :metrics_key_prefix def metrics_key_prefix 'i_ecosystem_slack_service' end diff --git a/ee/app/models/ee/integrations/base/integration.rb b/ee/app/models/ee/integrations/base/integration.rb new file mode 100644 index 00000000000000..1431b0e576e0b2 --- /dev/null +++ b/ee/app/models/ee/integrations/base/integration.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module EE + module Integrations + module Base + module Integration + extend ActiveSupport::Concern + + prepended do + scope :vulnerability_hooks, -> { where(vulnerability_events: true, active: true) } + end + + EE_INTEGRATION_NAMES = %w[ + google_cloud_platform_workload_identity_federation + git_guardian + ].freeze + + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES = %w[ + github + google_cloud_platform_artifact_registry + ].freeze + + GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES = %w[ + google_cloud_platform_artifact_registry + google_cloud_platform_workload_identity_federation + ].freeze + + class_methods do + extend ::Gitlab::Utils::Override + + override :integration_names + def integration_names + names = super + EE_INTEGRATION_NAMES + + unless ::Gitlab::Saas.feature_available?(:google_cloud_support) + names.delete('google_cloud_platform_workload_identity_federation') + end + + names.delete('git_guardian') unless ::Feature.enabled?(:git_guardian_integration) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Legacy use + + names + end + + override :project_specific_integration_names + def project_specific_integration_names + names = super + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES + + unless ::Gitlab::Saas.feature_available?(:google_cloud_support) + names.delete('google_cloud_platform_artifact_registry') + end + + names + end + + # Returns the STI type for the given integration name. + # Example: "asana" => "Integrations::Asana" + override :integration_name_to_type + def integration_name_to_type(name) + name = name.to_s + + if GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES.include?(name) + name = name.delete_prefix("google_cloud_platform_") + "Integrations::GoogleCloudPlatform::#{name.camelize}" + else + super + end + end + end + end + end + end +end -- GitLab From 852b34f2f999a1508309e0b103fc044ee5d7f52b Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 8 Nov 2024 16:05:18 +0000 Subject: [PATCH 6/7] Update CI & monitoring integrations to use shared modules In order to support the split of integrations between instance/non-instance we need to use shared functionality via modules. For this, we need to update existing CI & monitoring integrations to use shared modules. Changelog: other --- .rubocop_todo/layout/class_structure.yml | 3 - .../layout/line_continuation_spacing.yml | 1 - ...e_end_string_concatenation_indentation.yml | 1 - .rubocop_todo/layout/line_length.yml | 1 - .../string_identifier_argument.yml | 1 - .rubocop_todo/rspec/any_instance_of.yml | 1 - .rubocop_todo/rspec/context_wording.yml | 1 - .../rspec/example_without_description.yml | 1 - .rubocop_todo/style/guard_clause.yml | 2 - .../style/inline_disable_annotation.yml | 2 - .rubocop_todo/style/redundant_self.yml | 3 - .../integrations/slash_commands_controller.rb | 5 +- app/models/concerns/integrations/base/ci.rb | 54 ++++++ .../concerns/integrations/base/integration.rb | 14 +- .../integrations/base/issue_tracker.rb | 171 ++++++++++++++++++ .../concerns/integrations/base/monitoring.rb | 31 ++++ .../integrations/base/slash_commands.rb | 79 ++++++++ .../integrations/base/third_party_wiki.rb | 48 +++++ .../concerns/mentionable/reference_regexes.rb | 2 +- app/models/integrations/bamboo.rb | 3 +- app/models/integrations/base_ci.rb | 44 ----- app/models/integrations/base_issue_tracker.rb | 157 ---------------- app/models/integrations/base_monitoring.rb | 23 --- .../integrations/base_slash_commands.rb | 72 -------- .../integrations/base_third_party_wiki.rb | 41 ----- app/models/integrations/bugzilla.rb | 3 +- app/models/integrations/buildkite.rb | 3 +- app/models/integrations/clickup.rb | 3 +- app/models/integrations/confluence.rb | 4 +- .../integrations/custom_issue_tracker.rb | 3 +- app/models/integrations/drone_ci.rb | 3 +- app/models/integrations/ewm.rb | 3 +- .../integrations/gitlab_slack_application.rb | 3 +- app/models/integrations/jenkins.rb | 3 +- app/models/integrations/jira.rb | 3 +- app/models/integrations/mattermost.rb | 1 + .../integrations/mattermost_slash_commands.rb | 3 +- app/models/integrations/mock_ci.rb | 3 +- app/models/integrations/mock_monitoring.rb | 4 +- app/models/integrations/phorge.rb | 3 +- app/models/integrations/prometheus.rb | 3 +- app/models/integrations/redmine.rb | 3 +- app/models/integrations/slack.rb | 1 + .../integrations/slack_slash_commands.rb | 3 +- app/models/integrations/teamcity.rb | 3 +- app/models/integrations/youtrack.rb | 3 +- app/models/integrations/zentao.rb | 3 +- db/docs/integrations.yml | 5 - .../ee/integrations/base/integration.rb | 72 -------- spec/factories/integrations.rb | 8 +- .../count/reltuples_count_strategy_spec.rb | 4 +- .../integrations/base/issue_tracker_spec.rb} | 16 +- spec/models/integrations/bamboo_spec.rb | 2 +- spec/models/integrations/buildkite_spec.rb | 2 +- spec/models/integrations/drone_ci_spec.rb | 2 +- spec/models/integrations/jenkins_spec.rb | 2 +- .../mattermost_slash_commands_spec.rb | 2 +- spec/models/integrations/mock_ci_spec.rb | 4 +- spec/models/integrations/prometheus_spec.rb | 2 +- .../integrations/slack_slash_commands_spec.rb | 2 +- spec/models/integrations/teamcity_spec.rb | 2 +- .../integrations/base/ci_shared_examples.rb} | 2 +- .../base/monitoring_shared_examples.rb} | 2 +- .../base/slash_commands_shared_examples.rb} | 14 +- 64 files changed, 471 insertions(+), 497 deletions(-) create mode 100644 app/models/concerns/integrations/base/ci.rb create mode 100644 app/models/concerns/integrations/base/issue_tracker.rb create mode 100644 app/models/concerns/integrations/base/monitoring.rb create mode 100644 app/models/concerns/integrations/base/slash_commands.rb create mode 100644 app/models/concerns/integrations/base/third_party_wiki.rb delete mode 100644 app/models/integrations/base_ci.rb delete mode 100644 app/models/integrations/base_issue_tracker.rb delete mode 100644 app/models/integrations/base_monitoring.rb delete mode 100644 app/models/integrations/base_slash_commands.rb delete mode 100644 app/models/integrations/base_third_party_wiki.rb delete mode 100644 ee/app/models/ee/integrations/base/integration.rb rename spec/models/{integrations/base_issue_tracker_spec.rb => concerns/integrations/base/issue_tracker_spec.rb} (78%) rename spec/support/shared_examples/models/{integrations/base_ci_shared_examples.rb => concerns/integrations/base/ci_shared_examples.rb} (71%) rename spec/support/shared_examples/models/{integrations/base_monitoring_shared_examples.rb => concerns/integrations/base/monitoring_shared_examples.rb} (69%) rename spec/support/shared_examples/models/{integrations/base_slash_commands_shared_examples.rb => concerns/integrations/base/slash_commands_shared_examples.rb} (91%) diff --git a/.rubocop_todo/layout/class_structure.yml b/.rubocop_todo/layout/class_structure.yml index 615306df72dcc5..e6054eb5e88e27 100644 --- a/.rubocop_todo/layout/class_structure.yml +++ b/.rubocop_todo/layout/class_structure.yml @@ -69,9 +69,6 @@ Layout/ClassStructure: - 'app/models/gpg_key.rb' - 'app/models/hooks/web_hook.rb' - 'app/models/identity.rb' - - 'app/models/integrations/base_ci.rb' - - 'app/models/integrations/base_issue_tracker.rb' - - 'app/models/integrations/base_slash_commands.rb' - 'app/models/integrations/buildkite.rb' - 'app/models/integrations/clickup.rb' - 'app/models/integrations/confluence.rb' diff --git a/.rubocop_todo/layout/line_continuation_spacing.yml b/.rubocop_todo/layout/line_continuation_spacing.yml index ddfa81c64c4265..0d1c22b5738977 100644 --- a/.rubocop_todo/layout/line_continuation_spacing.yml +++ b/.rubocop_todo/layout/line_continuation_spacing.yml @@ -9,7 +9,6 @@ Layout/LineContinuationSpacing: - 'app/helpers/tags_helper.rb' - 'app/helpers/tree_helper.rb' - 'app/models/concerns/spammable.rb' - - 'app/models/integrations/base_third_party_wiki.rb' - 'app/models/integrations/teamcity.rb' - 'app/models/work_items/parent_link.rb' - 'app/services/feature_flags/update_service.rb' diff --git a/.rubocop_todo/layout/line_end_string_concatenation_indentation.yml b/.rubocop_todo/layout/line_end_string_concatenation_indentation.yml index 2eb541cf8c2876..ffa710adbb4480 100644 --- a/.rubocop_todo/layout/line_end_string_concatenation_indentation.yml +++ b/.rubocop_todo/layout/line_end_string_concatenation_indentation.yml @@ -15,7 +15,6 @@ Layout/LineEndStringConcatenationIndentation: - 'app/models/concerns/spammable.rb' - 'app/models/concerns/taskable.rb' - 'app/models/integrations/bamboo.rb' - - 'app/models/integrations/base_third_party_wiki.rb' - 'app/models/integrations/diffblue_cover.rb' - 'app/models/integrations/gitlab_slack_application.rb' - 'app/models/integrations/hangouts_chat.rb' diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index bd6600e325efe0..7bf12778f7ccc8 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -156,7 +156,6 @@ Layout/LineLength: - 'app/models/incident_management/project_incident_management_setting.rb' - 'app/models/instance_configuration.rb' - 'app/models/integrations/asana.rb' - - 'app/models/integrations/base_issue_tracker.rb' - 'app/models/integrations/chat_message/merge_message.rb' - 'app/models/integrations/chat_message/note_message.rb' - 'app/models/integrations/chat_message/pipeline_message.rb' diff --git a/.rubocop_todo/performance/string_identifier_argument.yml b/.rubocop_todo/performance/string_identifier_argument.yml index de4492b95452eb..c6cf705eb7a9f9 100644 --- a/.rubocop_todo/performance/string_identifier_argument.yml +++ b/.rubocop_todo/performance/string_identifier_argument.yml @@ -21,7 +21,6 @@ Performance/StringIdentifierArgument: - 'app/models/concerns/sanitizable.rb' - 'app/models/concerns/signature_type.rb' - 'app/models/concerns/token_authenticatable.rb' - - 'app/models/integrations/base_third_party_wiki.rb' - 'app/models/integrations/field.rb' - 'app/models/namespace_statistics.rb' - 'app/models/packages/debian/file_entry.rb' diff --git a/.rubocop_todo/rspec/any_instance_of.yml b/.rubocop_todo/rspec/any_instance_of.yml index ac22761ac473a9..443788bc1647ad 100644 --- a/.rubocop_todo/rspec/any_instance_of.yml +++ b/.rubocop_todo/rspec/any_instance_of.yml @@ -289,7 +289,6 @@ RSpec/AnyInstanceOf: - 'spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb' - 'spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb' - 'spec/support/shared_examples/models/diff_note_after_commit_shared_examples.rb' - - 'spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb' - 'spec/support/shared_examples/models/mentionable_shared_examples.rb' - 'spec/support/shared_examples/models/with_uploads_shared_examples.rb' - 'spec/support/shared_examples/requests/api/discussions_shared_examples.rb' diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml index a9c27e1e4b66cb..44852f41735aa4 100644 --- a/.rubocop_todo/rspec/context_wording.yml +++ b/.rubocop_todo/rspec/context_wording.yml @@ -2718,7 +2718,6 @@ RSpec/ContextWording: - 'spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb' - 'spec/support/shared_examples/models/cycle_analytics_stage_shared_examples.rb' - 'spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb' - - 'spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb' - 'spec/support/shared_examples/models/mentionable_shared_examples.rb' - 'spec/support/shared_examples/models/packages/debian/component_file_shared_example.rb' - 'spec/support/shared_examples/models/project_latest_successful_build_for_shared_examples.rb' diff --git a/.rubocop_todo/rspec/example_without_description.yml b/.rubocop_todo/rspec/example_without_description.yml index 44ae7dc6868cbf..9223560de1d54b 100644 --- a/.rubocop_todo/rspec/example_without_description.yml +++ b/.rubocop_todo/rspec/example_without_description.yml @@ -576,7 +576,6 @@ RSpec/ExampleWithoutDescription: - 'spec/support/shared_examples/mailers/notify_shared_examples.rb' - 'spec/support/shared_examples/models/concerns/linkable_items_shared_examples.rb' - 'spec/support/shared_examples/models/concerns/protected_ref_access_shared_examples.rb' - - 'spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb' - 'spec/support/shared_examples/models/issuable_link_shared_examples.rb' - 'spec/support/shared_examples/models/packages/debian/component_file_shared_example.rb' - 'spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb' diff --git a/.rubocop_todo/style/guard_clause.yml b/.rubocop_todo/style/guard_clause.yml index 45626acaa05d10..42e551789da4af 100644 --- a/.rubocop_todo/style/guard_clause.yml +++ b/.rubocop_todo/style/guard_clause.yml @@ -81,8 +81,6 @@ Style/GuardClause: - 'app/models/error_tracking/project_error_tracking_setting.rb' - 'app/models/generic_commit_status.rb' - 'app/models/grafana_integration.rb' - - 'app/models/integrations/base_issue_tracker.rb' - - 'app/models/integrations/base_third_party_wiki.rb' - 'app/models/integrations/confluence.rb' - 'app/models/integrations/datadog.rb' - 'app/models/integrations/emails_on_push.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index 75d609eee41f49..c1ecdb4fd801f9 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -403,8 +403,6 @@ Style/InlineDisableAnnotation: - 'app/models/group_deploy_key.rb' - 'app/models/hooks/web_hook.rb' - 'app/models/hooks/web_hook_log.rb' - - 'app/models/integrations/base_slash_commands.rb' - - 'app/models/integrations/base_third_party_wiki.rb' - 'app/models/integrations/google_play.rb' - 'app/models/issue.rb' - 'app/models/key.rb' diff --git a/.rubocop_todo/style/redundant_self.yml b/.rubocop_todo/style/redundant_self.yml index 97ed6716fcfcaa..c38a0e7b103b29 100644 --- a/.rubocop_todo/style/redundant_self.yml +++ b/.rubocop_todo/style/redundant_self.yml @@ -75,9 +75,6 @@ Style/RedundantSelf: - 'app/models/group_group_link.rb' - 'app/models/hooks/web_hook_log.rb' - 'app/models/identity.rb' - - 'app/models/integrations/base_ci.rb' - - 'app/models/integrations/base_issue_tracker.rb' - - 'app/models/integrations/base_slash_commands.rb' - 'app/models/integrations/emails_on_push.rb' - 'app/models/integrations/pipelines_email.rb' - 'app/models/integrations/zentao.rb' diff --git a/app/controllers/projects/integrations/slash_commands_controller.rb b/app/controllers/projects/integrations/slash_commands_controller.rb index 891a7c1a7497bd..bd3682141509c1 100644 --- a/app/controllers/projects/integrations/slash_commands_controller.rb +++ b/app/controllers/projects/integrations/slash_commands_controller.rb @@ -41,7 +41,10 @@ def cached_params end def cache_key - @cache_key ||= Kernel.format(::Integrations::BaseSlashCommands::CACHE_KEY, secret: request_params[:command_id]) + @cache_key ||= Kernel.format( + ::Integrations::Base::SlashCommands::CACHE_KEY, + secret: request_params[:command_id] + ) end def integration diff --git a/app/models/concerns/integrations/base/ci.rb b/app/models/concerns/integrations/base/ci.rb new file mode 100644 index 00000000000000..5393c656bff81a --- /dev/null +++ b/app/models/concerns/integrations/base/ci.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# Base module for CI integrations +# List methods you need to implement to get your CI integration +# working with GitLab merge requests +module Integrations + module Base + module Ci + extend ActiveSupport::Concern + + class_methods do + def supported_events + %w[push] + end + end + + included do + attribute :category, default: 'ci' + end + + def valid_token?(token) + respond_to?(:token) && + self.token.present? && + ActiveSupport::SecurityUtils.secure_compare(token, self.token) + end + + # Return complete url to build page + # + # Ex. + # http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c + # + def build_page(sha, ref) + # implement inside child + end + + # Return string with build status or :error symbol + # + # Allowed states: 'success', 'failed', 'running', 'pending', 'skipped' + # + # + # Ex. + # @integration.commit_status('13be4ac', 'master') + # # => 'success' + # + # @integration.commit_status('2abe4ac', 'dev') + # # => 'running' + # + # + def commit_status(sha, ref) + # implement inside child + end + end + end +end diff --git a/app/models/concerns/integrations/base/integration.rb b/app/models/concerns/integrations/base/integration.rb index b73f4e33920f0d..2fb3a3757b0452 100644 --- a/app/models/concerns/integrations/base/integration.rb +++ b/app/models/concerns/integrations/base/integration.rb @@ -38,13 +38,7 @@ module Integration ].freeze # Base classes which aren't actual integrations. - BASE_CLASSES = %w[ - Integrations::BaseCi - Integrations::BaseIssueTracker - Integrations::BaseMonitoring - Integrations::BaseSlashCommands - Integrations::BaseThirdPartyWiki - ].freeze + BASE_CLASSES = %w[].freeze BASE_ATTRIBUTES = %w[id instance project_id group_id created_at updated_at encrypted_properties encrypted_properties_iv properties].freeze @@ -817,12 +811,6 @@ def validate_belongs_to_project_or_group def validate_recipients? activated? && !importing? end - - class_methods do - def supported_events - %w[commit push tag_push issue confidential_issue merge_request wiki_page] - end - end end end end diff --git a/app/models/concerns/integrations/base/issue_tracker.rb b/app/models/concerns/integrations/base/issue_tracker.rb new file mode 100644 index 00000000000000..620cc17f0d864a --- /dev/null +++ b/app/models/concerns/integrations/base/issue_tracker.rb @@ -0,0 +1,171 @@ +# frozen_string_literal: true + +module Integrations + module Base + module IssueTracker + extend ActiveSupport::Concern + + REFERENCE_PATTERN_LONG_REGEXP = /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ + REFERENCE_PATTERN_REGEXP = /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ + + included do + validate :one_issue_tracker, if: :activated?, on: :manual_change + + attribute :category, default: 'issue_tracker' + + before_validation :handle_properties + before_validation :set_default_data, on: :create + end + + class_methods do + def supported_events + %w[push] + end + + # Pattern used to extract links from comments + # Override this method on services that uses different patterns + # This pattern does not support cross-project references + # The other code assumes that this pattern is a superset of all + # overridden patterns. See ReferenceRegexes.external_pattern + def base_reference_pattern(only_long: false) + return REFERENCE_PATTERN_LONG_REGEXP if only_long + + REFERENCE_PATTERN_REGEXP + end + end + + def reference_pattern(only_long: false) + self.class.base_reference_pattern(only_long: only_long) + end + + def handle_properties + # this has been moved from initialize_properties and should be improved + # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 + return unless properties.present? + + safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] + + @legacy_properties_data = properties.dup # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Legacy use + + data_values = properties.slice(*safe_keys) + data_values.reject! { |key| data_fields.changed.include?(key) } + + data_fields.assign_attributes(data_values) if data_values.present? + + self.properties = {} + end + + def legacy_properties_data + @legacy_properties_data ||= {} + end + + def supports_data_fields? + true + end + + def data_fields + issue_tracker_data || build_issue_tracker_data + end + + def default? + default + end + + def issue_url(iid) + issues_url.gsub(':id', iid.to_s) + end + + def issue_tracker_path + project_url + end + + def new_issue_path + new_issue_url + end + + def issue_path(iid) + issue_url(iid) + end + + # Initialize with default properties values + def set_default_data + return unless issues_tracker.present? + + # we don't want to override if we have set something + return if project_url || issues_url || new_issue_url + + data_fields.project_url = issues_tracker['project_url'] + data_fields.issues_url = issues_tracker['issues_url'] + data_fields.new_issue_url = issues_tracker['new_issue_url'] + end + + def execute(data) + return unless supported_events.include?(data[:object_kind]) + + message = "#{type} was unable to reach #{project_url}. Check the url and try again." + result = false + + begin + response = Gitlab::HTTP.head(project_url, verify: true) + + if response + message = "#{type} received response #{response.code} when attempting to connect to #{project_url}" + result = true + end + rescue Gitlab::HTTP::Error, + Timeout::Error, + SocketError, + Errno::ECONNRESET, + Errno::ECONNREFUSED, + OpenSSL::SSL::SSLError => e + message = "#{type} had an error when trying to connect to #{project_url}: #{e.message}" + end + log_info(message) + result + end + + def support_close_issue? + false + end + + def support_cross_reference? + false + end + + def create_cross_reference_note(external_issue, mentioned_in, author) + # implement inside child + end + + def activate_disabled_reason + { trackers: other_external_issue_trackers } if other_external_issue_trackers.any? + end + + private + + def other_external_issue_trackers + return [] unless project_level? + + @other_external_issue_trackers ||= project.integrations.external_issue_trackers.where.not(id: id) # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Legacy use + end + + def enabled_in_gitlab_config + Gitlab.config.issues_tracker && + Gitlab.config.issues_tracker.values.any? && + issues_tracker + end + + def issues_tracker + Gitlab.config.issues_tracker[to_param] + end + + def one_issue_tracker + return if instance? + return if project.blank? + return unless other_external_issue_trackers.any? + + errors.add(:base, _('Another issue tracker is already in use. ' \ + 'Only one issue tracker service can be active at a time')) + end + end + end +end diff --git a/app/models/concerns/integrations/base/monitoring.rb b/app/models/concerns/integrations/base/monitoring.rb new file mode 100644 index 00000000000000..7d2fea6be01ff4 --- /dev/null +++ b/app/models/concerns/integrations/base/monitoring.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Base module for monitoring services +# +# These services integrate with a deployment solution like Prometheus +# to provide additional features for environments. +module Integrations + module Base + module Monitoring + extend ActiveSupport::Concern + + class_methods do + def supported_events + %w[] + end + end + + included do + attribute :category, default: 'monitoring' + end + + def can_query? + raise NotImplementedError + end + + def query(_, *_) + raise NotImplementedError + end + end + end +end diff --git a/app/models/concerns/integrations/base/slash_commands.rb b/app/models/concerns/integrations/base/slash_commands.rb new file mode 100644 index 00000000000000..7be60585483b5d --- /dev/null +++ b/app/models/concerns/integrations/base/slash_commands.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +# Base module for ChatOps integrations +module Integrations + module Base + module SlashCommands + extend ActiveSupport::Concern + + CACHE_KEY = "slash-command-requests:%{secret}" + CACHE_EXPIRATION_TIME = 3.minutes + + class_methods do + def supported_events + %w[] + end + end + + included do + attribute :category, default: 'chat' + end + + def trigger(params) + return unless valid_token?(params[:token]) + + chat_user = find_chat_user(params) + user = chat_user&.user + + return unknown_user_message(params) unless user + + unless user.can?(:use_slash_commands) + return Gitlab::SlashCommands::Presenters::Access.new.deactivated if user.deactivated? + + return Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) + end + + if Gitlab::SlashCommands::VerifyRequest.new(self, chat_user).valid? + Gitlab::SlashCommands::Command.new(project, chat_user, params).execute + else + command_id = cache_slash_commands_request!(params) + Gitlab::SlashCommands::Presenters::Access.new.confirm(confirmation_url(command_id, params)) + end + end + + def valid_token?(token) + respond_to?(:token) && + self.token.present? && + ActiveSupport::SecurityUtils.secure_compare(token, self.token) + end + + def testable? + false + end + + private + + def find_chat_user(params) + ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute # rubocop: disable CodeReuse/ServiceClass -- Legacy use + end + + def authorize_chat_name_url(params) + ChatNames::AuthorizeUserService.new(params).execute # rubocop: disable CodeReuse/ServiceClass -- Legacy use + end + + def unknown_user_message(params) + url = authorize_chat_name_url(params) + Gitlab::SlashCommands::Presenters::Access.new(url).authorize + end + + def cache_slash_commands_request!(params) + secret = SecureRandom.uuid + Kernel.format(CACHE_KEY, secret: secret).tap do |cache_key| + Rails.cache.write(cache_key, params, expires_in: CACHE_EXPIRATION_TIME) + end + + secret + end + end + end +end diff --git a/app/models/concerns/integrations/base/third_party_wiki.rb b/app/models/concerns/integrations/base/third_party_wiki.rb new file mode 100644 index 00000000000000..37d60a2c695ef7 --- /dev/null +++ b/app/models/concerns/integrations/base/third_party_wiki.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Integrations + module Base + module ThirdPartyWiki + extend ActiveSupport::Concern + include SafeFormatHelper + + class_methods do + def supported_events + %w[] + end + end + + included do + attribute :category, default: 'third_party_wiki' + + validate :only_one_third_party_wiki, if: :activated?, on: :manual_change + + after_commit :cache_project_has_integration + end + + private + + def only_one_third_party_wiki + return unless project_level? + + return unless project.integrations.third_party_wikis.id_not_in(id).any? + + errors.add(:base, _('Another third-party wiki is already in use. ' \ + 'Only one third-party wiki integration can be active at a time')) + end + + def cache_project_has_integration + return unless project && !project.destroyed? + + project_setting = project.project_setting + + project_setting.public_send(:"#{project_settings_cache_key}=", active?) # rubocop:disable GitlabSecurity/PublicSend -- Legacy use + project_setting.save! + end + + def project_settings_cache_key + "has_#{self.class.to_param}" + end + end + end +end diff --git a/app/models/concerns/mentionable/reference_regexes.rb b/app/models/concerns/mentionable/reference_regexes.rb index b5634ba3b6d07b..755060e0ff6c4b 100644 --- a/app/models/concerns/mentionable/reference_regexes.rb +++ b/app/models/concerns/mentionable/reference_regexes.rb @@ -27,7 +27,7 @@ def self.default_pattern def self.external_pattern strong_memoize(:external_pattern) do - issue_pattern = Integrations::BaseIssueTracker.base_reference_pattern + issue_pattern = Integrations::Base::IssueTracker::REFERENCE_PATTERN_REGEXP link_patterns = URI::DEFAULT_PARSER.make_regexp(%w[http https]) reference_pattern(link_patterns, issue_pattern) end diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index f84a5682cd68bf..2ad573ad3bc66e 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Bamboo < BaseCi + class Bamboo < Integration + include Base::Ci include ReactivelyCached prepend EnableSslVerification diff --git a/app/models/integrations/base_ci.rb b/app/models/integrations/base_ci.rb deleted file mode 100644 index db29f228e60073..00000000000000 --- a/app/models/integrations/base_ci.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -# Base class for CI integrations -# List methods you need to implement to get your CI integration -# working with GitLab merge requests -module Integrations - class BaseCi < Integration - attribute :category, default: 'ci' - - def valid_token?(token) - self.respond_to?(:token) && self.token.present? && ActiveSupport::SecurityUtils.secure_compare(token, self.token) - end - - def self.supported_events - %w[push] - end - - # Return complete url to build page - # - # Ex. - # http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c - # - def build_page(sha, ref) - # implement inside child - end - - # Return string with build status or :error symbol - # - # Allowed states: 'success', 'failed', 'running', 'pending', 'skipped' - # - # - # Ex. - # @integration.commit_status('13be4ac', 'master') - # # => 'success' - # - # @integration.commit_status('2abe4ac', 'dev') - # # => 'running' - # - # - def commit_status(sha, ref) - # implement inside child - end - end -end diff --git a/app/models/integrations/base_issue_tracker.rb b/app/models/integrations/base_issue_tracker.rb deleted file mode 100644 index b59aee6743df27..00000000000000 --- a/app/models/integrations/base_issue_tracker.rb +++ /dev/null @@ -1,157 +0,0 @@ -# frozen_string_literal: true - -module Integrations - class BaseIssueTracker < Integration - validate :one_issue_tracker, if: :activated?, on: :manual_change - - attribute :category, default: 'issue_tracker' - - before_validation :handle_properties - before_validation :set_default_data, on: :create - - # Pattern used to extract links from comments - # Override this method on services that uses different patterns - # This pattern does not support cross-project references - # The other code assumes that this pattern is a superset of all - # overridden patterns. See ReferenceRegexes.external_pattern - def self.base_reference_pattern(only_long: false) - if only_long - /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/ - else - /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/ - end - end - - def reference_pattern(only_long: false) - self.class.base_reference_pattern(only_long: only_long) - end - - def handle_properties - # this has been moved from initialize_properties and should be improved - # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 - return unless properties.present? - - safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at] - - @legacy_properties_data = properties.dup - - data_values = properties.slice(*safe_keys) - data_values.reject! { |key| data_fields.changed.include?(key) } - - data_fields.assign_attributes(data_values) if data_values.present? - - self.properties = {} - end - - def legacy_properties_data - @legacy_properties_data ||= {} - end - - def supports_data_fields? - true - end - - def data_fields - issue_tracker_data || self.build_issue_tracker_data - end - - def default? - default - end - - def issue_url(iid) - issues_url.gsub(':id', iid.to_s) - end - - def issue_tracker_path - project_url - end - - def new_issue_path - new_issue_url - end - - def issue_path(iid) - issue_url(iid) - end - - # Initialize with default properties values - def set_default_data - return unless issues_tracker.present? - - # we don't want to override if we have set something - return if project_url || issues_url || new_issue_url - - data_fields.project_url = issues_tracker['project_url'] - data_fields.issues_url = issues_tracker['issues_url'] - data_fields.new_issue_url = issues_tracker['new_issue_url'] - end - - def self.supported_events - %w[push] - end - - def execute(data) - return unless supported_events.include?(data[:object_kind]) - - message = "#{self.type} was unable to reach #{self.project_url}. Check the url and try again." - result = false - - begin - response = Gitlab::HTTP.head(self.project_url, verify: true) - - if response - message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}" - result = true - end - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, OpenSSL::SSL::SSLError => e - message = "#{self.type} had an error when trying to connect to #{self.project_url}: #{e.message}" - end - log_info(message) - result - end - - def support_close_issue? - false - end - - def support_cross_reference? - false - end - - def create_cross_reference_note(external_issue, mentioned_in, author) - # implement inside child - end - - def activate_disabled_reason - { trackers: other_external_issue_trackers } if other_external_issue_trackers.any? - end - - private - - def other_external_issue_trackers - return [] unless project_level? - - @other_external_issue_trackers ||= project.integrations.external_issue_trackers.where.not(id: id) - end - - def enabled_in_gitlab_config - Gitlab.config.issues_tracker && - Gitlab.config.issues_tracker.values.any? && - issues_tracker - end - - def issues_tracker - Gitlab.config.issues_tracker[to_param] - end - - def one_issue_tracker - return if instance? - return if project.blank? - - if other_external_issue_trackers.any? - errors.add(:base, _('Another issue tracker is already in use. Only one issue tracker service can be active at a time')) - end - end - end -end diff --git a/app/models/integrations/base_monitoring.rb b/app/models/integrations/base_monitoring.rb deleted file mode 100644 index 12ea57f59a3d8c..00000000000000 --- a/app/models/integrations/base_monitoring.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -# Base class for monitoring services -# -# These services integrate with a deployment solution like Prometheus -# to provide additional features for environments. -module Integrations - class BaseMonitoring < Integration - attribute :category, default: 'monitoring' - - def self.supported_events - %w[] - end - - def can_query? - raise NotImplementedError - end - - def query(_, *_) - raise NotImplementedError - end - end -end diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb deleted file mode 100644 index f477263303f8b1..00000000000000 --- a/app/models/integrations/base_slash_commands.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -# Base class for ChatOps integrations -# This class is not meant to be used directly, but only to inherrit from. -module Integrations - class BaseSlashCommands < Integration - CACHE_KEY = "slash-command-requests:%{secret}" - CACHE_EXPIRATION_TIME = 3.minutes - - attribute :category, default: 'chat' - - def valid_token?(token) - self.respond_to?(:token) && - self.token.present? && - ActiveSupport::SecurityUtils.secure_compare(token, self.token) - end - - def self.supported_events - %w[] - end - - def testable? - false - end - - def trigger(params) - return unless valid_token?(params[:token]) - - chat_user = find_chat_user(params) - user = chat_user&.user - - return unknown_user_message(params) unless user - - unless user.can?(:use_slash_commands) - return Gitlab::SlashCommands::Presenters::Access.new.deactivated if user.deactivated? - - return Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) - end - - if Gitlab::SlashCommands::VerifyRequest.new(self, chat_user).valid? - Gitlab::SlashCommands::Command.new(project, chat_user, params).execute - else - command_id = cache_slash_commands_request!(params) - Gitlab::SlashCommands::Presenters::Access.new.confirm(confirmation_url(command_id, params)) - end - end - - private - - def find_chat_user(params) - ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute # rubocop: disable CodeReuse/ServiceClass - end - - def authorize_chat_name_url(params) - ChatNames::AuthorizeUserService.new(params).execute # rubocop: disable CodeReuse/ServiceClass - end - - def unknown_user_message(params) - url = authorize_chat_name_url(params) - Gitlab::SlashCommands::Presenters::Access.new(url).authorize - end - - def cache_slash_commands_request!(params) - secret = SecureRandom.uuid - Kernel.format(CACHE_KEY, secret: secret).tap do |cache_key| - Rails.cache.write(cache_key, params, expires_in: CACHE_EXPIRATION_TIME) - end - - secret - end - end -end diff --git a/app/models/integrations/base_third_party_wiki.rb b/app/models/integrations/base_third_party_wiki.rb deleted file mode 100644 index 27e324fb301267..00000000000000 --- a/app/models/integrations/base_third_party_wiki.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -module Integrations - class BaseThirdPartyWiki < Integration - include SafeFormatHelper - - attribute :category, default: 'third_party_wiki' - - validate :only_one_third_party_wiki, if: :activated?, on: :manual_change - - after_commit :cache_project_has_integration - - def self.supported_events - %w[] - end - - private - - def only_one_third_party_wiki - return unless project_level? - - if project.integrations.third_party_wikis.id_not_in(id).any? - errors.add(:base, _('Another third-party wiki is already in use. '\ - 'Only one third-party wiki integration can be active at a time')) - end - end - - def cache_project_has_integration - return unless project && !project.destroyed? - - project_setting = project.project_setting - - project_setting.public_send("#{project_settings_cache_key}=", active?) # rubocop:disable GitlabSecurity/PublicSend - project_setting.save! - end - - def project_settings_cache_key - "has_#{self.class.to_param}" - end - end -end diff --git a/app/models/integrations/bugzilla.rb b/app/models/integrations/bugzilla.rb index d5083fe7c8528b..fd3a4e8f2b8098 100644 --- a/app/models/integrations/bugzilla.rb +++ b/app/models/integrations/bugzilla.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Bugzilla < BaseIssueTracker + class Bugzilla < Integration + include Base::IssueTracker include Integrations::HasIssueTrackerFields include HasAvatar diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index be0a0785bb0808..a573422a5a2eaa 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -3,7 +3,8 @@ require "addressable/uri" module Integrations - class Buildkite < BaseCi + class Buildkite < Integration + include Base::Ci include HasWebHook include ReactivelyCached include HasAvatar diff --git a/app/models/integrations/clickup.rb b/app/models/integrations/clickup.rb index 6f88c8f95a8eca..daf3147180f828 100644 --- a/app/models/integrations/clickup.rb +++ b/app/models/integrations/clickup.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Clickup < BaseIssueTracker + class Clickup < Integration + include Base::IssueTracker include HasIssueTrackerFields include HasAvatar diff --git a/app/models/integrations/confluence.rb b/app/models/integrations/confluence.rb index ae5596c89c7303..24656a0d767867 100644 --- a/app/models/integrations/confluence.rb +++ b/app/models/integrations/confluence.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true module Integrations - class Confluence < BaseThirdPartyWiki + class Confluence < Integration + include Base::ThirdPartyWiki + VALID_SCHEME_MATCH = %r{\Ahttps?\Z} VALID_HOST_MATCH = %r{\A.+\.atlassian\.net\Z} VALID_PATH_MATCH = %r{\A/wiki(/|\Z)} diff --git a/app/models/integrations/custom_issue_tracker.rb b/app/models/integrations/custom_issue_tracker.rb index 7ce4eb08496d94..a489c15b98ac20 100644 --- a/app/models/integrations/custom_issue_tracker.rb +++ b/app/models/integrations/custom_issue_tracker.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class CustomIssueTracker < BaseIssueTracker + class CustomIssueTracker < Integration + include Base::IssueTracker include HasIssueTrackerFields validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index a5be640cfcebeb..d7e7356bb55266 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class DroneCi < BaseCi + class DroneCi < Integration + include Base::Ci include HasWebHook include HasAvatar include PushDataValidations diff --git a/app/models/integrations/ewm.rb b/app/models/integrations/ewm.rb index 085b82d4c1003c..736fd140b085c3 100644 --- a/app/models/integrations/ewm.rb +++ b/app/models/integrations/ewm.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Ewm < BaseIssueTracker + class Ewm < Integration + include Base::IssueTracker include HasIssueTrackerFields validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? diff --git a/app/models/integrations/gitlab_slack_application.rb b/app/models/integrations/gitlab_slack_application.rb index 77ff9504e7210f..2ed5a38333a458 100644 --- a/app/models/integrations/gitlab_slack_application.rb +++ b/app/models/integrations/gitlab_slack_application.rb @@ -88,8 +88,6 @@ def upgrade_needed? slack_integration.present? && slack_integration.upgrade_needed? end - def default_channel_placeholder; end - private override :notify @@ -162,6 +160,7 @@ def test_notification_channels ) end + override :metrics_key_prefix def metrics_key_prefix 'i_integrations_gitlab_for_slack_app' end diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index b7981f0be28ede..032ed7fce3cf35 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Jenkins < BaseCi + class Jenkins < Integration + include Base::Ci include HasWebHook prepend EnableSslVerification diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index b4c7c99ad8fdd5..8ed2dee95cf09a 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -2,7 +2,8 @@ # Accessible as Project#external_issue_tracker module Integrations - class Jira < BaseIssueTracker + class Jira < Integration + include Base::IssueTracker include Gitlab::Routing include ApplicationHelper include SafeFormatHelper diff --git a/app/models/integrations/mattermost.rb b/app/models/integrations/mattermost.rb index 37ea3d1a665591..ed9365f72a7437 100644 --- a/app/models/integrations/mattermost.rb +++ b/app/models/integrations/mattermost.rb @@ -33,6 +33,7 @@ def self.webhook_help 'http://mattermost.example.com/hooks/...' end + override :configurable_channels? def configurable_channels? true end diff --git a/app/models/integrations/mattermost_slash_commands.rb b/app/models/integrations/mattermost_slash_commands.rb index b43c779244531b..6292aed0901c05 100644 --- a/app/models/integrations/mattermost_slash_commands.rb +++ b/app/models/integrations/mattermost_slash_commands.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class MattermostSlashCommands < BaseSlashCommands + class MattermostSlashCommands < Integration + include Base::SlashCommands include Ci::TriggersHelper MATTERMOST_URL = '%{ORIGIN}/%{TEAM}/channels/%{CHANNEL}' diff --git a/app/models/integrations/mock_ci.rb b/app/models/integrations/mock_ci.rb index 8b9f2f0dd5dd14..53780eedc2fb15 100644 --- a/app/models/integrations/mock_ci.rb +++ b/app/models/integrations/mock_ci.rb @@ -2,7 +2,8 @@ # For an example companion mocking service, see https://gitlab.com/gitlab-org/gitlab-mock-ci-service module Integrations - class MockCi < BaseCi + class MockCi < Integration + include Base::Ci prepend EnableSslVerification ALLOWED_STATES = %w[failed canceled running pending success success-with-warnings skipped not_found].freeze diff --git a/app/models/integrations/mock_monitoring.rb b/app/models/integrations/mock_monitoring.rb index 9e474078b28b53..f3debecd967f80 100644 --- a/app/models/integrations/mock_monitoring.rb +++ b/app/models/integrations/mock_monitoring.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true module Integrations - class MockMonitoring < BaseMonitoring + class MockMonitoring < Integration + include Base::Monitoring + def self.title 'Mock monitoring' end diff --git a/app/models/integrations/phorge.rb b/app/models/integrations/phorge.rb index 959186be32cfd1..d6533ca301defa 100644 --- a/app/models/integrations/phorge.rb +++ b/app/models/integrations/phorge.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Phorge < BaseIssueTracker + class Phorge < Integration + include Base::IssueTracker include HasIssueTrackerFields include HasAvatar diff --git a/app/models/integrations/prometheus.rb b/app/models/integrations/prometheus.rb index 97dab35601867d..13a38545677390 100644 --- a/app/models/integrations/prometheus.rb +++ b/app/models/integrations/prometheus.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Prometheus < BaseMonitoring + class Prometheus < Integration + include Base::Monitoring include PrometheusAdapter include Gitlab::Utils::StrongMemoize diff --git a/app/models/integrations/redmine.rb b/app/models/integrations/redmine.rb index 4ac5249787aa5e..176fd18ffd06f6 100644 --- a/app/models/integrations/redmine.rb +++ b/app/models/integrations/redmine.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Redmine < BaseIssueTracker + class Redmine < Integration + include Base::IssueTracker include Integrations::HasIssueTrackerFields validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index 0c195b09dae57b..6495d2240a1e75 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -24,6 +24,7 @@ def self.webhook_help private + override :metrics_key_prefix def metrics_key_prefix 'i_ecosystem_slack_service' end diff --git a/app/models/integrations/slack_slash_commands.rb b/app/models/integrations/slack_slash_commands.rb index ff1ac22fee7dc7..2af1bbc599cb6f 100644 --- a/app/models/integrations/slack_slash_commands.rb +++ b/app/models/integrations/slack_slash_commands.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class SlackSlashCommands < BaseSlashCommands + class SlackSlashCommands < Integration + include Base::SlashCommands include Ci::TriggersHelper SLACK_REDIRECT_URL = 'slack://channel?team=%{TEAM}&id=%{CHANNEL}' diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index 5e88aaa0b49624..19e57e47569285 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Teamcity < BaseCi + class Teamcity < Integration + include Base::Ci include PushDataValidations include ReactivelyCached include HasAvatar diff --git a/app/models/integrations/youtrack.rb b/app/models/integrations/youtrack.rb index f2bfb75cf62b1a..7e751ed9209e67 100644 --- a/app/models/integrations/youtrack.rb +++ b/app/models/integrations/youtrack.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Youtrack < BaseIssueTracker + class Youtrack < Integration + include Base::IssueTracker include Integrations::HasIssueTrackerFields include HasAvatar diff --git a/app/models/integrations/zentao.rb b/app/models/integrations/zentao.rb index a16734fb6e7b20..5c9ff3c3740cf7 100644 --- a/app/models/integrations/zentao.rb +++ b/app/models/integrations/zentao.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module Integrations - class Zentao < BaseIssueTracker + class Zentao < Integration + include Base::IssueTracker include Gitlab::Routing self.field_storage = :data_fields diff --git a/db/docs/integrations.yml b/db/docs/integrations.yml index 4be931202e97e1..ac0956954e2ddf 100644 --- a/db/docs/integrations.yml +++ b/db/docs/integrations.yml @@ -6,11 +6,6 @@ classes: - Integrations::Asana - Integrations::Assembla - Integrations::Bamboo -- Integrations::BaseCi -- Integrations::BaseIssueTracker -- Integrations::BaseMonitoring -- Integrations::BaseSlashCommands -- Integrations::BaseThirdPartyWiki - Integrations::BeyondIdentity - Integrations::Bugzilla - Integrations::Buildkite diff --git a/ee/app/models/ee/integrations/base/integration.rb b/ee/app/models/ee/integrations/base/integration.rb deleted file mode 100644 index 1431b0e576e0b2..00000000000000 --- a/ee/app/models/ee/integrations/base/integration.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module EE - module Integrations - module Base - module Integration - extend ActiveSupport::Concern - - prepended do - scope :vulnerability_hooks, -> { where(vulnerability_events: true, active: true) } - end - - EE_INTEGRATION_NAMES = %w[ - google_cloud_platform_workload_identity_federation - git_guardian - ].freeze - - EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES = %w[ - github - google_cloud_platform_artifact_registry - ].freeze - - GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES = %w[ - google_cloud_platform_artifact_registry - google_cloud_platform_workload_identity_federation - ].freeze - - class_methods do - extend ::Gitlab::Utils::Override - - override :integration_names - def integration_names - names = super + EE_INTEGRATION_NAMES - - unless ::Gitlab::Saas.feature_available?(:google_cloud_support) - names.delete('google_cloud_platform_workload_identity_federation') - end - - names.delete('git_guardian') unless ::Feature.enabled?(:git_guardian_integration) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Legacy use - - names - end - - override :project_specific_integration_names - def project_specific_integration_names - names = super + EE_PROJECT_LEVEL_ONLY_INTEGRATION_NAMES - - unless ::Gitlab::Saas.feature_available?(:google_cloud_support) - names.delete('google_cloud_platform_artifact_registry') - end - - names - end - - # Returns the STI type for the given integration name. - # Example: "asana" => "Integrations::Asana" - override :integration_name_to_type - def integration_name_to_type(name) - name = name.to_s - - if GOOGLE_CLOUD_PLATFORM_INTEGRATION_NAMES.include?(name) - name = name.delete_prefix("google_cloud_platform_") - "Integrations::GoogleCloudPlatform::#{name.camelize}" - else - super - end - end - end - end - end - end -end diff --git a/spec/factories/integrations.rb b/spec/factories/integrations.rb index adbe228431065a..72ebefb9ae4142 100644 --- a/spec/factories/integrations.rb +++ b/spec/factories/integrations.rb @@ -509,14 +509,14 @@ issue_tracker_data { nil } create_data { false } - after(:build) do - Integrations::BaseIssueTracker.skip_callback(:validation, :before, :handle_properties) + after(:build) do |integration| + integration.class.skip_callback(:validation, :before, :handle_properties) end to_create { |instance| instance.save!(validate: false) } - after(:create) do - Integrations::BaseIssueTracker.set_callback(:validation, :before, :handle_properties) + after(:create) do |integration| + integration.class.set_callback(:validation, :before, :handle_properties) end end diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb index 8ec916e17f8ebb..9075db8ad79e64 100644 --- a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do +RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy, feature_category: :database do before do create_list(:project, 3) create_list(:ci_instance_variable, 2) @@ -27,7 +27,7 @@ end context 'when models using single-type inheritance are used' do - let(:models) { [Group, Integrations::BaseCi, Namespace] } + let(:models) { [Group, Namespace] } before do models.each do |model| diff --git a/spec/models/integrations/base_issue_tracker_spec.rb b/spec/models/concerns/integrations/base/issue_tracker_spec.rb similarity index 78% rename from spec/models/integrations/base_issue_tracker_spec.rb rename to spec/models/concerns/integrations/base/issue_tracker_spec.rb index 1bb24876222ddd..69696c2765674d 100644 --- a/spec/models/integrations/base_issue_tracker_spec.rb +++ b/spec/models/concerns/integrations/base/issue_tracker_spec.rb @@ -2,13 +2,21 @@ require 'spec_helper' -RSpec.describe Integrations::BaseIssueTracker, feature_category: :integrations do - let(:integration) { build(:redmine_integration, project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } +RSpec.describe Integrations::Base::IssueTracker, feature_category: :integrations do + let(:integration) do + build( + :redmine_integration, + project: project, + active: true, + issue_tracker_data: + build(:issue_tracker_data) + ) + end let_it_be_with_refind(:project) { create(:project) } describe 'default values' do - it { expect(subject.category).to eq(:issue_tracker) } + it { expect(integration.category).to eq(:issue_tracker) } end describe 'Validations' do @@ -46,7 +54,7 @@ end context 'when there is no existing issue tracker integration' do - it { is_expected.to be(nil) } + it { is_expected.to be_nil } end end end diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 62080fa7a128eb..8a25d368b80e64 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -12,7 +12,7 @@ subject(:integration) { build(:bamboo_integration, project: project, bamboo_url: bamboo_url) } - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci it_behaves_like Integrations::ResetSecretFields diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index 793046a9869a14..32efcaf5a17a39 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -18,7 +18,7 @@ ) end - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci it_behaves_like Integrations::ResetSecretFields diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index ffd24981c89e66..ab01d4641e4629 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -9,7 +9,7 @@ let_it_be(:project) { create(:project, :repository, name: 'project') } - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci it_behaves_like Integrations::ResetSecretFields do let(:integration) { subject } diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 0924305130a9cb..14445ee95cfe8a 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -23,7 +23,7 @@ } end - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci it_behaves_like Integrations::ResetSecretFields do let(:integration) { jenkins_integration } diff --git a/spec/models/integrations/mattermost_slash_commands_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index 183e9643859e8c..940e96eaaddd70 100644 --- a/spec/models/integrations/mattermost_slash_commands_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Integrations::MattermostSlashCommands, feature_category: :integrations do - it_behaves_like Integrations::BaseSlashCommands + it_behaves_like Integrations::Base::SlashCommands describe 'Mattermost API' do let_it_be_with_reload(:project) { create(:project) } diff --git a/spec/models/integrations/mock_ci_spec.rb b/spec/models/integrations/mock_ci_spec.rb index 3ff47ab2f0b3cb..7b9d7d4257c15b 100644 --- a/spec/models/integrations/mock_ci_spec.rb +++ b/spec/models/integrations/mock_ci_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -RSpec.describe Integrations::MockCi do +RSpec.describe Integrations::MockCi, feature_category: :integrations do let_it_be(:project) { build(:project) } subject(:integration) { described_class.new(project: project, mock_service_url: generate(:url)) } - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci include_context Integrations::EnableSslVerification diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 2366a8d16c76b3..d0cc1a902a23e9 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -12,7 +12,7 @@ let(:integration) { project.prometheus_integration } - it_behaves_like Integrations::BaseMonitoring + it_behaves_like Integrations::Base::Monitoring context 'redirects' do it 'does not follow redirects' do diff --git a/spec/models/integrations/slack_slash_commands_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index 5d6f214a5d5814..77e7df0f3bd5f5 100644 --- a/spec/models/integrations/slack_slash_commands_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Integrations::SlackSlashCommands, feature_category: :integrations do - it_behaves_like Integrations::BaseSlashCommands + it_behaves_like Integrations::Base::SlashCommands describe '#trigger' do context 'when an auth url is generated' do diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index daea849f6aa122..f98088aa648230 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -24,7 +24,7 @@ ) end - it_behaves_like Integrations::BaseCi + it_behaves_like Integrations::Base::Ci it_behaves_like Integrations::ResetSecretFields diff --git a/spec/support/shared_examples/models/integrations/base_ci_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/base/ci_shared_examples.rb similarity index 71% rename from spec/support/shared_examples/models/integrations/base_ci_shared_examples.rb rename to spec/support/shared_examples/models/concerns/integrations/base/ci_shared_examples.rb index 08fab45e41b1f9..171f0d9fe4d950 100644 --- a/spec/support/shared_examples/models/integrations/base_ci_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/base/ci_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples Integrations::BaseCi do +RSpec.shared_examples Integrations::Base::Ci do describe 'default values' do it { expect(subject.category).to eq(:ci) } end diff --git a/spec/support/shared_examples/models/integrations/base_monitoring_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/base/monitoring_shared_examples.rb similarity index 69% rename from spec/support/shared_examples/models/integrations/base_monitoring_shared_examples.rb rename to spec/support/shared_examples/models/concerns/integrations/base/monitoring_shared_examples.rb index 5d7e7633a23e8a..4885bc41e17cf6 100644 --- a/spec/support/shared_examples/models/integrations/base_monitoring_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/base/monitoring_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples Integrations::BaseMonitoring do +RSpec.shared_examples Integrations::Base::Monitoring do describe 'default values' do it { expect(subject.category).to eq(:monitoring) } end diff --git a/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb similarity index 91% rename from spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb rename to spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb index 90bb75d402f608..f98d92b503e731 100644 --- a/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples Integrations::BaseSlashCommands do +RSpec.shared_examples Integrations::Base::SlashCommands do describe "Associations" do it { is_expected.to respond_to :token } end @@ -32,7 +32,7 @@ describe '#trigger' do subject { described_class.new } - context 'no token is passed' do + context 'when no token is passed' do let(:params) { {} } it 'returns nil' do @@ -48,7 +48,7 @@ allow(subject).to receive(:token).and_return('token') end - context 'no user can be found' do + context 'when no user can be found' do context 'when no url can be generated' do it 'responds with the authorize url' do response = subject.trigger(params) @@ -99,14 +99,14 @@ end it 'triggers the command' do - expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) + expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) # rubocop:disable RSpec/AnyInstanceOf -- Legacy use subject.trigger(params) end shared_examples_for 'blocks command execution' do - it do - expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) + it 'blocks command execution' do + expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) # rubocop:disable RSpec/AnyInstanceOf -- Legacy use result = subject.trigger(params) expect(result[:text]).to match(error_message) @@ -154,7 +154,7 @@ it 'caches the slash command params and returns confirmation message' do expect(Rails.cache).to receive(:write).with(an_instance_of(String), params, { expires_in: 3.minutes }) - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Access).to receive(:confirm) + expect_any_instance_of(Gitlab::SlashCommands::Presenters::Access).to receive(:confirm) # rubocop:disable RSpec/AnyInstanceOf -- Legacy use subject.trigger(params) end -- GitLab From b96fd27f426bc40a6350120aefcc4be3681823bc Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Tue, 19 Nov 2024 17:07:48 +0000 Subject: [PATCH 7/7] Code review feedback --- app/models/concerns/integrations/base/ci.rb | 4 ++-- app/models/concerns/integrations/base/issue_tracker.rb | 2 +- app/models/concerns/integrations/base/third_party_wiki.rb | 2 +- doc/development/integrations/index.md | 2 +- .../integrations/base/slash_commands_shared_examples.rb | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/integrations/base/ci.rb b/app/models/concerns/integrations/base/ci.rb index 5393c656bff81a..0abd280d559a41 100644 --- a/app/models/concerns/integrations/base/ci.rb +++ b/app/models/concerns/integrations/base/ci.rb @@ -30,7 +30,7 @@ def valid_token?(token) # http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c # def build_page(sha, ref) - # implement inside child + # override this method in the including class end # Return string with build status or :error symbol @@ -47,7 +47,7 @@ def build_page(sha, ref) # # def commit_status(sha, ref) - # implement inside child + # override this method in the including class end end end diff --git a/app/models/concerns/integrations/base/issue_tracker.rb b/app/models/concerns/integrations/base/issue_tracker.rb index 620cc17f0d864a..d09acef98b9bcd 100644 --- a/app/models/concerns/integrations/base/issue_tracker.rb +++ b/app/models/concerns/integrations/base/issue_tracker.rb @@ -133,7 +133,7 @@ def support_cross_reference? end def create_cross_reference_note(external_issue, mentioned_in, author) - # implement inside child + # override this method in the including class end def activate_disabled_reason diff --git a/app/models/concerns/integrations/base/third_party_wiki.rb b/app/models/concerns/integrations/base/third_party_wiki.rb index 37d60a2c695ef7..1265be94c1ec85 100644 --- a/app/models/concerns/integrations/base/third_party_wiki.rb +++ b/app/models/concerns/integrations/base/third_party_wiki.rb @@ -25,7 +25,7 @@ def supported_events def only_one_third_party_wiki return unless project_level? - return unless project.integrations.third_party_wikis.id_not_in(id).any? + return if project.integrations.third_party_wikis.id_not_in(id).empty? errors.add(:base, _('Another third-party wiki is already in use. ' \ 'Only one third-party wiki integration can be active at a time')) diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index 6e9ad58291cf56..4a0490fdee2bd2 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -21,7 +21,7 @@ if you need clarification or spot any outdated information. 1. Add a new model in `app/models/integrations` extending from `Integration`. - For example, `Integrations::FooBar` in `app/models/integrations/foo_bar.rb`. - - For certain types of integrations, you can include these base modules (or inherit from them if they are classes): + - For certain types of integrations, you can include these base modules: - `Integrations::Base::ChatNotification` - `Integrations::Base::Ci` - `Integrations::Base::IssueTracker` diff --git a/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb index f98d92b503e731..7de625540344b4 100644 --- a/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/base/slash_commands_shared_examples.rb @@ -32,7 +32,7 @@ describe '#trigger' do subject { described_class.new } - context 'when no token is passed' do + context 'when token is not passed' do let(:params) { {} } it 'returns nil' do @@ -48,7 +48,7 @@ allow(subject).to receive(:token).and_return('token') end - context 'when no user can be found' do + context 'when user does not exist' do context 'when no url can be generated' do it 'responds with the authorize url' do response = subject.trigger(params) -- GitLab