diff --git a/app/controllers/concerns/integrations/slack_controller_settings.rb b/app/controllers/concerns/integrations/slack_controller_settings.rb index d3e8aca439ac5c8d3a6b2b8be2e135c2ad765845..a1d9d813cf6bd62ffd4fdb9aca3efe3096f3e951 100644 --- a/app/controllers/concerns/integrations/slack_controller_settings.rb +++ b/app/controllers/concerns/integrations/slack_controller_settings.rb @@ -30,6 +30,8 @@ def slack_auth def destroy slack_integration.destroy + PropagateIntegrationWorker.perform_async(integration.id) unless integration.project_level? + redirect_to_integration_page end diff --git a/app/models/slack_integration.rb b/app/models/slack_integration.rb index 2f7f193cfe3a2d8fe6c3ee46b15fa97fcb5ffcab..8e852aa4fa3c1732b44ef9cb9c7a757f34ae58ff 100644 --- a/app/models/slack_integration.rb +++ b/app/models/slack_integration.rb @@ -13,6 +13,9 @@ class SlackIntegration < ApplicationRecord # will need reauthorization. # https://api.slack.com/authentication/oauth-v2#asking SCOPES = [SCOPE_COMMANDS, SCOPE_CHAT_WRITE, SCOPE_CHAT_WRITE_PUBLIC].freeze + DATABASE_ATTRIBUTES = %w[ + team_id team_name user_id bot_user_id encrypted_bot_access_token encrypted_bot_access_token_iv + ].freeze belongs_to :integration @@ -32,6 +35,7 @@ class SlackIntegration < ApplicationRecord scope :with_bot, -> { where.not(bot_user_id: nil) } scope :by_team, ->(team_id) { where(team_id: team_id) } + scope :by_integration, ->(integration_ids) { where(integration_id: integration_ids) } validates :team_id, presence: true validates :team_name, presence: true @@ -75,6 +79,10 @@ def authorized_scope_names slack_api_scopes.pluck(:name) end + def to_database_hash + attributes_for_database.slice(*DATABASE_ATTRIBUTES) + end + private def update_active_status_of_integration diff --git a/app/services/integrations/propagation/bulk_create_service.rb b/app/services/integrations/propagation/bulk_create_service.rb index 766516a1ff3f38dc9c7d144fcd6b4faef2c6cb8a..d5b9fc3e85712ee6bb1b90afbe946d09244a76f2 100644 --- a/app/services/integrations/propagation/bulk_create_service.rb +++ b/app/services/integrations/propagation/bulk_create_service.rb @@ -7,7 +7,7 @@ class BulkCreateService def initialize(integration, batch, association) @integration = integration - @batch = batch + @batch = batch.to_a @association = association end @@ -16,6 +16,12 @@ def execute inserted_ids = bulk_insert_integrations bulk_insert_data_fields(inserted_ids) if integration.data_fields_present? + + if integration.is_a?(GitlabSlackApplication) && integration.active? && + Feature.enabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) + inserted_slack_ids = bulk_insert_slack_integrations(inserted_ids) + bulk_insert_slack_integration_scopes(inserted_slack_ids) + end end end @@ -33,7 +39,7 @@ def bulk_insert_new(model, items_to_insert) def bulk_insert_integrations attributes = integration_hash(:create) - items_to_insert = batch.select(:id).map do |record| + items_to_insert = batch.map do |record| attributes.merge("#{association}_id" => record.id) end @@ -51,6 +57,34 @@ def bulk_insert_data_fields(integration_ids) bulk_insert_new(model, items_to_insert) end + + def bulk_insert_slack_integrations(integration_ids) + hash = integration.slack_integration.to_database_hash + + items_to_insert = integration_ids.zip(batch).map do |integration_id, record| + hash.merge( + 'integration_id' => integration_id, + 'alias' => record.full_path + ) + end + + bulk_insert_new(SlackIntegration, items_to_insert) + end + + def bulk_insert_slack_integration_scopes(inserted_slack_ids) + scopes = integration.slack_integration.slack_api_scopes + + items_to_insert = scopes.flat_map do |scope| + inserted_slack_ids.map do |record_id| + { + 'slack_integration_id' => record_id, + 'slack_api_scope_id' => scope.id + } + end + end + + bulk_insert_new(SlackWorkspace::IntegrationApiScope, items_to_insert) + end end end end diff --git a/app/services/integrations/propagation/bulk_update_service.rb b/app/services/integrations/propagation/bulk_update_service.rb index e4a4c27c4ebe8d1eedfef60c7c9bc73680170f0d..4f287e0d716f3382cc4e56d23fc9214c0d8a499e 100644 --- a/app/services/integrations/propagation/bulk_update_service.rb +++ b/app/services/integrations/propagation/bulk_update_service.rb @@ -21,6 +21,15 @@ def execute data_fields_hash(:update) ) end + + if integration.is_a?(GitlabSlackApplication) && + Feature.enabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) + if integration.active? # rubocop: disable Cop/LineBreakAroundConditionalBlock -- Misidentified + bulk_update_slack_integrations + else + bulk_delete_slack_integrations + end + end end end # rubocop: enable CodeReuse/ActiveRecord @@ -42,6 +51,23 @@ def batch_ids batch.map(&:id) end end + + def bulk_update_slack_integrations + slack_integration_batch = SlackIntegration.by_integration(batch_ids) + + slack_integration_batch.update_all( + integration.slack_integration.to_database_hash + ) + + Integrations::SlackWorkspace::IntegrationApiScope.update_scopes( + slack_integration_batch.pluck_primary_key, + integration.slack_integration.slack_api_scopes + ) + end + + def bulk_delete_slack_integrations + SlackIntegration.by_integration(batch_ids).delete_all + end end end end diff --git a/app/services/integrations/slack_installation/base_service.rb b/app/services/integrations/slack_installation/base_service.rb index 64addfd2b4a9b210ea1c75e3d75cbdd3fbd91fc4..c9781533cdfe1af7dad269bdf5f2a581d8ddcff0 100644 --- a/app/services/integrations/slack_installation/base_service.rb +++ b/app/services/integrations/slack_installation/base_service.rb @@ -64,6 +64,8 @@ def execute update_other_installations!(installation) + PropagateIntegrationWorker.perform_async(integration.id) unless integration.project_level? + ServiceResponse.success end diff --git a/app/views/shared/integrations/gitlab_slack_application/_slack_integration_form.html.haml b/app/views/shared/integrations/gitlab_slack_application/_slack_integration_form.html.haml index ca9a9aef33f3b1b1f614060cff7857c637658404..3f4ce7c322c62b3d59800fb37572b584c1bc6fa0 100644 --- a/app/views/shared/integrations/gitlab_slack_application/_slack_integration_form.html.haml +++ b/app/views/shared/integrations/gitlab_slack_application/_slack_integration_form.html.haml @@ -1,5 +1,7 @@ - slack_integration = integration.slack_integration - if slack_integration + - inherited = integration.inherit_from_id.present? + - destroy_button_modal = inherited ? {} : { data: { confirm_btn_variant: "danger", confirm: s_('SlackIntegration|Are you sure you want to unlink this Slack Workspace from this integration?') } } %table.gl-table.gl-w-full %thead %tr @@ -22,9 +24,10 @@ - if integration.project_level? = render Pajamas::ButtonComponent.new(href: edit_project_settings_slack_path(integration.parent)) do = _('Edit') - = render Pajamas::ButtonComponent.new(method: :delete, category: 'secondary', variant: "danger", href: slack_integration_destroy_path(integration.parent), icon: 'remove', button_options: { aria: { label: s_('Remove') }, data: { confirm_btn_variant: "danger", confirm: s_('SlackIntegration|Are you sure you want to unlink this Slack Workspace from this integration?') }}) + .destroy-button-container{ inherited ? { title: s_('SlackIntegration|GitLab for Slack app configured on the group or instance level and can only be modified there.'), data: { toggle: 'tooltip' } } : {} } + = render Pajamas::ButtonComponent.new(method: :delete, category: 'secondary', variant: "danger", href: slack_integration_destroy_path(integration.parent), icon: 'remove', disabled: inherited, button_options: { aria: { label: s_('Remove') }, **destroy_button_modal}) .gl-my-5 - = render Pajamas::ButtonComponent.new(href: add_to_slack_link(integration.parent, slack_app_id)) do + = render Pajamas::ButtonComponent.new(href: add_to_slack_link(integration.parent, slack_app_id), disabled: inherited) do = s_('SlackIntegration|Reinstall GitLab for Slack app…') %p = html_escape(s_('SlackIntegration|You may need to reinstall the GitLab for Slack app when we %{linkStart}make updates or change permissions%{linkEnd}.')) % { linkStart: %().html_safe, linkEnd: ''.html_safe} diff --git a/app/workers/propagate_integration_worker.rb b/app/workers/propagate_integration_worker.rb index 3ebea30a65eeae8e735a6618bd088a102bcce969..10ffc34da881209061f0518dab520adb5b0cf484 100644 --- a/app/workers/propagate_integration_worker.rb +++ b/app/workers/propagate_integration_worker.rb @@ -14,6 +14,7 @@ class PropagateIntegrationWorker def perform(integration_id) integration = Integration.find_by_id(integration_id) return unless integration + return if integration.project_level? ::Integrations::PropagateService.new(integration).execute end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d14950d542a273048308f9342ef4a6e9f58fa289..d9b9c5c985686d15ed62e7d60264a5437446c27b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -47149,6 +47149,9 @@ msgstr "" msgid "SlackIntegration|GitLab for Slack" msgstr "" +msgid "SlackIntegration|GitLab for Slack app configured on the group or instance level and can only be modified there." +msgstr "" + msgid "SlackIntegration|GitLab for Slack was successfully installed." msgstr "" diff --git a/spec/features/groups/integrations/user_manages_gitlab_for_slack_app_spec.rb b/spec/features/groups/integrations/user_manages_gitlab_for_slack_app_spec.rb index 1829acb8954004b17c0af634992f48c2fc7fec42..0c8aaee0375da51119f6917e3b81b60fc803c153 100644 --- a/spec/features/groups/integrations/user_manages_gitlab_for_slack_app_spec.rb +++ b/spec/features/groups/integrations/user_manages_gitlab_for_slack_app_spec.rb @@ -7,7 +7,7 @@ include_context 'group integration activation' - let_it_be(:integration) do + let_it_be_with_reload(:integration) do create(:gitlab_slack_application_integration, :group, group: group, slack_integration: build(:slack_integration) ) @@ -46,21 +46,41 @@ def visit_slack_application_form end end - it 'allows the user to unlink the GitLab for Slack app' do - visit_slack_application_form + context 'when an integration is inherited' do + let_it_be(:instance_integration) do + create(:gitlab_slack_application_integration, :instance, slack_integration: build(:slack_integration)) + end - within_testid 'integration-settings-form' do - page.find('a.btn-danger').click + before do + integration.update!(inherit_from_id: instance_integration.id) end - within_modal do - expect(page).to have_content('Are you sure you want to unlink this Slack Workspace from this integration?') - click_button('Remove') + it 'does not allow the user to unlink the GitLab for Slack app' do + visit_slack_application_form + + within_testid 'integration-settings-form' do + expect(page).to have_css('a.btn-danger[disabled]') + end end + end - wait_for_requests + context 'when an integration is not inherited' do + it 'allows the user to unlink the GitLab for Slack app' do + visit_slack_application_form + + within_testid 'integration-settings-form' do + page.find('a.btn-danger').click + end - expect(page).to have_content('Install GitLab for Slack app') + within_modal do + expect(page).to have_content('Are you sure you want to unlink this Slack Workspace from this integration?') + click_button('Remove') + end + + wait_for_requests + + expect(page).to have_content('Install GitLab for Slack app') + end end it 'shows the trigger form fields' do diff --git a/spec/models/slack_integration_spec.rb b/spec/models/slack_integration_spec.rb index 0f1ab5c2845f297df949b5e83a36c0d14006cf10..4766f07270ad6e965a90158cf87d5081b76adc60 100644 --- a/spec/models/slack_integration_spec.rb +++ b/spec/models/slack_integration_spec.rb @@ -3,12 +3,14 @@ require 'spec_helper' RSpec.describe SlackIntegration, feature_category: :integrations do + let_it_be(:integration) { create(:slack_integration) } + describe "Associations" do it { is_expected.to belong_to(:integration) } end describe 'authorized_scope_names' do - subject(:slack_integration) { create(:slack_integration) } + subject(:slack_integration) { integration } it 'accepts assignment to nil' do slack_integration.update!(authorized_scope_names: nil) @@ -42,7 +44,7 @@ end describe 'all_features_supported?/upgrade_needed?' do - subject(:slack_integration) { create(:slack_integration) } + subject(:slack_integration) { integration } context 'with enough scopes' do before do @@ -67,7 +69,7 @@ end describe 'feature_available?' do - subject(:slack_integration) { create(:slack_integration) } + subject(:slack_integration) { integration } context 'without any scopes' do it 'is always true for :commands' do @@ -117,6 +119,14 @@ end end + describe '#to_database_hash' do + subject(:slack_integration) { integration } + + it 'includes the correct attributes' do + expect(slack_integration.to_database_hash.keys).to contain_exactly(*described_class::DATABASE_ATTRIBUTES) + end + end + it 'toggles the integration to active when created' do integration = create(:gitlab_slack_application_integration, active: false, slack_integration: nil) @@ -130,7 +140,7 @@ end describe 'Scopes' do - let_it_be(:slack_integration) { create(:slack_integration) } + let_it_be(:slack_integration) { integration } let_it_be(:legacy_slack_integration) { create(:slack_integration, :legacy) } describe '#with_bot' do @@ -147,6 +157,14 @@ expect(described_class.by_team(team_id)).to contain_exactly(slack_integration, team_slack_integration) end end + + describe '#by_integration' do + it 'returns records by the integration' do + integration = legacy_slack_integration.integration + + expect(described_class.by_integration(integration)).to contain_exactly(legacy_slack_integration) + end + end end describe 'Validations' do diff --git a/spec/requests/admin/slacks_controller_spec.rb b/spec/requests/admin/slacks_controller_spec.rb index 29825ef37d7eaf30caefdc9a28e13b8e7d5fdc3c..5433d563fc76a5f9ed13b44bc8c533a9c5675d06 100644 --- a/spec/requests/admin/slacks_controller_spec.rb +++ b/spec/requests/admin/slacks_controller_spec.rb @@ -14,6 +14,7 @@ let(:slack_auth_path) { slack_auth_admin_application_settings_slack_path } let(:destroy_path) { admin_application_settings_slack_path } let(:service) { Integrations::SlackInstallation::InstanceService } + let(:propagates_on_destroy) { true } let(:flag_protected) { true } let(:redirect_url) do diff --git a/spec/requests/groups/settings/slacks_controller_spec.rb b/spec/requests/groups/settings/slacks_controller_spec.rb index 0cacfc81a5739ab613b403ad99af4aa2bb440aae..a705d8ba9ea60fce3cb720127339256cfc2e7834 100644 --- a/spec/requests/groups/settings/slacks_controller_spec.rb +++ b/spec/requests/groups/settings/slacks_controller_spec.rb @@ -19,6 +19,7 @@ let(:slack_auth_path) { slack_auth_group_settings_slack_path(group) } let(:destroy_path) { group_settings_slack_path(group) } let(:service) { Integrations::SlackInstallation::GroupService } + let(:propagates_on_destroy) { true } let(:flag_protected) { true } let(:redirect_url) do diff --git a/spec/requests/projects/settings/slacks_controller_spec.rb b/spec/requests/projects/settings/slacks_controller_spec.rb index 15b3ca3d0fcec6f835b9cc5be046d9ed698d657b..6690b6af6f48bf053434855e86516da2bb01e581 100644 --- a/spec/requests/projects/settings/slacks_controller_spec.rb +++ b/spec/requests/projects/settings/slacks_controller_spec.rb @@ -25,6 +25,7 @@ let(:slack_auth_path) { slack_auth_project_settings_slack_path(project) } let(:destroy_path) { project_settings_slack_path(project) } let(:service) { Integrations::SlackInstallation::ProjectService } + let(:propagates_on_destroy) { false } let(:flag_protected) { false } def create_integration diff --git a/spec/services/integrations/propagation/bulk_create_service_spec.rb b/spec/services/integrations/propagation/bulk_create_service_spec.rb index 8888e06ee651eb024385606b69805ba920721c57..bcd9c01d29ebaeaffef5d9008676480760260307 100644 --- a/spec/services/integrations/propagation/bulk_create_service_spec.rb +++ b/spec/services/integrations/propagation/bulk_create_service_spec.rb @@ -21,13 +21,15 @@ ] end + subject(:execute_service) { described_class.new(integration, batch, association).execute } + shared_examples 'creates integration successfully' do def attributes(record) record.reload.attributes.except(*excluded_attributes) end it 'updates the inherited integrations' do - described_class.new(integration, batch, association).execute + execute_service expect(attributes(created_integration)).to eq attributes(integration) end @@ -36,14 +38,14 @@ def attributes(record) let(:excluded_attributes) { %w[id service_id integration_id created_at updated_at] } it 'updates the data fields from inherited integrations' do - described_class.new(integration, batch, association).execute + execute_service expect(attributes(created_integration.data_fields)) .to eq attributes(integration.data_fields) end it 'sets created_at and updated_at timestamps', :freeze_time do - described_class.new(integration, batch, association).execute + execute_service expect(created_integration.data_fields.reload).to have_attributes( created_at: eq(Time.current), @@ -53,13 +55,13 @@ def attributes(record) end it 'updates inherit_from_id attributes' do - described_class.new(integration, batch, association).execute + execute_service expect(created_integration.reload.inherit_from_id).to eq(inherit_from_id) end it 'sets created_at and updated_at timestamps', :freeze_time do - described_class.new(integration, batch, association).execute + execute_service expect(created_integration.reload).to have_attributes( created_at: eq(Time.current), @@ -68,17 +70,78 @@ def attributes(record) end end + shared_examples 'creates GitLab for Slack app data successfully' do + it 'creates associated SlackIntegration record and scopes' do + inherited_slack_integration = integration.slack_integration + + execute_service + + expect(created_integration.reload.slack_integration).to have_attributes( + team_id: inherited_slack_integration.team_id, + team_name: inherited_slack_integration.team_name, + alias: expected_alias, + user_id: inherited_slack_integration.user_id, + bot_user_id: inherited_slack_integration.bot_user_id, + bot_access_token: inherited_slack_integration.bot_access_token, + created_at: be_present, + updated_at: be_present, + authorized_scope_names: inherited_slack_integration.authorized_scope_names + ) + end + + context 'when integration is disabled' do + before do + integration.update!(active: false) + end + + it 'does not create associated SlackIntegration record' do + execute_service + + expect(created_integration.reload.slack_integration).to be_nil + end + end + + context 'when flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'does not create associated SlackIntegration record' do + execute_service + + expect(created_integration.reload.slack_integration).to be_nil + end + end + end + context 'with an instance-level integration' do let(:integration) { instance_integration } let(:inherit_from_id) { integration.id } + let_it_be_with_reload(:instance_slack_integration) do + create(:gitlab_slack_application_integration, :instance, + slack_integration: build(:slack_integration, + team_id: 'instance_team_id', + team_name: 'instance_team_name', + alias: 'instance_alias', + bot_access_token: 'instance_bot_access_token', + authorized_scope_names: %w[instance_scope1 instance_scope2] + ) + ) + end + context 'with a project association' do let!(:project) { create(:project) } - let(:created_integration) { project.jira_integration } + let(:created_integration) { Integration.find_by(project: project) } let(:batch) { Project.where(id: project.id) } let(:association) { 'project' } it_behaves_like 'creates integration successfully' + + it_behaves_like 'creates GitLab for Slack app data successfully' do + let(:integration) { instance_slack_integration } + let(:expected_alias) { project.full_path } + end end context 'with a group association' do @@ -88,16 +151,34 @@ def attributes(record) let(:association) { 'group' } it_behaves_like 'creates integration successfully' + + it_behaves_like 'creates GitLab for Slack app data successfully' do + let(:integration) { instance_slack_integration } + let(:expected_alias) { group.full_path } + end end end context 'with a group-level integration' do let_it_be(:group) { create(:group) } + let_it_be_with_reload(:group_slack_integration) do + create(:gitlab_slack_application_integration, :group, + group: group, + slack_integration: build(:slack_integration, + team_id: 'group_team_id', + team_name: 'group_team_name', + alias: 'group_alias', + bot_access_token: 'group_bot_access_token', + authorized_scope_names: %w[group_scope1 group_scope2] + ) + ) + end + context 'with a project association' do let!(:project) { create(:project, group: group) } let(:integration) { create(:jira_integration, :group, group: group) } - let(:created_integration) { project.jira_integration } + let(:created_integration) { Integration.find_by(project: project) } let(:batch) { Project.without_integration(integration).in_namespace(integration.group.self_and_descendants) } let(:association) { 'project' } let(:inherit_from_id) { integration.id } @@ -106,10 +187,14 @@ def attributes(record) context 'with different foreign key of data_fields' do let(:integration) { create(:zentao_integration, :group, group: group) } - let(:created_integration) { project.zentao_integration } it_behaves_like 'creates integration successfully' end + + it_behaves_like 'creates GitLab for Slack app data successfully' do + let(:integration) { group_slack_integration } + let(:expected_alias) { project.full_path } + end end context 'with a group association' do @@ -129,6 +214,11 @@ def attributes(record) it_behaves_like 'creates integration successfully' end + + it_behaves_like 'creates GitLab for Slack app data successfully' do + let(:integration) { group_slack_integration } + let(:expected_alias) { subgroup.full_path } + end end end end diff --git a/spec/services/integrations/propagation/bulk_update_service_spec.rb b/spec/services/integrations/propagation/bulk_update_service_spec.rb index 837ddbb64563b0779a9c5cd2733f7367b40060c1..98e31028ae01d65d90cd8574eed0747aa47e0f03 100644 --- a/spec/services/integrations/propagation/bulk_update_service_spec.rb +++ b/spec/services/integrations/propagation/bulk_update_service_spec.rb @@ -131,4 +131,162 @@ end.to change { integration.reload.url }.to(group_integration.url) end end + + context 'with a GitLab for Slack app integration' do + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project) { create(:project, group: subgroup) } + + let_it_be(:group_integration) do + create(:gitlab_slack_application_integration, :group, + group: group, + slack_integration: build(:slack_integration, + team_id: 'group_integration_team_id', + team_name: 'group_integration_team_name', + alias: 'group_alias', + bot_access_token: 'group_integration_token', + authorized_scope_names: %w[group_scope] + ) + ) + end + + let_it_be(:subgroup_integration) do + create(:gitlab_slack_application_integration, :group, + group: subgroup, + inherit_from_id: group_integration.id, + slack_integration: build(:slack_integration, + team_id: 'subgroup_integration_team_id', + team_name: 'subgroup_integration_team_name', + alias: 'subgroup_alias', + bot_access_token: 'subgroup_integration_token', + authorized_scope_names: %w[subgroup_scope] + ) + ) + end + + let_it_be(:integration) do + create(:gitlab_slack_application_integration, + project: project, + inherit_from_id: subgroup_integration.id, + slack_integration: build(:slack_integration, + alias: 'project_alias', + authorized_scope_names: %w[project_scope] + ) + ) + end + + let_it_be(:excluded_integration) do + create(:gitlab_slack_application_integration, + slack_integration: build(:slack_integration, + team_id: 'excluded_team_id', + alias: 'excluded_alias', + authorized_scope_names: %w[excluded_scope] + ) + ) + end + + let(:group_slack_integration) { group_integration.slack_integration } + + let(:batch) { Integration.id_in([subgroup_integration, integration]) } + + subject(:execute_service) do + described_class.new(group_integration, batch).execute + end + + it 'updates the SlackIntegration records and scopes, but not aliases' do + execute_service + + expect(subgroup_integration.reload.slack_integration).to have_attributes( + team_id: group_slack_integration.team_id, + team_name: group_slack_integration.team_name, + alias: 'subgroup_alias', + user_id: group_slack_integration.user_id, + bot_user_id: group_slack_integration.bot_user_id, + bot_access_token: group_slack_integration.bot_access_token, + created_at: be_present, + updated_at: be_present, + authorized_scope_names: group_slack_integration.authorized_scope_names + ) + + expect(integration.reload.slack_integration).to have_attributes( + team_id: group_slack_integration.team_id, + team_name: group_slack_integration.team_name, + alias: 'project_alias', + user_id: group_slack_integration.user_id, + bot_user_id: group_slack_integration.bot_user_id, + bot_access_token: group_slack_integration.bot_access_token, + created_at: be_present, + updated_at: be_present, + authorized_scope_names: group_slack_integration.authorized_scope_names + ) + + expect(excluded_integration.reload.slack_integration).to have_attributes( + team_id: 'excluded_team_id', + alias: 'excluded_alias', + authorized_scope_names: %w[excluded_scope] + ) + end + + context 'when integration is disabled' do + before do + group_integration.update!(active: false) + end + + it 'deletes associated SlackIntegration records' do + expect { execute_service }.to change { SlackIntegration.count }.by(-2) + expect(integration.reload.slack_integration).to be_nil + expect(subgroup_integration.reload.slack_integration).to be_nil + expect(excluded_integration.reload.slack_integration).to be_kind_of(SlackIntegration) + end + + it 'deletes associated IntegrationApiScope records' do + expect { execute_service } + .to change { Integrations::SlackWorkspace::IntegrationApiScope.count } + .by(-2) + + ids = Integrations::SlackWorkspace::IntegrationApiScope.pluck(:slack_integration_id) + + expect(ids).to contain_exactly( + group_integration.slack_integration.id, + excluded_integration.slack_integration.id + ) + end + end + + it 'avoids N+1 database queries' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { execute_service } + + project_2 = create(:project, group: subgroup) + project_2_integration = create(:gitlab_slack_application_integration, + project: project_2, + inherit_from_id: subgroup_integration.id, + slack_integration: build(:slack_integration, + alias: 'project_2_alias', + authorized_scope_names: %w[project_2_scope] + ) + ) + + batch = Integration.id_in([subgroup_integration, integration, project_2_integration]) + + expect do + described_class.new(group_integration, batch).execute + end.to issue_same_number_of_queries_as(control) + expect(project_2_integration.reload.slack_integration).to have_attributes( + team_id: group_slack_integration.team_id, + authorized_scope_names: group_slack_integration.authorized_scope_names + ) + end + + context 'when flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'does not update associated SlackIntegration record or scopes' do + expect { execute_service }.not_to change { integration.reload.slack_integration.attributes } + expect(integration.slack_integration.authorized_scope_names).to eq( + %w[project_scope] + ) + end + end + end end diff --git a/spec/services/integrations/slack_installation/group_service_spec.rb b/spec/services/integrations/slack_installation/group_service_spec.rb index a2e0a9f35e84234af24ee1ca59d193668450d451..d13de025fb64b125517297edfd3235a2c253b58a 100644 --- a/spec/services/integrations/slack_installation/group_service_spec.rb +++ b/spec/services/integrations/slack_installation/group_service_spec.rb @@ -17,6 +17,7 @@ let(:installation_alias) { group.full_path } let(:integration) { Integrations::GitlabSlackApplication.for_group(group).first } let(:redirect_url) { Gitlab::Routing.url_helpers.slack_auth_group_settings_slack_url(group) } + let(:enqueues_propagation_worker) { true } def create_gitlab_slack_application_integration! Integrations::GitlabSlackApplication.create!(group: group) diff --git a/spec/services/integrations/slack_installation/instance_service_spec.rb b/spec/services/integrations/slack_installation/instance_service_spec.rb index a51ace31916299192a11bb7ff2f962ca8fca955a..b4dee6e88d8fde39bc257b200388d789545726db 100644 --- a/spec/services/integrations/slack_installation/instance_service_spec.rb +++ b/spec/services/integrations/slack_installation/instance_service_spec.rb @@ -12,6 +12,7 @@ let(:installation_alias) { '_gitlab-instance' } let(:integration) { Integrations::GitlabSlackApplication.for_instance.first } let(:redirect_url) { Gitlab::Routing.url_helpers.slack_auth_admin_application_settings_slack_url } + let(:enqueues_propagation_worker) { true } def create_gitlab_slack_application_integration! Integrations::GitlabSlackApplication.create!(instance: true) diff --git a/spec/services/integrations/slack_installation/project_service_spec.rb b/spec/services/integrations/slack_installation/project_service_spec.rb index 58f4785624cef31850a80fa5a9eb5ce3e14e462c..483a7046d6600b4951a0d25e8963e6fae39a6ae0 100644 --- a/spec/services/integrations/slack_installation/project_service_spec.rb +++ b/spec/services/integrations/slack_installation/project_service_spec.rb @@ -17,6 +17,7 @@ let(:installation_alias) { project.full_path } let(:integration) { project.gitlab_slack_application_integration } let(:redirect_url) { Gitlab::Routing.url_helpers.slack_auth_project_settings_slack_url(project) } + let(:enqueues_propagation_worker) { false } def create_gitlab_slack_application_integration! project.create_gitlab_slack_application_integration! diff --git a/spec/support/shared_examples/requests/integrations/slack_controller_settings_shared_examples.rb b/spec/support/shared_examples/requests/integrations/slack_controller_settings_shared_examples.rb index f8a397c986b91dab80a7a88a80f542b5923d72db..d8e1c8eba998fd148d0c613d7b816ee384c1920b 100644 --- a/spec/support/shared_examples/requests/integrations/slack_controller_settings_shared_examples.rb +++ b/spec/support/shared_examples/requests/integrations/slack_controller_settings_shared_examples.rb @@ -92,14 +92,24 @@ describe 'DELETE destroy' do subject(:delete_destroy) { delete destroy_path } - it 'destroys the record and redirects back to #edit' do - integration = create_integration + let!(:integration) { create_integration } + it 'destroys the record and redirects back to #edit' do expect { delete_destroy }.to change { integration.reload.slack_integration }.to(nil) expect(response).to have_gitlab_http_status(:found) expect(response).to redirect_to(redirect_url) end + it 'enqueues a worker job' do + if propagates_on_destroy + expect(PropagateIntegrationWorker).to receive(:perform_async).with(integration.id) + else + expect(PropagateIntegrationWorker).not_to receive(:perform_async) + end + + delete_destroy + end + context 'when the flag is disabled' do before do skip unless flag_protected @@ -107,8 +117,6 @@ end it 'responds with status :not_found' do - integration = create_integration - expect { delete_destroy } .not_to change { integration.reload.slack_integration } .from(kind_of(SlackIntegration)) diff --git a/spec/support/shared_examples/services/integrations/slack_installation_shared_examples.rb b/spec/support/shared_examples/services/integrations/slack_installation_shared_examples.rb index 3dd17d59cdf0fc4d6d9b8712d21fe7c18c3758f9..4c607f7b4c0da64e0b673b6224810a90e681c0f2 100644 --- a/spec/support/shared_examples/services/integrations/slack_installation_shared_examples.rb +++ b/spec/support/shared_examples/services/integrations/slack_installation_shared_examples.rb @@ -150,6 +150,18 @@ end end + it 'handles propagation correctly' do + allow(PropagateIntegrationWorker).to receive(:perform_async) + + service.execute + + if enqueues_propagation_worker + expect(PropagateIntegrationWorker).to have_received(:perform_async).with(integration.id) + else + expect(PropagateIntegrationWorker).not_to have_received(:perform_async) + end + end + context 'when the team has other Slack installation records' do let_it_be_with_reload(:other_installation) { create(:slack_integration, team_id: team_id) } let_it_be_with_reload(:other_legacy_installation) { create(:slack_integration, :legacy, team_id: team_id) } diff --git a/spec/workers/propagate_integration_worker_spec.rb b/spec/workers/propagate_integration_worker_spec.rb index e097deb53a6f356fc869984c29e1cd70afbc90b9..147d3a03cc0a391131528af83d5484751579502c 100644 --- a/spec/workers/propagate_integration_worker_spec.rb +++ b/spec/workers/propagate_integration_worker_spec.rb @@ -3,20 +3,7 @@ require 'spec_helper' RSpec.describe PropagateIntegrationWorker, feature_category: :integrations do - describe '#perform' do - let(:project) { create(:project) } - let(:integration) do - Integrations::Pushover.create!( - project: project, - active: true, - device: 'MyDevice', - sound: 'mic', - priority: 4, - user_key: 'asdf', - api_key: '123456789' - ) - end - + shared_examples 'propagated integration' do it 'calls the propagate service with the integration' do expect_next_instance_of(Integrations::PropagateService, integration) do |service| expect(service).to receive(:execute) @@ -24,11 +11,39 @@ subject.perform(integration.id) end + end - it 'does nothing when the integration does not exist' do + shared_examples 'not-propagated integration' do + it 'does not call the propagate service' do expect(Integrations::PropagateService).not_to receive(:new) - subject.perform(non_existing_record_id) + subject.perform(integration.id) + end + end + + describe '#perform' do + context 'with integration on instance level' do + let(:integration) { create(:pushover_integration, :instance) } + + it_behaves_like 'propagated integration' + end + + context 'with integration on group level' do + let(:integration) { create(:pushover_integration, :group) } + + it_behaves_like 'propagated integration' + end + + context 'with integration on project level' do + let(:integration) { create(:pushover_integration) } + + it_behaves_like 'not-propagated integration' + end + + context 'when integration does not exist' do + let(:integration) { build(:pushover_integration) } + + it_behaves_like 'not-propagated integration' end end end