diff --git a/app/models/integration.rb b/app/models/integration.rb index 0f7168620329f0f530499831107d8c5f9202653a..2bc8e3752e17b7ef5dbf7e0618a1d64a24b90e35 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -28,7 +28,7 @@ class Integration < ApplicationRecord # TODO Shimo is temporary disabled on group and instance-levels. # See: https://gitlab.com/gitlab-org/gitlab/-/issues/345677 PROJECT_SPECIFIC_INTEGRATION_NAMES = %w[ - apple_app_store google_play jenkins shimo + apple_app_store gitlab_slack_application google_play jenkins shimo ].freeze # Fake integrations to help with local development. @@ -282,7 +282,6 @@ def self.nonexistent_integration_types_for(scope) # Returns a list of available integration names. # Example: ["asana", ...] - # @deprecated def self.available_integration_names(include_project_specific: true, include_dev: true) names = integration_names names += project_specific_integration_names if include_project_specific @@ -302,7 +301,9 @@ def self.dev_integration_names end def self.project_specific_integration_names - PROJECT_SPECIFIC_INTEGRATION_NAMES + names = PROJECT_SPECIFIC_INTEGRATION_NAMES.dup + names.delete('gitlab_slack_application') unless Gitlab::CurrentSettings.slack_app_enabled || Rails.env.test? + names end # Returns a list of available integration types. diff --git a/app/models/project.rb b/app/models/project.rb index ef87b611fca6d32dcf9c18dead4be899ca42e254..a243842c59ba9dc276081ea22cdac12df0962991 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -195,6 +195,7 @@ def self.integration_association_name(name) has_one :emails_on_push_integration, class_name: 'Integrations::EmailsOnPush' has_one :ewm_integration, class_name: 'Integrations::Ewm' has_one :external_wiki_integration, class_name: 'Integrations::ExternalWiki' + has_one :gitlab_slack_application_integration, class_name: 'Integrations::GitlabSlackApplication' has_one :google_play_integration, class_name: 'Integrations::GooglePlay' has_one :hangouts_chat_integration, class_name: 'Integrations::HangoutsChat' has_one :harbor_integration, class_name: 'Integrations::Harbor' @@ -805,6 +806,20 @@ def self.with_web_entity_associations preload(:project_feature, :route, :creator, group: :parent, namespace: [:route, :owner]) end + def self.with_slack_application_disabled + # Using Arel to avoid exposing what the column backing the type: attribute is + # rubocop: disable GitlabSecurity/PublicSend + with_active_slack = Integration.active.by_name(:gitlab_slack_application) + join_contraint = arel_table[:id].eq(Integration.arel_table[:project_id]) + constraint = with_active_slack.where_clause.send(:predicates).reduce(join_contraint) { |a, b| a.and(b) } + join = arel_table.join(Integration.arel_table, Arel::Nodes::OuterJoin).on(constraint).join_sources + # rubocop: enable GitlabSecurity/PublicSend + + joins(join).where(integrations: { id: nil }) + rescue Integration::UnknownType + all + end + def self.eager_load_namespace_and_owner includes(namespace: :owner) end @@ -1713,7 +1728,13 @@ def find_or_initialize_integrations end def disabled_integrations - %w[shimo zentao] + return [] if Rails.env.development? + + names = %w[shimo zentao] + + # The Slack Slash Commands integration is only available for customers who cannot use the GitLab for Slack app. + # The GitLab for Slack app integration is only available when enabled through settings. + names << (Gitlab::CurrentSettings.slack_app_enabled ? 'slack_slash_commands' : 'gitlab_slack_application') end def find_or_initialize_integration(name) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 39ee028faa0154cc7dc5e049c0c0f8c7e9011884..db3f0f0c53fa0d53025d7538a2f665633604070f 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -25703,7 +25703,7 @@ State of a Sentry error. | `EWM_SERVICE` | EwmService type. | | `EXTERNAL_WIKI_SERVICE` | ExternalWikiService type. | | `GITHUB_SERVICE` | GithubService type. | -| `GITLAB_SLACK_APPLICATION_SERVICE` | GitlabSlackApplicationService type (Gitlab.com only). | +| `GITLAB_SLACK_APPLICATION_SERVICE` | GitlabSlackApplicationService type. | | `GOOGLE_PLAY_SERVICE` | GooglePlayService type. | | `HANGOUTS_CHAT_SERVICE` | HangoutsChatService type. | | `HARBOR_SERVICE` | HarborService type. | diff --git a/ee/app/graphql/ee/types/projects/service_type_enum.rb b/ee/app/graphql/ee/types/projects/service_type_enum.rb deleted file mode 100644 index 1a28100abbb163ea959663c90e63c2b2831a0dbb..0000000000000000000000000000000000000000 --- a/ee/app/graphql/ee/types/projects/service_type_enum.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module EE - module Types - module Projects - module ServiceTypeEnum - extend ActiveSupport::Concern - - class_methods do - extend ::Gitlab::Utils::Override - - private - - override :type_description - def type_description(name, type) - description = super - description = [description, ' (Gitlab.com only)'].join if saas_only?(name) - description - end - - def saas_only?(name) - ::Integration.saas_only_integration_names.include?(name) - end - end - end - end - end -end diff --git a/ee/app/models/ee/integration.rb b/ee/app/models/ee/integration.rb index 453562a8dbb233be204eea05f75b4e2fcccdc17b..97b75fd65c9cb85f8c5b79991a42b13a6ced9dab 100644 --- a/ee/app/models/ee/integration.rb +++ b/ee/app/models/ee/integration.rb @@ -10,11 +10,6 @@ module Integration EE_PROJECT_SPECIFIC_INTEGRATION_NAMES = %w[ github - gitlab_slack_application - ].freeze - - EE_SAAS_ONLY_INTEGRATION_NAMES = %w[ - gitlab_slack_application ].freeze class_methods do @@ -24,24 +19,6 @@ module Integration def project_specific_integration_names super + EE_PROJECT_SPECIFIC_INTEGRATION_NAMES end - - def saas_only_integration_names - EE_SAAS_ONLY_INTEGRATION_NAMES - end - - override :available_integration_names - def available_integration_names(...) - names = super - names -= saas_only_integration_names unless include_saas_only? - names - end - - private - - # Returns true if this instance can show SaaS-only integrations. - def include_saas_only? - ::Gitlab.dev_or_test_env? || ::Gitlab.com? - end end end end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 8720b0f503e2ab446ca644d2b820a8b93082f7fb..6c082880dab5b1a1c8069866f93b6dbf7e8a6dca 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -55,7 +55,6 @@ def inapplicable_to_branch(branch) has_one :index_status has_one :github_integration, class_name: 'Integrations::Github' - has_one :gitlab_slack_application_integration, class_name: 'Integrations::GitlabSlackApplication' has_one :status_page_setting, inverse_of: :project, class_name: 'StatusPage::ProjectSetting' has_one :compliance_framework_setting, class_name: 'ComplianceManagement::ComplianceFramework::ProjectSettings', inverse_of: :project @@ -414,20 +413,6 @@ def search_by_visibility(level) where(visibility_level: ::Gitlab::VisibilityLevel.string_options[level]) end - def with_slack_application_disabled - # Using Arel to avoid exposing what the column backing the type: attribute is - # rubocop: disable GitlabSecurity/PublicSend - with_active_slack = ::Integration.active.by_name(:gitlab_slack_application) - join_contraint = arel_table[:id].eq(::Integration.arel_table[:project_id]) - constraint = with_active_slack.where_clause.send(:predicates).reduce(join_contraint) { |a, b| a.and(b) } - join = arel_table.join(::Integration.arel_table, Arel::Nodes::OuterJoin).on(constraint).join_sources - # rubocop: enable GitlabSecurity/PublicSend - - joins(join).where(integrations: { id: nil }) - rescue ::Integration::UnknownType - all - end - override :with_web_entity_associations def with_web_entity_associations super.preload(:compliance_framework_setting, group: [:ip_restrictions, :saml_provider]) @@ -809,15 +794,8 @@ def remove_import_data def disabled_integrations strong_memoize(:disabled_integrations) do gh = github_integration_enabled? ? [] : %w[github] - slack = if Rails.env.development? - [] - elsif ::Gitlab::CurrentSettings.slack_app_enabled - %w[slack_slash_commands] - else - %w[gitlab_slack_application] - end - - super + gh + slack + + super + gh end end diff --git a/ee/spec/controllers/projects/settings/integrations_controller_spec.rb b/ee/spec/controllers/projects/settings/integrations_controller_spec.rb index 133fa70c646ee44ff5b5bab2d07ab6bb6b0e6f32..fa9cba590927962bd3cfaadb58482b8fd866878a 100644 --- a/ee/spec/controllers/projects/settings/integrations_controller_spec.rb +++ b/ee/spec/controllers/projects/settings/integrations_controller_spec.rb @@ -40,7 +40,6 @@ it 'enables GitlabSlackApplication and disables SlackSlashCommands' do stub_application_setting(slack_app_enabled: true) - allow(::Gitlab).to receive(:com?).and_return(true) get :index, params: { namespace_id: project.namespace, project_id: project } diff --git a/ee/spec/graphql/ee/types/projects/service_type_enum_spec.rb b/ee/spec/graphql/ee/types/projects/service_type_enum_spec.rb deleted file mode 100644 index d53277ae6908f6bb7c021dfeed886b4d90992c3d..0000000000000000000000000000000000000000 --- a/ee/spec/graphql/ee/types/projects/service_type_enum_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe GitlabSchema.types['ServiceType'] do - context 'GitLabSlackApplicationService' do - subject { described_class.values['GITLAB_SLACK_APPLICATION_SERVICE'] } - - it 'appends a note to the description' do - expect(subject.description).to end_with(' (Gitlab.com only)') - end - end -end diff --git a/ee/spec/models/ee/integration_spec.rb b/ee/spec/models/ee/integration_spec.rb index 11af263189040ba439df1e9cf594a50d3ab41973..85c5f0ed03fd1d9c006339e78ff5618d43d694be 100644 --- a/ee/spec/models/ee/integration_spec.rb +++ b/ee/spec/models/ee/integration_spec.rb @@ -4,24 +4,13 @@ RSpec.describe Integration do describe '.available_integration_names' do - let(:include_saas_only) { true } - subject { described_class.available_integration_names } before do - allow(described_class).to receive(:integration_names).and_return(%w(foo saas_only)) - allow(described_class).to receive(:saas_only_integration_names).and_return(['saas_only']) - allow(described_class).to receive(:include_saas_only?).and_return(include_saas_only) + allow(described_class).to receive(:integration_names).and_return(%w(foo)) end - it { is_expected.to include('foo', 'saas_only') } - - context 'when instance is not SaaS' do - let(:include_saas_only) { false } - - it { is_expected.to include('foo') } - it { is_expected.not_to include('saas_only') } - end + it { is_expected.to include('foo') } end describe '.project_specific_integration_names' do @@ -33,15 +22,6 @@ end end - describe '.saas_only_integration_names' do - specify do - stub_const("EE::#{described_class.name}::EE_SAAS_ONLY_INTEGRATION_NAMES", ['ee_sass_only_name']) - - expect(described_class.saas_only_integration_names) - .to eq(['ee_sass_only_name']) - end - end - describe '.vulnerability_hooks' do it 'includes integrations where vulnerability_events is true' do create(:integration, active: true, vulnerability_events: true) diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 4ab88a1d3cc2ea1b94a288bdd67da184cc692144..5beb1cdd66a10f72d08566dd3aa4baae5fc466e1 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -1993,24 +1993,6 @@ end end end - - context 'slack' do - where(:development, :slack_app_enabled, :disabled_integrations) do - true | true | [] - true | false | [] - false | true | %w[slack_slash_commands] - false | false | %w[gitlab_slack_application] - end - - with_them do - before do - allow(Rails.env).to receive(:development?).and_return(development) - allow(Gitlab::CurrentSettings).to receive(:slack_app_enabled).and_return(slack_app_enabled) - end - - it { is_expected.to include(*disabled_integrations) } - end - end end describe '#pull_mirror_available?' do @@ -2089,38 +2071,6 @@ end end - describe '#with_slack_application_disabled' do - let_it_be(:project1) { create(:project) } - let_it_be(:project2) { create(:project) } - let_it_be(:project3) { create(:project) } - - before_all do - create(:gitlab_slack_application_integration, project: project2) - create(:gitlab_slack_application_integration, project: project3).update!(active: false) - end - - context 'when slack applications are available' do - it 'returns projects where Slack application is disabled or absent' do - projects = described_class.with_slack_application_disabled - - expect(projects).to include(project1, project3) - expect(projects).not_to include(project2) - end - end - - context 'when slack applications are not available' do - before do - allow(::Gitlab).to receive(:dev_or_test_env?).and_return(false) - end - - it 'returns projects where Slack application is disabled or absent' do - projects = described_class.with_slack_application_disabled - - expect(projects).to include(project1, project2, project3) - end - end - end - describe '#licensed_features', :saas do let(:plan_license) { :free } let(:global_license) { create(:license) } diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 46c30074ae7c0ce8199a05d2f0aa7a3ce21158da..ed49009d6d9d00494b13680fe5f0146046038059 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -994,9 +994,25 @@ def fields end describe '.project_specific_integration_names' do - specify do - expect(described_class.project_specific_integration_names) - .to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) + subject { described_class.project_specific_integration_names } + + it { is_expected.to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) } + it { is_expected.to include('gitlab_slack_application') } + + context 'when Rails.env is not test' do + before do + allow(Rails.env).to receive(:test?).and_return(false) + end + + it { is_expected.not_to include('gitlab_slack_application') } + + context 'when `slack_app_enabled` setting is enabled' do + before do + stub_application_setting(slack_app_enabled: true) + end + + it { is_expected.to include('gitlab_slack_application') } + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 32158ef9509199c60c78f9ea9c89a54b554ecd8f..312a942ff0e400e9f69b3aef6c3e5b1e54259da5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -82,6 +82,7 @@ it { is_expected.to have_one(:ewm_integration) } it { is_expected.to have_one(:external_wiki_integration) } it { is_expected.to have_one(:confluence_integration) } + it { is_expected.to have_one(:gitlab_slack_application_integration) } it { is_expected.to have_one(:project_feature) } it { is_expected.to have_one(:project_repository) } it { is_expected.to have_one(:container_expiration_policy) } @@ -2136,6 +2137,43 @@ def has_external_wiki end end + describe '.with_slack_application_disabled' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:project3) { create(:project) } + + before_all do + create(:gitlab_slack_application_integration, project: project2) + create(:gitlab_slack_application_integration, project: project3).update!(active: false) + end + + context 'when the Slack app setting is enabled' do + before do + stub_application_setting(slack_app_enabled: true) + end + + it 'includes only projects where Slack app is disabled or absent' do + projects = described_class.with_slack_application_disabled + + expect(projects).to include(project1, project3) + expect(projects).not_to include(project2) + end + end + + context 'when the Slack app setting is not enabled' do + before do + stub_application_setting(slack_app_enabled: false) + allow(Rails.env).to receive(:test?).and_return(false, true) + end + + it 'includes all projects' do + projects = described_class.with_slack_application_disabled + + expect(projects).to include(project1, project2, project3) + end + end + end + describe '.cached_count', :use_clean_rails_memory_store_caching do let(:group) { create(:group, :public) } let!(:project1) { create(:project, :public, group: group) } @@ -6749,6 +6787,31 @@ def has_external_wiki end end + describe '#disabled_integrations' do + subject { build(:project).disabled_integrations } + + it { is_expected.to include('gitlab_slack_application') } + it { is_expected.not_to include('slack_slash_commands') } + + context 'when slack_app_enabled setting is enabled' do + before do + stub_application_setting(slack_app_enabled: true) + end + + it { is_expected.to include('slack_slash_commands') } + it { is_expected.not_to include('gitlab_slack_application') } + end + + context 'when Rails.env.development?' do + before do + allow(Rails.env).to receive(:development?).and_return(true) + end + + it { is_expected.not_to include('slack_slash_commands') } + it { is_expected.not_to include('gitlab_slack_application') } + end + end + describe '#find_or_initialize_integration' do it 'avoids N+1 database queries' do allow(Integration).to receive(:available_integration_names).and_return(%w[prometheus pushover]) diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index 8d348dc0a54ffee7c40f57c076d8c8f3b1e9b2a4..8334d6b683287890517204087de3f638d28b3f26 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -44,11 +44,17 @@ end where(:integration) do - # The API supports all integrations except the GitLab Slack Application - # integration; this integration must be installed via the UI. + # The Project Integrations API supports all integrations except: + # - The GitLab Slack Application integration, as it must be installed via the UI. + # - Shimo and ZenTao integrations, as new integrations are blocked from being created. + unavailable_integration_names = [ + Integrations::GitlabSlackApplication.to_param, + Integrations::Shimo.to_param, + Integrations::Zentao.to_param + ] + names = Integration.available_integration_names - names.delete(Integrations::GitlabSlackApplication.to_param) if Gitlab.ee? - names - %w[shimo zentao] + names.reject { |name| name.in?(unavailable_integration_names) } end with_them do