Merge branch '31511-jira-settings' into 'master'

Simplify test&save actions when setting a service integration

Closes #31511

See merge request !11599

31 files changed, 276 insertions, 70 deletions

diff --git a/app/assets/javascripts/flash.js b/app/assets/javascripts/flash.js
index eec30624ff2..ccff8f0ace7 100644
--- a/app/assets/javascripts/flash.js
+++ b/app/assets/javascripts/flash.js
@@ -7,8 +7,21 @@ window.Flash = (function() {
     return $(this).fadeOut();
   };
 
-  function Flash(message, type, parent) {
-    var flash, textDiv;
+  /**
+   * Flash banner supports different types of Flash configurations
+   * along with ability to provide actionConfig which can be used to show
+   * additional action or link on banner next to message
+   *
+   * @param {String} message Flash message
+   * @param {String} type Type of Flash, it can be `notice` or `alert` (default)
+   * @param {Object} parent Reference to Parent element under which Flash needs to appear
+   * @param {Object} actionConfig Map of config to show action on banner
+   *    @param {String} href URL to which action link should point (default '#')
+   *    @param {String} title Title of action
+   *    @param {Function} clickHandler Method to call when action is clicked on
+   */
+  function Flash(message, type, parent, actionConfig) {
+    var flash, textDiv, actionLink;
     if (type == null) {
       type = 'alert';
     }
@@ -30,6 +43,23 @@ window.Flash = (function() {
       text: message
     });
     textDiv.appendTo(flash);
+
+    if (actionConfig) {
+      const actionLinkConfig = {
+        class: 'flash-action',
+        href: actionConfig.href || '#',
+        text: actionConfig.title
+      };
+
+      if (!actionConfig.href) {
+        actionLinkConfig.role = 'button';
+      }
+
+      actionLink = $('<a/>', actionLinkConfig);
+
+      actionLink.appendTo(flash);
+      this.flashContainer.on('click', '.flash-action', actionConfig.clickHandler);
+    }
     if (this.flashContainer.parent().hasClass('content-wrapper')) {
       textDiv.addClass('container-fluid container-limited');
     }
diff --git a/app/assets/javascripts/gl_field_errors.js b/app/assets/javascripts/gl_field_errors.js
index 4f226ff96ea..4bef60264bb 100644
--- a/app/assets/javascripts/gl_field_errors.js
+++ b/app/assets/javascripts/gl_field_errors.js
@@ -31,9 +31,13 @@ class GlFieldErrors {
    * and prevents disabling of invalid submit button by application.js */
 
   catchInvalidFormSubmit (event) {
-    if (!event.currentTarget.checkValidity()) {
-      event.preventDefault();
-      event.stopPropagation();
+    const $form = $(event.currentTarget);
+
+    if (!$form.attr('novalidate')) {
+      if (!event.currentTarget.checkValidity()) {
+        event.preventDefault();
+        event.stopPropagation();
+      }
     }
   }
 
diff --git a/app/assets/javascripts/integrations/index.js b/app/assets/javascripts/integrations/index.js
new file mode 100644
index 00000000000..10fe6bac0e8
--- /dev/null
+++ b/app/assets/javascripts/integrations/index.js
@@ -0,0 +1,7 @@
+/* eslint-disable no-new */
+import IntegrationSettingsForm from './integration_settings_form';
+
+$(() => {
+  const integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form');
+  integrationSettingsForm.init();
+});
diff --git a/app/assets/javascripts/integrations/integration_settings_form.js b/app/assets/javascripts/integrations/integration_settings_form.js
new file mode 100644
index 00000000000..ddd3a6aab99
--- /dev/null
+++ b/app/assets/javascripts/integrations/integration_settings_form.js
@@ -0,0 +1,123 @@
+/* global Flash */
+
+export default class IntegrationSettingsForm {
+  constructor(formSelector) {
+    this.$form = $(formSelector);
+
+    // Form Metadata
+    this.canTestService = this.$form.data('can-test');
+    this.testEndPoint = this.$form.data('test-url');
+
+    // Form Child Elements
+    this.$serviceToggle = this.$form.find('#service_active');
+    this.$submitBtn = this.$form.find('button[type="submit"]');
+    this.$submitBtnLoader = this.$submitBtn.find('.js-btn-spinner');
+    this.$submitBtnLabel = this.$submitBtn.find('.js-btn-label');
+  }
+
+  init() {
+    // Initialize View
+    this.toggleServiceState(this.$serviceToggle.is(':checked'));
+
+    // Bind Event Listeners
+    this.$serviceToggle.on('change', e => this.handleServiceToggle(e));
+    this.$submitBtn.on('click', e => this.handleSettingsSave(e));
+  }
+
+  handleSettingsSave(e) {
+    // Check if Service is marked active, as if not marked active,
+    // We can skip testing it and directly go ahead to allow form to
+    // be submitted
+    if (!this.$serviceToggle.is(':checked')) {
+      return;
+    }
+
+    // Service was marked active so now we check;
+    // 1) If form contents are valid
+    // 2) If this service can be tested
+    // If both conditions are true, we override form submission
+    // and test the service using provided configuration.
+    if (this.$form.get(0).checkValidity() && this.canTestService) {
+      e.preventDefault();
+      this.testSettings(this.$form.serialize());
+    }
+  }
+
+  handleServiceToggle(e) {
+    this.toggleServiceState($(e.currentTarget).is(':checked'));
+  }
+
+  /**
+   * Change Form's validation enforcement based on service status (active/inactive)
+   */
+  toggleServiceState(serviceActive) {
+    this.toggleSubmitBtnLabel(serviceActive);
+    if (serviceActive) {
+      this.$form.removeAttr('novalidate');
+    } else if (!this.$form.attr('novalidate')) {
+      this.$form.attr('novalidate', 'novalidate');
+    }
+  }
+
+  /**
+   * Toggle Submit button label based on Integration status and ability to test service
+   */
+  toggleSubmitBtnLabel(serviceActive) {
+    let btnLabel = 'Save changes';
+
+    if (serviceActive && this.canTestService) {
+      btnLabel = 'Test settings and save changes';
+    }
+
+    this.$submitBtnLabel.text(btnLabel);
+  }
+
+  /**
+   * Toggle Submit button state based on provided boolean value of `saveTestActive`
+   * When enabled, it does two things, and reverts back when disabled
+   *
+   * 1. It shows load spinner on submit button
+   * 2. Makes submit button disabled
+   */
+  toggleSubmitBtnState(saveTestActive) {
+    if (saveTestActive) {
+      this.$submitBtn.disable();
+      this.$submitBtnLoader.removeClass('hidden');
+    } else {
+      this.$submitBtn.enable();
+      this.$submitBtnLoader.addClass('hidden');
+    }
+  }
+
+  /* eslint-disable promise/catch-or-return, no-new */
+  /**
+   * Test Integration config
+   */
+  testSettings(formData) {
+    this.toggleSubmitBtnState(true);
+    $.ajax({
+      type: 'PUT',
+      url: this.testEndPoint,
+      data: formData,
+    })
+    .done((res) => {
+      if (res.error) {
+        new Flash(res.message, null, null, {
+          title: 'Save anyway',
+          clickHandler: (e) => {
+            e.preventDefault();
+            this.$form.submit();
+          },
+        });
+      } else {
+        this.$form.submit();
+      }
+    })
+    .fail(() => {
+      new Flash('Something went wrong on our end.');
+    })
+    .always(() => {
+      this.toggleSubmitBtnState(false);
+    });
+  }
+}
diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss
index 25b4feca3c3..38d884bc7eb 100644
--- a/app/assets/stylesheets/framework/flash.scss
+++ b/app/assets/stylesheets/framework/flash.scss
@@ -16,6 +16,22 @@
     @extend .alert;
     @extend .alert-danger;
     margin: 0;
+
+    .flash-text,
+    .flash-action {
+      display: inline-block;
+    }
+
+    a.flash-action {
+      margin-left: 5px;
+      text-decoration: none;
+      font-weight: normal;
+      border-bottom: 1px solid;
+
+      &:hover {
+        border-color: transparent;
+      }
+    }
   }
 
   .flash-notice,
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index f9d798d0455..704f8cc8a79 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -4,6 +4,7 @@ class Projects::ServicesController < Projects::ApplicationController
   # Authorize
   before_action :authorize_admin_project!
   before_action :service, only: [:edit, :update, :test]
+  before_action :update_service, only: [:update, :test]
 
   respond_to :html
 
@@ -13,36 +14,46 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def update
-    @service.assign_attributes(service_params[:service])
     if @service.save(context: :manual_change)
-      redirect_to(
-        edit_namespace_project_service_path(@project.namespace, @project, @service.to_param),
-        notice: 'Successfully updated.'
-      )
+      redirect_to(namespace_project_settings_integrations_path(@project.namespace, @project), notice: success_message)
     else
       render 'edit'
     end
   end
 
   def test
-    return render_404 unless @service.can_test?
+    message = {}
+
+    if @service.can_test?
+      data = @service.test_data(project, current_user)
+      outcome = @service.test(data)
 
-    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
 
-    if outcome[:success]
-      message = { notice: 'We sent a request to the provided URL' }
+      status = :ok
     else
-      error_message = "We tried to send a request to the provided URL but an error occurred"
-      error_message << ": #{outcome[:result]}" if outcome[:result].present?
-      message = { alert: error_message }
+      status = :not_found
     end
 
-    redirect_back_or_default(options: message)
+    render json: message, status: status
   end
 
   private
 
+  def success_message
+    if @service.active?
+      "#{@service.title} activated."
+    else
+      "#{@service.title} settings saved, but not activated."
+    end
+  end
+
+  def update_service
+    @service.assign_attributes(service_params[:service])
+  end
+
   def service
     @service ||= @project.find_or_initialize_service(params[:id])
   end
diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb
index 3728f5642e4..9ce2d1153a7 100644
--- a/app/models/project_services/asana_service.rb
+++ b/app/models/project_services/asana_service.rb
@@ -34,7 +34,8 @@ http://app.asana.com/-/account_api'
       {
         type: 'text',
         name: 'api_key',
-        placeholder: 'User Personal Access Token. User must have access to task, all comments will be attributed to this user.'
+        placeholder: 'User Personal Access Token. User must have access to task, all comments will be attributed to this user.',
+        required: true
       },
       {
         type: 'text',
diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb
index aeeff8917bf..ae6af732ed4 100644
--- a/app/models/project_services/assembla_service.rb
+++ b/app/models/project_services/assembla_service.rb
@@ -18,7 +18,7 @@ class AssemblaService < Service
 
   def fields
     [
-      { type: 'text', name: 'token', placeholder: '' },
+      { type: 'text', name: 'token', placeholder: '', required: true },
       { type: 'text', name: 'subdomain', placeholder: '' }
     ]
   end
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 3f5b3eb159b..42939ea0ec8 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -47,9 +47,9 @@ class BambooService < CiService
   def fields
     [
         { type: 'text', name: 'bamboo_url',
-          placeholder: 'Bamboo root URL like https://bamboo.example.com' },
+          placeholder: 'Bamboo root URL like https://bamboo.example.com', required: true },
         { type: 'text', name: 'build_key',
-          placeholder: 'Bamboo build plan key like KEY' },
+          placeholder: 'Bamboo build plan key like KEY', required: true },
         { type: 'text', name: 'username',
           placeholder: 'A user with API access, if applicable' },
         { type: 'password', name: 'password' }
diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index 5fb95050b83..fc30f6e3365 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -58,11 +58,11 @@ class BuildkiteService < CiService
     [
       { type: 'text',
         name: 'token',
-        placeholder: 'Buildkite project GitLab token' },
+        placeholder: 'Buildkite project GitLab token', required: true },
 
       { type: 'text',
         name: 'project_url',
-        placeholder: "#{ENDPOINT}/example/project" },
+        placeholder: "#{ENDPOINT}/example/project", required: true },
 
       { type: 'checkbox',
         name: 'enable_ssl_verification',
diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb
index 0de59af5652..c3f5b310619 100644
--- a/app/models/project_services/campfire_service.rb
+++ b/app/models/project_services/campfire_service.rb
@@ -18,7 +18,7 @@ class CampfireService < Service
 
   def fields
     [
-      { type: 'text', name: 'token',     placeholder: '' },
+      { type: 'text', name: 'token',     placeholder: '', required: true },
       { type: 'text', name: 'subdomain', placeholder: '' },
       { type: 'text', name: 'room',      placeholder: '' }
     ]
@@ -76,7 +76,7 @@ class CampfireService < Service
   # Returns a list of rooms, or [].
   # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms
   def rooms(auth)
-    res = self.class.get("/rooms.json", auth) 
+    res = self.class.get("/rooms.json", auth)
     res.code == 200 ? res["rooms"] : []
   end
 
diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb
index 779ef54cfcb..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]
@@ -36,7 +32,7 @@ class ChatNotificationService < Service
 
   def default_fields
     [
-      { type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}" },
+      { type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}", required: true },
       { type: 'text', name: 'username', placeholder: 'e.g. GitLab' },
       { type: 'checkbox', name: 'notify_only_broken_pipelines' },
       { type: 'checkbox', name: 'notify_only_default_branch' }
diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb
index dea915a4d05..b9e3e982b64 100644
--- a/app/models/project_services/custom_issue_tracker_service.rb
+++ b/app/models/project_services/custom_issue_tracker_service.rb
@@ -31,9 +31,9 @@ class CustomIssueTrackerService < IssueTrackerService
     [
       { type: 'text', name: 'title', placeholder: title },
       { type: 'text', name: 'description', placeholder: description },
-      { type: 'text', name: 'project_url', placeholder: 'Project url' },
-      { type: 'text', name: 'issues_url', placeholder: 'Issue url' },
-      { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url' }
+      { type: 'text', name: 'project_url', placeholder: 'Project url', required: true },
+      { type: 'text', name: 'issues_url', placeholder: 'Issue url', required: true },
+      { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url', required: true }
     ]
   end
 end
diff --git a/app/models/project_services/deployment_service.rb b/app/models/project_services/deployment_service.rb
index 91a55514a9a..5b8320158fc 100644
--- a/app/models/project_services/deployment_service.rb
+++ b/app/models/project_services/deployment_service.rb
@@ -30,4 +30,8 @@ class DeploymentService < Service
   def terminals(environment)
     raise NotImplementedError
   end
+
+  def can_test?
+    false
+  end
 end
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 2717c240f05..f6cade9c290 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -93,8 +93,8 @@ class DroneCiService < CiService
 
   def fields
     [
-      { type: 'text', name: 'token', placeholder: 'Drone CI project specific token' },
-      { type: 'text', name: 'drone_url', placeholder: 'http://drone.example.com' },
+      { type: 'text', name: 'token', placeholder: 'Drone CI project specific token', required: true },
+      { type: 'text', name: 'drone_url', placeholder: 'http://drone.example.com', required: true },
       { type: 'checkbox', name: 'enable_ssl_verification', title: "Enable SSL verification" }
     ]
   end
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index b4d7c977ce4..720ad61162e 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -19,7 +19,7 @@ class ExternalWikiService < Service
 
   def fields
     [
-      { type: 'text', name: 'external_wiki_url', placeholder: 'The URL of the external Wiki' }
+      { type: 'text', name: 'external_wiki_url', placeholder: 'The URL of the external Wiki', required: true }
     ]
   end
 
diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb
index 2a05d757eb4..2db95b9aaa3 100644
--- a/app/models/project_services/flowdock_service.rb
+++ b/app/models/project_services/flowdock_service.rb
@@ -18,7 +18,7 @@ class FlowdockService < Service
 
   def fields
     [
-      { type: 'text', name: 'token', placeholder: 'Flowdock Git source token' }
+      { type: 'text', name: 'token', placeholder: 'Flowdock Git source token', required: true }
     ]
   end
 
diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb
index f271e1f1739..017a9b2df6e 100644
--- a/app/models/project_services/gemnasium_service.rb
+++ b/app/models/project_services/gemnasium_service.rb
@@ -18,8 +18,8 @@ class GemnasiumService < Service
 
   def fields
     [
-      { type: 'text', name: 'api_key', placeholder: 'Your personal API KEY on gemnasium.com ' },
-      { type: 'text', name: 'token', placeholder: 'The project\'s slug on gemnasium.com' }
+      { type: 'text', name: 'api_key', placeholder: 'Your personal API KEY on gemnasium.com ', required: true },
+      { type: 'text', name: 'token', placeholder: 'The project\'s slug on gemnasium.com', required: true }
     ]
   end
 
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index c19fed339ba..e3906943ecd 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -33,7 +33,7 @@ class HipchatService < Service
 
   def fields
     [
-      { type: 'text', name: 'token',     placeholder: 'Room token' },
+      { type: 'text', name: 'token',     placeholder: 'Room token', required: true },
       { type: 'text', name: 'room',      placeholder: 'Room name or ID' },
       { type: 'checkbox', name: 'notify' },
       { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) },
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index a51d43adcb9..19357f90810 100644
--- a/app/models/project_services/irker_service.rb
+++ b/app/models/project_services/irker_service.rb
@@ -49,7 +49,7 @@ class IrkerService < Service
         help: 'A default IRC URI to prepend before each recipient (optional)',
         placeholder: 'irc://irc.network.net:6697/' },
       { type: 'textarea', name: 'recipients',
-        placeholder: 'Recipients/channels separated by whitespaces',
+        placeholder: 'Recipients/channels separated by whitespaces', required: true,
         help: 'Recipients have to be specified with a full URI: '\
         'irc[s]://irc.network.net[:port]/#channel. Special cases: if '\
         'you want the channel to be a nickname instead, append ",isnick" to ' \
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index eddf308eae3..ff138b9066d 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -32,9 +32,9 @@ class IssueTrackerService < Service
   def fields
     [
       { type: 'text', name: 'description', placeholder: description },
-      { type: 'text', name: 'project_url', placeholder: 'Project url' },
-      { type: 'text', name: 'issues_url', placeholder: 'Issue url' },
-      { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url' }
+      { type: 'text', name: 'project_url', placeholder: 'Project url', required: true },
+      { type: 'text', name: 'issues_url', placeholder: 'Issue url', required: true },
+      { type: 'text', name: 'new_issue_url', placeholder: 'New Issue url', required: true }
     ]
   end
 
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 25d098b63c0..2450fb43212 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -86,11 +86,11 @@ class JiraService < IssueTrackerService
 
   def fields
     [
-      { type: 'text', name: 'url', title: 'Web URL', placeholder: 'https://jira.example.com' },
+      { type: 'text', name: 'url', title: 'Web URL', placeholder: 'https://jira.example.com', required: true },
       { type: 'text', name: 'api_url', title: 'JIRA API URL', placeholder: 'If different from Web URL' },
-      { type: 'text', name: 'project_key', placeholder: 'Project Key' },
-      { type: 'text', name: 'username', placeholder: '' },
-      { type: 'password', name: 'password', placeholder: '' },
+      { type: 'text', name: 'project_key', placeholder: 'Project Key', required: true },
+      { type: 'text', name: 'username', placeholder: '', required: true },
+      { type: 'password', name: 'password', placeholder: '', required: true },
       { type: 'text', name: 'jira_issue_transition_id', placeholder: '' }
     ]
   end
@@ -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/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index 546b6e0a498..72ddf9a4be3 100644
--- a/app/models/project_services/mock_ci_service.rb
+++ b/app/models/project_services/mock_ci_service.rb
@@ -21,7 +21,8 @@ class MockCiService < CiService
     [
       { type: 'text',
         name: 'mock_service_url',
-        placeholder: 'http://localhost:4004' }
+        placeholder: 'http://localhost:4004',
+        required: true }
     ]
   end
 
@@ -79,4 +80,8 @@ class MockCiService < CiService
       :error
     end
   end
+
+  def can_test?
+    false
+  end
 end
diff --git a/app/models/project_services/mock_monitoring_service.rb b/app/models/project_services/mock_monitoring_service.rb
index dd04e04e198..ed0318c6b27 100644
--- a/app/models/project_services/mock_monitoring_service.rb
+++ b/app/models/project_services/mock_monitoring_service.rb
@@ -14,4 +14,8 @@ class MockMonitoringService < MonitoringService
   def metrics(environment)
     JSON.parse(File.read(Rails.root + 'spec/fixtures/metrics.json'))
   end
+
+  def can_test?
+    false
+  end
 end
diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb
index f824171ad09..9d37184be2c 100644
--- a/app/models/project_services/pipelines_email_service.rb
+++ b/app/models/project_services/pipelines_email_service.rb
@@ -53,7 +53,8 @@ class PipelinesEmailService < Service
     [
       { type: 'textarea',
         name: 'recipients',
-        placeholder: 'Emails separated by comma' },
+        placeholder: 'Emails separated by comma',
+        required: true },
       { type: 'checkbox',
         name: 'notify_only_broken_pipelines' }
     ]
diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb
index d86f4f6f448..f9dfa2e91c3 100644
--- a/app/models/project_services/pivotaltracker_service.rb
+++ b/app/models/project_services/pivotaltracker_service.rb
@@ -23,7 +23,8 @@ class PivotaltrackerService < Service
       {
         type: 'text',
         name: 'token',
-        placeholder: 'Pivotal Tracker API token.'
+        placeholder: 'Pivotal Tracker API token.',
+        required: true
       },
       {
         type: 'text',
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index ec72cb6856d..110b8bc209b 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -49,7 +49,8 @@ class PrometheusService < MonitoringService
         type: 'text',
         name: 'api_url',
         title: 'API URL',
-        placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/'
+        placeholder: 'Prometheus API Base URL, like http://prometheus.example.com/',
+        required: true
       }
     ]
   end
diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb
index fc29a5277bb..aa7bd4c3c84 100644
--- a/app/models/project_services/pushover_service.rb
+++ b/app/models/project_services/pushover_service.rb
@@ -19,10 +19,10 @@ class PushoverService < Service
 
   def fields
     [
-      { type: 'text', name: 'api_key', placeholder: 'Your application key' },
-      { type: 'text', name: 'user_key', placeholder: 'Your user key' },
+      { type: 'text', name: 'api_key', placeholder: 'Your application key', required: true },
+      { type: 'text', name: 'user_key', placeholder: 'Your user key', required: true },
       { type: 'text', name: 'device', placeholder: 'Leave blank for all active devices' },
-      { type: 'select', name: 'priority', choices:
+      { type: 'select', name: 'priority', required: true, choices:
         [
           ['Lowest Priority', -2],
           ['Low Priority', -1],
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index b16beb406b9..cbe137452bd 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -50,9 +50,9 @@ class TeamcityService < CiService
   def fields
     [
       { type: 'text', name: 'teamcity_url',
-        placeholder: 'TeamCity root URL like https://teamcity.example.com' },
+        placeholder: 'TeamCity root URL like https://teamcity.example.com', required: true },
       { type: 'text', name: 'build_type',
-        placeholder: 'Build configuration ID' },
+        placeholder: 'Build configuration ID', required: true },
       { type: 'text', name: 'username',
         placeholder: 'A user with permissions to trigger a manual build' },
       { type: 'password', name: 'password' }
diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml
index f1a80f1d5e1..9167789a69d 100644
--- a/app/views/projects/services/_form.html.haml
+++ b/app/views/projects/services/_form.html.haml
@@ -1,3 +1,6 @@
+- content_for :page_specific_javascripts do
+  = webpack_bundle_tag('integrations')
+
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
     %h4.prepend-top-0
@@ -6,15 +9,17 @@
 
     %p= @service.description
   .col-lg-9
-    = form_for(@service, as: :service, url: namespace_project_service_path(@project.namespace, @project, @service.to_param), method: :put, html: { class: 'form-horizontal' }) do |form|
+    = form_for(@service, as: :service, url: namespace_project_service_path(@project.namespace, @project, @service.to_param), method: :put, html: { class: 'gl-show-field-errors form-horizontal js-integration-settings-form', data: { 'can-test' => @service.can_test?, 'test-url' => test_namespace_project_service_path } }) do |form|
       = render 'shared/service_settings', form: form, subject: @service
       .footer-block.row-content-block
-        = form.submit 'Save changes', class: 'btn btn-save'
+        %button.btn.btn-save{ type: 'submit' }
+          = icon('spinner spin', class: 'hidden js-btn-spinner')
+          %span.js-btn-label
+            Save changes
         &nbsp;
         - if @service.valid? && @service.activated?
           - unless @service.can_test?
             - disabled_class = 'disabled'
             - disabled_title = @service.disabled_title
 
-          = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled_class}", title: disabled_title
-        = link_to "Cancel", namespace_project_settings_integrations_path(@project.namespace, @project), class: "btn btn-cancel"
+        = link_to 'Cancel', namespace_project_settings_integrations_path(@project.namespace, @project), class: 'btn btn-cancel'
diff --git a/app/views/shared/_field.html.haml b/app/views/shared/_field.html.haml
index d74b0043949..795447a9ca6 100644
--- a/app/views/shared/_field.html.haml
+++ b/app/views/shared/_field.html.haml
@@ -3,6 +3,7 @@
 - value = @service.send(name)
 - type = field[:type]
 - placeholder = field[:placeholder]
+- required = field[:required]
 - choices = field[:choices]
 - default_choice = field[:default_choice]
 - help = field[:help]
@@ -14,14 +15,14 @@
     = form.label name, title, class: "control-label"
   .col-sm-10
     - if type == 'text'
-      = form.text_field name, class: "form-control", placeholder: placeholder
+      = form.text_field name, class: "form-control", placeholder: placeholder, required: required
     - elsif type == 'textarea'
-      = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder
+      = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required
     - elsif type == 'checkbox'
       = form.check_box name
     - elsif type == 'select'
       = form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control" }
     - elsif type == 'password'
-      = form.password_field name, autocomplete: "new-password", class: "form-control"
+      = form.password_field name, autocomplete: "new-password", class: "form-control", required: value.blank? && :required
     - if help
       %span.help-block= help