Add feature tests for improved JIRA settings

7 files changed, 99 insertions, 19 deletions

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?
+    message = {}
 
-    data = @service.test_data(project, current_user)
-    outcome = @service.test(data)
+    if @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 }
+      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