diff --git a/app/controllers/admin/slacks_controller.rb b/app/controllers/admin/slacks_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..a03b87b427247eb4333b14052d37afc83447451b --- /dev/null +++ b/app/controllers/admin/slacks_controller.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Admin + class SlacksController < Admin::ApplicationController + before_action do + render_404 if Feature.disabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) + end + + include ::Integrations::SlackControllerSettings + + def slack_auth; end + + private + + def integration + @integration ||= Integrations::GitlabSlackApplication.for_instance.first + end + + def redirect_to_integration_page + redirect_to edit_admin_application_settings_integration_path( + integration || Integrations::GitlabSlackApplication.for_instance.new + ) + end + end +end diff --git a/app/controllers/concerns/integrations/slack_controller_settings.rb b/app/controllers/concerns/integrations/slack_controller_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..8a0e310b6d4b2c24701e0e26bd696b3ca395078d --- /dev/null +++ b/app/controllers/concerns/integrations/slack_controller_settings.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# Shared concern for controllers to handle editing the GitLab for Slack app +# integration at project, group and instance-levels. +# +# Controllers should define these methods: +# - `#integration` to return the Integrations::GitLabSlackApplication record. +# - `#redirect_to_integration_page` to redirect to the integration edit page +module Integrations + module SlackControllerSettings + extend ActiveSupport::Concern + + included do + feature_category :integrations + end + + def destroy + slack_integration.destroy + + redirect_to_integration_page + end + + private + + def slack_integration + @slack_integration ||= integration.slack_integration + end + end +end diff --git a/app/controllers/groups/settings/slacks_controller.rb b/app/controllers/groups/settings/slacks_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..771fc5170169e5f1a3b1f7cb55af33fa43668164 --- /dev/null +++ b/app/controllers/groups/settings/slacks_controller.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Groups + module Settings + class SlacksController < Groups::ApplicationController + before_action :authorize_admin_group! + + before_action do + render_404 if Feature.disabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) + end + + include ::Integrations::SlackControllerSettings + + layout 'group_settings' + + def slack_auth; end + + private + + def integration + @integration ||= Integrations::GitlabSlackApplication.for_group(group).first + end + + def redirect_to_integration_page + redirect_to edit_group_settings_integration_path( + group, integration || Integrations::GitlabSlackApplication.for_group(group).new + ) + end + end + end +end diff --git a/app/controllers/projects/settings/slacks_controller.rb b/app/controllers/projects/settings/slacks_controller.rb index 33c290c51c71659cad59d7f6a988b1eb8d81366a..32dece1b40ec7e2576c0bfee4bc7c2f399e6435f 100644 --- a/app/controllers/projects/settings/slacks_controller.rb +++ b/app/controllers/projects/settings/slacks_controller.rb @@ -5,26 +5,22 @@ module Settings class SlacksController < Projects::ApplicationController before_action :handle_oauth_error, only: :slack_auth before_action :check_oauth_state, only: :slack_auth + + include ::Integrations::SlackControllerSettings + before_action :authorize_admin_project! + before_action :integration, only: [:edit, :update] before_action :slack_integration, only: [:edit, :update] layout 'project_settings' - feature_category :integrations - def slack_auth result = Projects::SlackApplicationInstallService.new(project, current_user, params).execute flash[:alert] = result[:message] if result[:status] == :error session[:slack_install_success] = true - redirect_to_service_page - end - - def destroy - slack_integration.destroy - - redirect_to_service_page + redirect_to_integration_page end def edit; end @@ -33,7 +29,7 @@ def update if slack_integration.update(slack_integration_params) flash[:notice] = 'The project alias was updated successfully' - redirect_to_service_page + redirect_to_integration_page else render :edit end @@ -41,10 +37,13 @@ def update private - def redirect_to_service_page + def integration + @integration ||= project.gitlab_slack_application_integration + end + + def redirect_to_integration_page redirect_to edit_project_settings_integration_path( - project, - project.gitlab_slack_application_integration || project.build_gitlab_slack_application_integration + project, integration || project.build_gitlab_slack_application_integration ) end @@ -58,11 +57,7 @@ def handle_oauth_error return unless params[:error] == 'access_denied' flash[:alert] = 'Access denied' - redirect_to_service_page - end - - def slack_integration - @slack_integration ||= project.gitlab_slack_application_integration.slack_integration + redirect_to_integration_page end def slack_integration_params diff --git a/app/graphql/types/projects/service_type_enum.rb b/app/graphql/types/projects/service_type_enum.rb index 7a8863c7d672237972416ebec8f6d42f2cc36d8e..6c576b3ddcd84cd6cc0aecaf5aab3d95509772c6 100644 --- a/app/graphql/types/projects/service_type_enum.rb +++ b/app/graphql/types/projects/service_type_enum.rb @@ -17,7 +17,9 @@ def type_description(name, type) # This prepend must stay here because the dynamic block below depends on it. prepend_mod # rubocop: disable Cop/InjectEnterpriseEditionModule - ::Integration.available_integration_names(include_instance_specific: false, include_dev: false).each do |name| + ::Integration.available_integration_names( + include_instance_specific: false, include_dev: false, include_disabled: true + ).each do |name| type = "#{name.camelize}Service" domain_value = Integration.integration_name_to_type(name) value type.underscore.upcase, value: domain_value, description: type_description(name, type) diff --git a/app/helpers/integrations_helper.rb b/app/helpers/integrations_helper.rb index 510561ec61483ac59f0bb958ee8b2c0161e46c50..89c1f6a64bf9aa2a10b519ba76a86f273ee3b19b 100644 --- a/app/helpers/integrations_helper.rb +++ b/app/helpers/integrations_helper.rb @@ -137,7 +137,7 @@ def integration_form_data(integration, project: nil, group: nil) end if integration.is_a?(::Integrations::GitlabSlackApplication) - form_data[:upgrade_slack_url] = add_to_slack_link(project, slack_app_id) + form_data[:upgrade_slack_url] = add_to_slack_link(integration.parent, slack_app_id) form_data[:should_upgrade_slack] = integration.upgrade_needed?.to_s end @@ -218,17 +218,28 @@ def integration_webhook_event_human_name(event) event_i18n_map[event] || event.to_s.humanize end - def add_to_slack_link(project, slack_app_id) + def add_to_slack_link(parent, slack_app_id) query = { scope: SlackIntegration::SCOPES.join(','), client_id: slack_app_id, - redirect_uri: slack_auth_project_settings_slack_url(project), + redirect_uri: add_to_slack_link_redirect_url(parent), state: form_authenticity_token } "#{::Projects::SlackApplicationInstallService::SLACK_AUTHORIZE_URL}?#{query.to_query}" end + def slack_integration_destroy_path(parent) + case parent + when Project + project_settings_slack_path(parent) + when Group + group_settings_slack_path(parent) + when nil + admin_application_settings_slack_path + end + end + def gitlab_slack_application_data(projects) { projects: (projects || []).to_json(only: [:id, :name], methods: [:avatar_url, :name_with_namespace]), @@ -244,6 +255,17 @@ def gitlab_slack_application_data(projects) private + def add_to_slack_link_redirect_url(parent) + case parent + when Project + slack_auth_project_settings_slack_url(parent) + when Group + slack_auth_group_settings_slack_url(parent) + when nil + slack_auth_admin_application_settings_slack_url + end + end + def jira_integration_event_description(event) case event when "merge_request", "merge_request_events" diff --git a/app/models/integration.rb b/app/models/integration.rb index 2491e961bc3aff87db615232fa9bbef4fa9470a1..b1725cdf87dab2b0882389cd55d11ae36e52ad4e 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -20,7 +20,8 @@ class Integration < ApplicationRecord INTEGRATION_NAMES = %w[ asana assembla bamboo bugzilla buildkite campfire clickup confluence custom_issue_tracker - datadog diffblue_cover discord drone_ci emails_on_push ewm external_wiki hangouts_chat harbor irker jira + datadog diffblue_cover discord drone_ci emails_on_push ewm external_wiki + gitlab_slack_application hangouts_chat harbor irker jira mattermost mattermost_slash_commands microsoft_teams packagist pipelines_email pivotaltracker prometheus pumble pushover redmine slack slack_slash_commands squash_tm teamcity telegram unify_circuit webex_teams youtrack zentao @@ -32,7 +33,7 @@ class Integration < ApplicationRecord # See: https://gitlab.com/gitlab-org/gitlab/-/issues/345677 PROJECT_SPECIFIC_INTEGRATION_NAMES = %w[ - apple_app_store gitlab_slack_application google_play jenkins + apple_app_store google_play jenkins ].freeze # Fake integrations to help with local development. @@ -311,18 +312,26 @@ def self.nonexistent_integration_types_for(scope, include_instance_specific: fal # Returns a list of available integration names. # Example: ["asana", ...] def self.available_integration_names( - include_project_specific: true, include_dev: true, include_instance_specific: true + include_project_specific: true, include_dev: true, include_instance_specific: true, include_disabled: false ) names = integration_names names += project_specific_integration_names if include_project_specific names += dev_integration_names if include_dev names += instance_specific_integration_names if include_instance_specific + names -= disabled_integration_names unless include_disabled names.sort_by(&:downcase) end def self.integration_names - INTEGRATION_NAMES + names = INTEGRATION_NAMES.dup + + unless Feature.enabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) && + (Gitlab::CurrentSettings.slack_app_enabled || Gitlab.dev_or_test_env?) + names.delete('gitlab_slack_application') + end + + names end def self.instance_specific_integration_names @@ -337,7 +346,12 @@ def self.dev_integration_names def self.project_specific_integration_names names = PROJECT_SPECIFIC_INTEGRATION_NAMES.dup - names.delete('gitlab_slack_application') unless Gitlab::CurrentSettings.slack_app_enabled || Gitlab.dev_or_test_env? + + if Feature.disabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) && + (Gitlab::CurrentSettings.slack_app_enabled || Gitlab.dev_or_test_env?) + names << 'gitlab_slack_application' + end + names end @@ -349,6 +363,15 @@ def self.available_integration_types(...) end end + # Returns a list of disabled integration names. + # Example: ["gitlab_slack_application", ...] + def self.disabled_integration_names + # The GitLab for Slack app integration is only available when enabled through settings. + # The Slack Slash Commands integration is only available for customers who cannot use the GitLab for Slack app. + Gitlab::CurrentSettings.slack_app_enabled ? ['slack_slash_commands'] : ['gitlab_slack_application'] + end + private_class_method :disabled_integration_names + # Returns the model for the given integration name. # Example: :asana => Integrations::Asana def self.integration_name_to_model(name) @@ -360,7 +383,7 @@ def self.integration_name_to_model(name) # Example: "asana" => "Integrations::Asana" def self.integration_name_to_type(name) name = name.to_s - if available_integration_names.exclude?(name) + if available_integration_names(include_disabled: true).exclude?(name) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownType.new(name.inspect)) else "Integrations::#{name.camelize}" diff --git a/app/models/project.rb b/app/models/project.rb index c255ccfc7fa9a10f624809a2ce6dc3abc29794a2..8d462df3ab0cdde6c4c89cf76f81ca4bb30c3112 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1749,14 +1749,12 @@ def find_or_initialize_integrations .sort_by(&:title) end + # Returns a list of integration names that should be disabled at the project-level. + # Globally disabled integrations should go in Integration.disabled_integration_names. def disabled_integrations return [] if Rails.env.development? - names = %w[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') + %w[zentao] end def find_or_initialize_integration(name) diff --git a/app/views/shared/integrations/gitlab_slack_application/_help.html.haml b/app/views/shared/integrations/gitlab_slack_application/_help.html.haml index 2e7768e54f481ac85a7d21fb4643a5bd41797fda..ae0e535efa0c01cf115a6d77f91d13946c427f89 100644 --- a/app/views/shared/integrations/gitlab_slack_application/_help.html.haml +++ b/app/views/shared/integrations/gitlab_slack_application/_help.html.haml @@ -1,10 +1,10 @@ .info-well .well-segment %p - = s_("SlackIntegration|This integration allows users to perform common operations on this project by entering slash commands in Slack.") + = s_("SlackIntegration|This integration allows users to perform common operations on their projects by entering slash commands in Slack.") = link_to _('Learn more'), help_page_path('user/project/integrations/gitlab_slack_application') %p = s_("SlackIntegration|See the list of available commands in Slack after setting up this integration by entering") %kbd.inline /gitlab help -- if integration.project_level? +- if integration.project_level? || Feature.enabled?(:gitlab_for_slack_app_instance_and_group_level, type: :wip) = render "shared/integrations/#{integration.to_param}/slack_integration_form", integration: integration 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 7f708a1e730d275bcba9265a0da25c3221a74302..ca9a9aef33f3b1b1f614060cff7857c637658404 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,35 +1,33 @@ - slack_integration = integration.slack_integration - if slack_integration %table.gl-table.gl-w-full - %colgroup - %col{ width: "25%" } - %col{ width: "35%" } - %col{ width: "20%" } - %col %thead %tr - %th= s_('SlackIntegration|Team name') - %th= s_('SlackIntegration|Project alias') + %th= s_('SlackIntegration|Workspace name') + - if integration.project_level? + %th + = s_('SlackIntegration|Project alias') %th= _('Created') %th %tr %td{ class: 'gl-py-3!' } = slack_integration.team_name - %td{ class: 'gl-py-3!' } - = slack_integration.alias + - if integration.project_level? + %td{ class: 'gl-py-3!' } + = slack_integration.alias %td{ class: 'gl-py-3!' } = time_ago_with_tooltip(slack_integration.created_at) %td{ class: 'gl-py-3!' } - .controls.gl-display-flex.gl-gap-3 - - project = integration.project - = render Pajamas::ButtonComponent.new(href: edit_project_settings_slack_path(project)) do - = _('Edit') - = render Pajamas::ButtonComponent.new(method: :delete, category: 'secondary', variant: "danger", href: project_settings_slack_path(project), icon: 'remove', button_options: { aria: { label: s_('SlackIntegration|Remove project') }, data: { confirm_btn_variant: "danger", confirm: s_('SlackIntegration|Are you sure you want to remove this project from the GitLab for Slack app?') }}) + .controls.gl-display-flex.gl-justify-content-end.gl-gap-3 + - 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?') }}) .gl-my-5 - = render Pajamas::ButtonComponent.new(href: add_to_slack_link(@project, slack_app_id)) do + = render Pajamas::ButtonComponent.new(href: add_to_slack_link(integration.parent, slack_app_id)) 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: %(<a href="#{help_page_path('user/project/integrations/gitlab_slack_application', anchor: 'reinstall-the-gitlab-for-slack-app')}">).html_safe, linkEnd: '</a>'.html_safe} - else - = render Pajamas::ButtonComponent.new(href: add_to_slack_link(@project, slack_app_id)) do + = render Pajamas::ButtonComponent.new(href: add_to_slack_link(integration.parent, slack_app_id)) do = s_('SlackIntegration|Install GitLab for Slack app…') diff --git a/config/feature_flags/wip/gitlab_for_slack_app_instance_and_group_level.yml b/config/feature_flags/wip/gitlab_for_slack_app_instance_and_group_level.yml new file mode 100644 index 0000000000000000000000000000000000000000..4d7ad1715404a60311f536fcf61851d8f9d0ecc1 --- /dev/null +++ b/config/feature_flags/wip/gitlab_for_slack_app_instance_and_group_level.yml @@ -0,0 +1,9 @@ +--- +name: gitlab_for_slack_app_instance_and_group_level +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/391526 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142868 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17410 +milestone: '16.10' +group: group::import and integrate +type: wip +default_enabled: false diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 44e2a0f5d533d76299e8138f662f6ae8f1569442..6de58cbada2c9b32eda010ba3203bcb67549b96e 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -156,6 +156,10 @@ end end + resource :slack, only: [:destroy] do + get :slack_auth + end + get :usage_data put :reset_registration_token put :reset_health_check_token diff --git a/config/routes/group.rb b/config/routes/group.rb index 115c6abeb5f5c1cc885ad6e57be7dac70fbd4ea2..2de5938a0e7a5551625636346169dd7e8c1850e1 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -60,6 +60,10 @@ end end + resource :slack, only: [:destroy] do + get :slack_auth + end + resources :applications do put 'renew', on: :member end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index e36bf9ff6adf303d6d84774696a2a428885c09ef..ae45269e08cea4bd7a2b52cd1a3a76793667f110 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -231,7 +231,7 @@ def topology_usage_data # rubocop: disable CodeReuse/ActiveRecord def integrations_usage # rubocop: disable UsageData/LargeTable: - Integration.available_integration_names(include_dev: false).each_with_object({}) do |name, response| + Integration.available_integration_names(include_dev: false, include_disabled: true).each_with_object({}) do |name, response| type = Integration.integration_name_to_type(name) response[:"projects_#{name}_active"] = count(Integration.active.where.not(project: nil).where(type: type)) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b08b9d371ec2d32c3a00c057e7db13a373120a7b..95b767dfbcb20c2d1aa46dc3636dd7cf766b330f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -46988,7 +46988,7 @@ msgstr "" msgid "SlackIntegration|- *Slash commands:* Quickly open, access, or close issues from Slack using the `%{slash_command}` command. Streamline your GitLab deployments with ChatOps." msgstr "" -msgid "SlackIntegration|Are you sure you want to remove this project from the GitLab for Slack app?" +msgid "SlackIntegration|Are you sure you want to unlink this Slack Workspace from this integration?" msgstr "" msgid "SlackIntegration|Client ID" @@ -47039,9 +47039,6 @@ msgstr "" msgid "SlackIntegration|Reinstall GitLab for Slack app…" msgstr "" -msgid "SlackIntegration|Remove project" -msgstr "" - msgid "SlackIntegration|Run ChatOps jobs." msgstr "" @@ -47063,10 +47060,7 @@ msgstr "" msgid "SlackIntegration|Step 2: Configure the app settings" msgstr "" -msgid "SlackIntegration|Team name" -msgstr "" - -msgid "SlackIntegration|This integration allows users to perform common operations on this project by entering slash commands in Slack." +msgid "SlackIntegration|This integration allows users to perform common operations on their projects by entering slash commands in Slack." msgstr "" msgid "SlackIntegration|Update to the latest version" @@ -47096,6 +47090,9 @@ msgstr "" msgid "SlackIntegration|When GitLab releases new features for the GitLab for Slack app, you might have to manually update your copy to use the new features." msgstr "" +msgid "SlackIntegration|Workspace name" +msgstr "" + msgid "SlackIntegration|You can now close this window and go to your Slack workspace." msgstr "" diff --git a/spec/features/admin/integrations/admin_manages_gitlab_for_slack_app_spec.rb b/spec/features/admin/integrations/admin_manages_gitlab_for_slack_app_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3ca1ddf0b0881aabf20ed1016e0165841b869c88 --- /dev/null +++ b/spec/features/admin/integrations/admin_manages_gitlab_for_slack_app_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Admin manages the instance-level GitLab for Slack app integration', :js, feature_category: :integrations do + include Spec::Support::Helpers::ModalHelpers + + include_context 'instance integration activation' + + let_it_be(:integration) do + create(:gitlab_slack_application_integration, :instance, + slack_integration: build(:slack_integration) + ) + end + + before do + stub_application_setting(slack_app_enabled: true) + end + + def visit_slack_application_form + visit_instance_integration('GitLab for Slack app') + wait_for_requests + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'hides the integration' do + visit_instance_integrations + + expect(page).not_to have_content('GitLab for Slack app') + end + end + + it 'shows the workspace name but not the alias and does not allow the user to edit it' do + visit_slack_application_form + + within_testid 'integration-settings-form' do + expect(page).to have_content('Workspace name') + expect(page).to have_content(integration.slack_integration.team_name) + expect(page).not_to have_content('Project alias') + expect(page).not_to have_content(integration.slack_integration.alias) + expect(page).not_to have_content('Edit') + end + end + + 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 + + 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 + + it 'shows the trigger form fields' do + visit_slack_application_form + + expect(page).to have_selector('[data-testid="trigger-fields-group"]') + end + + context 'when the integration is disabled' do + before do + Integrations::GitlabSlackApplication.for_instance.first.update!(active: false) + end + + it 'does not show the trigger form fields' do + visit_slack_application_form + + expect(page).not_to have_selector('[data-testid="trigger-fields-group"]') + end + end +end 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 new file mode 100644 index 0000000000000000000000000000000000000000..1829acb8954004b17c0af634992f48c2fc7fec42 --- /dev/null +++ b/spec/features/groups/integrations/user_manages_gitlab_for_slack_app_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User manages the group-level GitLab for Slack app integration', :js, feature_category: :integrations do + include Spec::Support::Helpers::ModalHelpers + + include_context 'group integration activation' + + let_it_be(:integration) do + create(:gitlab_slack_application_integration, :group, group: group, + slack_integration: build(:slack_integration) + ) + end + + before do + stub_application_setting(slack_app_enabled: true) + end + + def visit_slack_application_form + visit_group_integration('GitLab for Slack app') + wait_for_requests + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'hides the integration' do + visit_group_integrations + + expect(page).not_to have_content('GitLab for Slack app') + end + end + + it 'shows the workspace name but not the alias and does not allow the user to edit it' do + visit_slack_application_form + + within_testid 'integration-settings-form' do + expect(page).to have_content('Workspace name') + expect(page).to have_content(integration.slack_integration.team_name) + expect(page).not_to have_content('Project alias') + expect(page).not_to have_content(integration.slack_integration.alias) + expect(page).not_to have_content('Edit') + end + end + + 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 + + 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 + + it 'shows the trigger form fields' do + visit_slack_application_form + + expect(page).to have_selector('[data-testid="trigger-fields-group"]') + end + + context 'when the integration is disabled' do + before do + Integrations::GitlabSlackApplication.for_group(group).first.update!(active: false) + end + + it 'does not show the trigger form fields' do + visit_slack_application_form + + expect(page).not_to have_selector('[data-testid="trigger-fields-group"]') + end + end +end diff --git a/spec/features/projects/settings/slack_application_spec.rb b/spec/features/projects/settings/slack_application_spec.rb index e2d195cdc263c31e06450e289e206658b96f4327..4d1aa17e9866b6733a4b5b49470d7f3a5f72ad02 100644 --- a/spec/features/projects/settings/slack_application_spec.rb +++ b/spec/features/projects/settings/slack_application_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'Slack application', :js, feature_category: :integrations do + include Spec::Support::Helpers::ModalHelpers + let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user, maintainer_projects: [project]) } let_it_be(:integration) { create(:gitlab_slack_application_integration, project: project) } @@ -14,10 +16,20 @@ gitlab_sign_in(user) end - it 'I can edit slack integration' do + def visit_slack_application_form visit slack_application_form_path + wait_for_requests + end + + it 'shows the workspace name and alias and allows the user to edit it' do + visit_slack_application_form within_testid 'integration-settings-form' do + expect(page).to have_content('Workspace name') + expect(page).to have_content(integration.slack_integration.team_name) + expect(page).to have_content('Project alias') + expect(page).to have_content(integration.slack_integration.alias) + click_link 'Edit' end @@ -31,8 +43,25 @@ end end + 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 + + 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 + it 'shows the trigger form fields' do - visit slack_application_form_path + visit_slack_application_form expect(page).to have_selector('[data-testid="trigger-fields-group"]') end @@ -43,6 +72,8 @@ end it 'does not show the trigger form fields' do + visit_slack_application_form + expect(page).not_to have_selector('[data-testid="trigger-fields-group"]') end end diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb index 7c626743f3a093adacc1b956f02aeb3844598583..4b87df0a28fc0715897bc580c63cf709bb04d9a6 100644 --- a/spec/helpers/integrations_helper_spec.rb +++ b/spec/helpers/integrations_helper_spec.rb @@ -222,15 +222,16 @@ end describe '#add_to_slack_link' do - let(:slack_link) { helper.add_to_slack_link(project, 'A12345') } + subject(:slack_link) { helper.add_to_slack_link(project, 'A12345') } + let(:query) { Rack::Utils.parse_query(URI.parse(slack_link).query) } before do allow(helper).to receive(:form_authenticity_token).and_return('a token') - allow(helper).to receive(:slack_auth_project_settings_slack_url).and_return('http://redirect') end it 'returns the endpoint URL with all needed params' do + expect(helper).to receive(:slack_auth_project_settings_slack_url).and_return('http://redirect') expect(slack_link).to start_with(Projects::SlackApplicationInstallService::SLACK_AUTHORIZE_URL) expect(slack_link).to include('&state=a+token') @@ -241,6 +242,55 @@ 'state' => 'a token' ) end + + context 'when passed a group' do + subject(:slack_link) { helper.add_to_slack_link(build_stubbed(:group), 'A12345') } + + it 'returns the endpoint URL a redirect-uri for the group' do + expect(helper).to receive(:slack_auth_group_settings_slack_url).and_return('http://group-redirect') + expect(query).to include('redirect_uri' => 'http://group-redirect') + end + end + + context 'when passed nil' do + subject(:slack_link) { helper.add_to_slack_link(nil, 'A12345') } + + it 'returns the endpoint URL a redirect-uri for the instance' do + expect(helper).to receive(:slack_auth_admin_application_settings_slack_url).and_return('http://instance-redirect') + expect(query).to include('redirect_uri' => 'http://instance-redirect') + end + end + end + + describe '#slack_integration_destroy_path' do + subject(:destroy_path) { helper.slack_integration_destroy_path(parent) } + + context 'when parent is a project' do + let(:parent) { project } + + it 'returns the correct path' do + expect(helper).to receive(:project_settings_slack_path).and_return('http://project-redirect') + expect(destroy_path).to eq('http://project-redirect') + end + end + + context 'when parent is a group' do + let(:parent) { build_stubbed(:group) } + + it 'returns the endpoint URL a redirect-uri for the group' do + expect(helper).to receive(:group_settings_slack_path).and_return('http://group-redirect') + expect(destroy_path).to eq('http://group-redirect') + end + end + + context 'when parent is nil' do + let(:parent) { nil } + + it 'returns the endpoint URL a redirect-uri for the instance' do + expect(helper).to receive(:admin_application_settings_slack_path).and_return('http://instance-redirect') + expect(destroy_path).to eq('http://instance-redirect') + end + end end describe '#gitlab_slack_application_data' do diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 8e7729b1468e53d64a58a83c665ee7877d919a86..be4df0ffe89154e53fb19957af0b7d78492aefa5 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -659,6 +659,12 @@ def integration_hash(type) .to raise_exception(described_class::UnknownType, /foo/) end + it 'does not raise an error if the name is a disabled integration' do + allow(described_class).to receive(:disabled_integration_names).and_return(['asana']) + + expect { described_class.integration_name_to_type('asana') }.not_to raise_exception + end + it 'handles all available_integration_names' do types = described_class.available_integration_names.map { described_class.integration_name_to_type(_1) } @@ -1002,10 +1008,11 @@ def fields subject { described_class.available_integration_names } before do - allow(described_class).to receive(:integration_names).and_return(%w[foo]) + allow(described_class).to receive(:integration_names).and_return(%w[foo disabled]) allow(described_class).to receive(:project_specific_integration_names).and_return(['bar']) allow(described_class).to receive(:dev_integration_names).and_return(['baz']) allow(described_class).to receive(:instance_specific_integration_names).and_return(['instance-specific']) + allow(described_class).to receive(:disabled_integration_names).and_return(['disabled']) end it { is_expected.to include('foo', 'bar', 'baz') } @@ -1014,28 +1021,34 @@ def fields subject { described_class.available_integration_names(include_project_specific: false) } it { is_expected.to include('foo', 'baz', 'instance-specific') } - it { is_expected.not_to include('bar') } + it { is_expected.not_to include('bar', 'disabled') } end context 'when `include_dev` is false' do subject { described_class.available_integration_names(include_dev: false) } it { is_expected.to include('foo', 'bar', 'instance-specific') } - it { is_expected.not_to include('baz') } + it { is_expected.not_to include('baz', 'disabled') } end context 'when `include_instance_specific` is false' do subject { described_class.available_integration_names(include_instance_specific: false) } it { is_expected.to include('foo', 'baz', 'bar') } - it { is_expected.not_to include('instance-specific') } + it { is_expected.not_to include('instance-specific', 'disabled') } + end + + context 'when `include_disabled` is true' do + subject { described_class.available_integration_names(include_disabled: true) } + + it { is_expected.to include('disabled') } end end - describe '.project_specific_integration_names' do - subject { described_class.project_specific_integration_names } + describe '.integration_names' do + subject { described_class.integration_names } - it { is_expected.to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) } + it { is_expected.to include(*described_class::INTEGRATION_NAMES) } it { is_expected.to include('gitlab_slack_application') } context 'when Rails.env is not test' do @@ -1051,7 +1064,55 @@ def fields end it { is_expected.to include('gitlab_slack_application') } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it { is_expected.not_to include('gitlab_slack_application') } + end + end + end + end + + describe '.project_specific_integration_names' do + subject { described_class.project_specific_integration_names } + + it { is_expected.to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) } + it { is_expected.not_to include('gitlab_slack_application') } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + 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 + + context 'when Rails.env is not test and `slack_app_enabled` setting is enabled' do + before do + allow(Rails.env).to receive(:test?).and_return(false) + stub_application_setting(slack_app_enabled: true) end + + it { is_expected.not_to include('gitlab_slack_application') } end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 936303d0be60e46186948cc951695337be1f25ce..8890561809e33849795696437ea6574e4744ccd6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6736,26 +6736,7 @@ def has_external_wiki 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 + it { is_expected.to include('zentao') } end describe '#find_or_initialize_integration' do diff --git a/spec/requests/admin/slacks_controller_spec.rb b/spec/requests/admin/slacks_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0c260f53e9cc4f0e931b634f5b3148a91f4d4eda --- /dev/null +++ b/spec/requests/admin/slacks_controller_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::SlacksController, :enable_admin_mode, feature_category: :integrations do + let_it_be(:admin) { create(:admin) } + let_it_be(:user) { create(:user) } + + before do + stub_application_setting(slack_app_enabled: true) + end + + def redirect_url + edit_admin_application_settings_integration_path( + Integrations::GitlabSlackApplication.to_param + ) + end + + describe 'DELETE destroy' do + subject(:destroy!) { delete admin_application_settings_slack_path } + + context 'when user is not an admin' do + before_all do + sign_in(user) + end + + it 'responds with status :not_found' do + destroy! + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is an admin' do + before do + sign_in(admin) + end + + it 'destroys the record and redirects back to #edit' do + create(:gitlab_slack_application_integration, :instance, + slack_integration: build(:slack_integration) + ) + + expect { destroy! } + .to change { Integrations::GitlabSlackApplication.for_instance.first&.slack_integration }.to(nil) + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(redirect_url) + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'responds with status :not_found' do + destroy! + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end +end diff --git a/spec/requests/groups/settings/slacks_controller_spec.rb b/spec/requests/groups/settings/slacks_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ea0f1440f8d29cc6c52fe98d74fd292b523e92ad --- /dev/null +++ b/spec/requests/groups/settings/slacks_controller_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::Settings::SlacksController, feature_category: :integrations do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + stub_application_setting(slack_app_enabled: true) + end + + def redirect_url(group) + edit_group_settings_integration_path( + group, + Integrations::GitlabSlackApplication.to_param + ) + end + + describe 'DELETE destroy' do + subject(:destroy!) { delete group_settings_slack_path(group) } + + context 'when user is not an admin' do + before_all do + group.add_developer(user) + end + + it 'responds with status :not_found' do + destroy! + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is an admin' do + before_all do + group.add_owner(user) + end + + it 'destroys the record and redirects back to #edit' do + create(:gitlab_slack_application_integration, :group, group: group, + slack_integration: build(:slack_integration) + ) + + expect { destroy! } + .to change { Integrations::GitlabSlackApplication.for_group(group).first&.slack_integration }.to(nil) + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(redirect_url(group)) + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(gitlab_for_slack_app_instance_and_group_level: false) + end + + it 'responds with status :not_found' do + destroy! + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end +end diff --git a/spec/support/shared_examples/integrations_shared_examples.rb b/spec/support/shared_examples/integrations_shared_examples.rb index 360ce711383135981279b187c9018511be05d9f5..cab7168dfaa384b267f789979eedf1557632d734 100644 --- a/spec/support/shared_examples/integrations_shared_examples.rb +++ b/spec/support/shared_examples/integrations_shared_examples.rb @@ -54,12 +54,26 @@ expect(response).to have_gitlab_http_status(expected_code) end - context 'when an integration is unavailable' do + context 'when an integration is disabled' do + before do + allow(Integration).to receive(:disabled_integration_names).and_return([integration.to_param]) + end + it 'returns bad request' do + put url, params: integration_attrs + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when an integration is disabled at the project-level' do + before do allow_next_found_instance_of(Project) do |project| allow(project).to receive(:disabled_integrations).and_return([integration]) end + end + it 'returns bad request' do put url, params: integration_attrs expect(response).to have_gitlab_http_status(:bad_request)