From 68fc1ac303accd7f9a72d68244f1cb4aebcbb720 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Wed, 19 May 2021 12:48:49 +1200 Subject: [PATCH 1/2] Bugfix `@service` being nil in integrations views In 5fd1c950e3e3ea5ad58cd2ad3e6c0f011874ebd2, `@service` was renamed in `IntegrationsActions` (used by admin and group integrations controllers) to `@integration`. We need to continue to support that variable as it is referred to in shared views. This fix applies the same approach as in the original commit to continue to support this variable: https://gitlab.com/gitlab-org/gitlab/-/blob/5fd1c950e3e3ea5ad58cd2ad3e6c0f011874ebd2/app/controllers/admin/services_controller.rb#L39 Removing the need for this will be addressed in: https://gitlab.com/gitlab-org/gitlab/-/issues/329759 https://gitlab.com/gitlab-org/gitlab/-/issues/331262 --- app/controllers/concerns/integrations_actions.rb | 3 +++ app/controllers/projects/services_controller.rb | 2 +- changelogs/unreleased/331262-bugfix-services-variable.yml | 5 +++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/331262-bugfix-services-variable.yml diff --git a/app/controllers/concerns/integrations_actions.rb b/app/controllers/concerns/integrations_actions.rb index f5a3ec913c2a60..fc8d4064b6521e 100644 --- a/app/controllers/concerns/integrations_actions.rb +++ b/app/controllers/concerns/integrations_actions.rb @@ -48,9 +48,12 @@ def reset private + # rubocop:disable Gitlab/ModuleWithInstanceVariables def integration @integration ||= find_or_initialize_non_project_specific_integration(params[:id]) + @service ||= @integration # TODO: remove references to @service https://gitlab.com/gitlab-org/gitlab/-/issues/329759 end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def success_message if integration.active? diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 74145a70b952b1..e72055c3d0f697 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -85,7 +85,7 @@ def success_message def integration @integration ||= @project.find_or_initialize_service(params[:id]) - @service ||= @integration # TODO: remove references to @service + @service ||= @integration # TODO: remove references to @service https://gitlab.com/gitlab-org/gitlab/-/issues/329759 end alias_method :service, :integration diff --git a/changelogs/unreleased/331262-bugfix-services-variable.yml b/changelogs/unreleased/331262-bugfix-services-variable.yml new file mode 100644 index 00000000000000..1702f676740f0a --- /dev/null +++ b/changelogs/unreleased/331262-bugfix-services-variable.yml @@ -0,0 +1,5 @@ +--- +title: Fix errors in instance and group-level integration pages for some integrations +merge_request: 62054 +author: +type: fixed -- GitLab From 7588b6b36dde6ebe1e48f7026023aeac63261e62 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Wed, 19 May 2021 12:56:29 +1200 Subject: [PATCH 2/2] Add group and instance level integration tests This adds an initial test of the Mattermost Slash Command integration for group- and instance-level. This is an initial effort in order to have coverage for https://gitlab.com/gitlab-org/gitlab/-/issues/331262. Further tests will be added in https://gitlab.com/gitlab-org/gitlab/-/issues/331325. --- ...activates_mattermost_slash_command_spec.rb | 16 +++++++++ ...activates_mattermost_slash_command_spec.rb | 16 +++++++++ ...activates_mattermost_slash_command_spec.rb | 31 ++--------------- .../group_integrations_shared_context.rb | 28 +++++++++++++++ ...e_and_group_integrations_shared_context.rb | 18 ++++++++++ .../instance_integrations_shared_context.rb | 24 +++++++++++++ .../integrations_shared_context.rb} | 6 ++++ .../project_integrations_jira_context.rb} | 0 .../project_integrations_shared_context.rb} | 6 ++-- ...ash_command_integration_shared_examples.rb | 34 +++++++++++++++++++ 10 files changed, 147 insertions(+), 32 deletions(-) create mode 100644 spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb create mode 100644 spec/features/groups/integrations/user_activates_mattermost_slash_command_spec.rb create mode 100644 spec/support/shared_contexts/features/integrations/group_integrations_shared_context.rb create mode 100644 spec/support/shared_contexts/features/integrations/instance_and_group_integrations_shared_context.rb create mode 100644 spec/support/shared_contexts/features/integrations/instance_integrations_shared_context.rb rename spec/support/shared_contexts/{services_shared_context.rb => features/integrations/integrations_shared_context.rb} (95%) rename spec/support/shared_contexts/{project_service_jira_context.rb => features/integrations/project_integrations_jira_context.rb} (100%) rename spec/support/shared_contexts/{project_service_shared_context.rb => features/integrations/project_integrations_shared_context.rb} (92%) create mode 100644 spec/support/shared_examples/features/integrations/user_activates_mattermost_slash_command_integration_shared_examples.rb diff --git a/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb b/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb new file mode 100644 index 00000000000000..6f091d37995031 --- /dev/null +++ b/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User activates the instance-level Mattermost Slash Command integration', :js do + include_context 'instance integration activation' + + before do + stub_mattermost_setting(enabled: true) + visit_instance_integration('Mattermost slash commands') + end + + let(:edit_path) { edit_admin_application_settings_integration_path(:mattermost_slash_commands) } + + include_examples 'user activates the Mattermost Slash Command integration' +end diff --git a/spec/features/groups/integrations/user_activates_mattermost_slash_command_spec.rb b/spec/features/groups/integrations/user_activates_mattermost_slash_command_spec.rb new file mode 100644 index 00000000000000..7703268af3944e --- /dev/null +++ b/spec/features/groups/integrations/user_activates_mattermost_slash_command_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User activates the group-level Mattermost Slash Command integration', :js do + include_context 'group integration activation' + + before do + stub_mattermost_setting(enabled: true) + visit_group_integration('Mattermost slash commands') + end + + let(:edit_path) { edit_group_settings_integration_path(group, :mattermost_slash_commands) } + + include_examples 'user activates the Mattermost Slash Command integration' +end diff --git a/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb b/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb index 54a501e89a2f46..84c9b8c6b54e80 100644 --- a/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb +++ b/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb @@ -14,35 +14,10 @@ context 'mattermost service is enabled' do let(:mattermost_enabled) { true } - it 'shows a help message' do - expect(page).to have_content("Use this service to perform common") - end - - it 'shows a token placeholder' do - token_placeholder = find_field('service_token')['placeholder'] - - expect(token_placeholder).to eq('XXxxXXxxXXxxXXxxXXxxXXxx') - end - - it 'redirects to the integrations page after saving but not activating' do - token = ('a'..'z').to_a.join - - fill_in 'service_token', with: token - click_active_checkbox - click_save_integration - - expect(current_path).to eq(edit_project_service_path(project, :mattermost_slash_commands)) - expect(page).to have_content('Mattermost slash commands settings saved, but not active.') - end - - it 'redirects to the integrations page after activating' do - token = ('a'..'z').to_a.join - - fill_in 'service_token', with: token - click_save_integration + describe 'activation' do + let(:edit_path) { edit_project_service_path(project, :mattermost_slash_commands) } - expect(current_path).to eq(edit_project_service_path(project, :mattermost_slash_commands)) - expect(page).to have_content('Mattermost slash commands settings saved and active.') + include_examples 'user activates the Mattermost Slash Command integration' end it 'shows the add to mattermost button' do diff --git a/spec/support/shared_contexts/features/integrations/group_integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/group_integrations_shared_context.rb new file mode 100644 index 00000000000000..5996fcc6593fa1 --- /dev/null +++ b/spec/support/shared_contexts/features/integrations/group_integrations_shared_context.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +RSpec.shared_context 'group integration activation' do + include_context 'instance and group integration activation' + + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + before_all do + group.add_owner(user) + end + + before do + sign_in(user) + end + + def visit_group_integrations + visit group_settings_integrations_path(group) + end + + def visit_group_integration(name) + visit_group_integrations + + within('#content-body') do + click_link(name) + end + end +end diff --git a/spec/support/shared_contexts/features/integrations/instance_and_group_integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/instance_and_group_integrations_shared_context.rb new file mode 100644 index 00000000000000..58ee341f71f3a2 --- /dev/null +++ b/spec/support/shared_contexts/features/integrations/instance_and_group_integrations_shared_context.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +RSpec.shared_context 'instance and group integration activation' do + include_context 'integration activation' + + def click_save_integration + click_save_changes_button + click_save_settings_modal + end + + def click_save_changes_button + click_button('Save changes') + end + + def click_save_settings_modal + click_button('Save') + end +end diff --git a/spec/support/shared_contexts/features/integrations/instance_integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/instance_integrations_shared_context.rb new file mode 100644 index 00000000000000..3b02db994a3d64 --- /dev/null +++ b/spec/support/shared_contexts/features/integrations/instance_integrations_shared_context.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.shared_context 'instance integration activation' do + include_context 'instance and group integration activation' + + let_it_be(:user) { create(:user, :admin) } + + before do + sign_in(user) + gitlab_enable_admin_mode_sign_in(user) + end + + def visit_instance_integrations + visit integrations_admin_application_settings_path + end + + def visit_instance_integration(name) + visit_instance_integrations + + within('#content-body') do + click_link(name) + end + end +end diff --git a/spec/support/shared_contexts/services_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb similarity index 95% rename from spec/support/shared_contexts/services_shared_context.rb rename to spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index 34c92367efa0d1..07c58114b30c37 100644 --- a/spec/support/shared_contexts/services_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -70,3 +70,9 @@ def enable_license_for_service(service) end end end + +RSpec.shared_context 'integration activation' do + def click_active_checkbox + find('label', text: 'Active').click + end +end diff --git a/spec/support/shared_contexts/project_service_jira_context.rb b/spec/support/shared_contexts/features/integrations/project_integrations_jira_context.rb similarity index 100% rename from spec/support/shared_contexts/project_service_jira_context.rb rename to spec/support/shared_contexts/features/integrations/project_integrations_jira_context.rb diff --git a/spec/support/shared_contexts/project_service_shared_context.rb b/spec/support/shared_contexts/features/integrations/project_integrations_shared_context.rb similarity index 92% rename from spec/support/shared_contexts/project_service_shared_context.rb rename to spec/support/shared_contexts/features/integrations/project_integrations_shared_context.rb index 0e3540a3e156d6..b10844320d06c9 100644 --- a/spec/support/shared_contexts/project_service_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/project_integrations_shared_context.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true RSpec.shared_context 'project service activation' do + include_context 'integration activation' + let(:project) { create(:project) } let(:user) { create(:user) } @@ -21,10 +23,6 @@ def visit_project_integration(name) end end - def click_active_checkbox - find('label', text: 'Active').click - end - def click_save_integration click_button('Save changes') end diff --git a/spec/support/shared_examples/features/integrations/user_activates_mattermost_slash_command_integration_shared_examples.rb b/spec/support/shared_examples/features/integrations/user_activates_mattermost_slash_command_integration_shared_examples.rb new file mode 100644 index 00000000000000..cfa043322db560 --- /dev/null +++ b/spec/support/shared_examples/features/integrations/user_activates_mattermost_slash_command_integration_shared_examples.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'user activates the Mattermost Slash Command integration' do + it 'shows a help message' do + expect(page).to have_content('Use this service to perform common') + end + + it 'shows a token placeholder' do + token_placeholder = find_field('service_token')['placeholder'] + + expect(token_placeholder).to eq('XXxxXXxxXXxxXXxxXXxxXXxx') + end + + it 'redirects to the integrations page after saving but not activating' do + token = ('a'..'z').to_a.join + + fill_in 'service_token', with: token + click_active_checkbox + click_save_integration + + expect(current_path).to eq(edit_path) + expect(page).to have_content('Mattermost slash commands settings saved, but not active.') + end + + it 'redirects to the integrations page after activating' do + token = ('a'..'z').to_a.join + + fill_in 'service_token', with: token + click_save_integration + + expect(current_path).to eq(edit_path) + expect(page).to have_content('Mattermost slash commands settings saved and active.') + end +end -- GitLab