diff --git a/app/assets/javascripts/integrations/integration_settings_form.js b/app/assets/javascripts/integrations/integration_settings_form.js index 5d555a3776e..5ecab0b11a2 100644 --- a/app/assets/javascripts/integrations/integration_settings_form.js +++ b/app/assets/javascripts/integrations/integration_settings_form.js @@ -93,7 +93,7 @@ export default class IntegrationSettingsForm { }) .done((res) => { if (res.error) { - new Flash(`${res.message}.`, null, null, { + new Flash(`${res.message}`, null, null, { title: 'Save anyway', clickHandler: (e) => { e.preventDefault(); diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 89c4e81a36f..264665942f8 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -22,17 +22,22 @@ class Projects::ServicesController < Projects::ApplicationController end def test - return render json: {}, status: :not_found unless @service.can_test? - - data = @service.test_data(project, current_user) - outcome = @service.test(data) - message = {} - unless outcome[:success] - message = { error: true, message: 'Test failed', service_response: outcome[:result].to_s } + + if @service.can_test? + data = @service.test_data(project, current_user) + outcome = @service.test(data) + + unless outcome[:success] + message = { error: true, message: 'Test failed.', service_response: outcome[:result].to_s } + end + + status = :ok + else + status = :not_found end - render json: message, status: :ok + render json: message, status: status end private diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index da7a4067d8e..6d1a321f651 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -21,10 +21,6 @@ class ChatNotificationService < Service end end - def can_test? - valid? - end - def self.supported_events %w[push issue confidential_issue merge_request note tag_push pipeline wiki_page] diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 5384d75994a..489208a3fd6 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -175,10 +175,6 @@ class JiraService < IssueTrackerService { success: result.present?, result: result } end - def can_test? - username.present? && password.present? - end - # JIRA does not need test data. # We are requesting the project that belongs to the project key. def test_data(user = nil, project = nil) diff --git a/changelogs/unreleased/31511-jira-settings.yml b/changelogs/unreleased/31511-jira-settings.yml new file mode 100644 index 00000000000..3592e92b49e --- /dev/null +++ b/changelogs/unreleased/31511-jira-settings.yml @@ -0,0 +1,4 @@ +--- +title: Simplify test&save actions when setting a service integration +merge_request: 11599 +author: diff --git a/spec/features/projects/services/jira_service_spec.rb b/spec/features/projects/services/jira_service_spec.rb new file mode 100644 index 00000000000..d40c332be6f --- /dev/null +++ b/spec/features/projects/services/jira_service_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +feature 'Setup Jira service', :feature, :js do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:service) { project.create_jira_service } + + let(:url) { 'http://jira.example.com' } + let(:project_url) { 'http://username:password@jira.example.com/rest/api/2/project/GitLabProject' } + + def fill_form(active = true) + check 'Active' if active + + fill_in 'service_url', with: url + fill_in 'service_project_key', with: 'GitLabProject' + fill_in 'service_username', with: 'username' + fill_in 'service_password', with: 'password' + fill_in 'service_jira_issue_transition_id', with: '25' + end + + before do + project.team << [user, :master] + login_as(user) + + visit namespace_project_settings_integrations_path(project.namespace, project) + end + + describe 'user sets and activates Jira Service' do + context 'when Jira connection test succeeds' do + before do + WebMock.stub_request(:get, project_url) + end + + it 'activates the JIRA service' do + click_link('JIRA') + fill_form + click_button('Test settings and save changes') + wait_for_ajax + + expect(page).to have_content('JIRA activated.') + expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project)) + end + end + + context 'when Jira connection test fails' do + before do + WebMock.stub_request(:get, project_url).to_return(status: 401) + end + + it 'activates the JIRA service' do + click_link('JIRA') + fill_form + click_button('Test settings and save changes') + wait_for_ajax + + expect(page).to have_content('Test failed.Save anyway') + + click_on('Save anyway') + wait_for_ajax + + expect(page).to have_content('JIRA activated.') + expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project)) + end + end + end + + describe 'user sets Jira Service but keeps it non active' do + context 'when Jira connection test succeeds' do + it 'activates the JIRA service' do + click_link('JIRA') + fill_form(false) + click_button('Save changes') + + expect(page).to have_content('JIRA settings saved, but not activated.') + expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project)) + end + end + end +end diff --git a/spec/features/projects/services/mattermost_slash_command_spec.rb b/spec/features/projects/services/mattermost_slash_command_spec.rb index bb12f64a552..1fe82222e59 100644 --- a/spec/features/projects/services/mattermost_slash_command_spec.rb +++ b/spec/features/projects/services/mattermost_slash_command_spec.rb @@ -28,7 +28,7 @@ feature 'Setup Mattermost slash commands', :feature, :js do token = ('a'..'z').to_a.join fill_in 'service_token', with: token - click_on 'Save' + click_on 'Save changes' expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project)) expect(page).to have_content('Mattermost slash commands settings saved, but not activated.') @@ -39,10 +39,10 @@ feature 'Setup Mattermost slash commands', :feature, :js do fill_in 'service_token', with: token check 'service_active' - click_on 'Save' + click_on 'Save changes' expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project)) - expect(page).to have_content(' Mattermost slash commands activated.') + expect(page).to have_content('Mattermost slash commands activated.') end it 'shows the add to mattermost button' do