simplify test&save actions when setting a service integration

30 files changed, 187 insertions, 174 deletions

diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index f9d798d0455..89c4e81a36f 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 :build_service, only: [:update, :test]
 
   respond_to :html
 
@@ -13,36 +14,41 @@ 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?
+    return render json: {}, status: :not_found unless @service.can_test?
 
     data = @service.test_data(project, current_user)
     outcome = @service.test(data)
 
-    if outcome[:success]
-      message = { notice: 'We sent a request to the provided URL' }
-    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 }
+    message = {}
+    unless outcome[:success]
+      message = { error: true, message: 'Test failed', service_response: outcome[:result].to_s }
     end
 
-    redirect_back_or_default(options: message)
+    render json: message, status: :ok
   end
 
   private
 
+  def success_message
+    if @service.active?
+      "#{@service.title} activated."
+    else
+      "#{@service.title} settings saved, but not activated."
+    end
+  end
+
+  def build_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..da7a4067d8e 100644
--- a/app/models/project_services/chat_notification_service.rb
+++ b/app/models/project_services/chat_notification_service.rb
@@ -36,7 +36,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/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..5384d75994a 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -86,12 +86,12 @@ 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: 'jira_issue_transition_id', 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: '', required: true }
     ]
   end
 
diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index 546b6e0a498..e5109dbbc93 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
 
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/shared/_field.html.haml b/app/views/shared/_field.html.haml
index d74b0043949..d67e20e988d 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]
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 5aac44fce10..14718e2f3c4 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -67,7 +67,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       resources :services, constraints: { id: /[^\/]+/ }, only: [:index, :edit, :update] do
         member do
-          get :test
+          put :test
         end
       end
 
diff --git a/features/project/service.feature b/features/project/service.feature
index cce5f58adec..54f07ebca92 100644
--- a/features/project/service.feature
+++ b/features/project/service.feature
@@ -11,77 +11,77 @@ Feature: Project Services
     When I visit project "Shop" services page
     And I click hipchat service link
     And I fill hipchat settings
-    Then I should see hipchat service settings saved
+    Then I should see the Hipchat success message
 
   Scenario: Activate hipchat service with custom server
     When I visit project "Shop" services page
     And I click hipchat service link
     And I fill hipchat settings with custom server
-    Then I should see hipchat service settings with custom server saved
+    Then I should see the Hipchat success message
 
   Scenario: Activate pivotaltracker service
     When I visit project "Shop" services page
     And I click pivotaltracker service link
     And I fill pivotaltracker settings
-    Then I should see pivotaltracker service settings saved
+    Then I should see the Pivotaltracker success message
 
   Scenario: Activate Flowdock service
     When I visit project "Shop" services page
     And I click Flowdock service link
     And I fill Flowdock settings
-    Then I should see Flowdock service settings saved
+    Then I should see the Flowdock success message
 
   Scenario: Activate Assembla service
     When I visit project "Shop" services page
     And I click Assembla service link
     And I fill Assembla settings
-    Then I should see Assembla service settings saved
+    Then I should see the Assembla success message
 
   Scenario: Activate Slack notifications service
     When I visit project "Shop" services page
     And I click Slack notifications service link
     And I fill Slack notifications settings
-    Then I should see Slack Notifications service settings saved
+    Then I should see the Slack notifications success message
 
   Scenario: Activate Pushover service
     When I visit project "Shop" services page
     And I click Pushover service link
     And I fill Pushover settings
-    Then I should see Pushover service settings saved
+    Then I should see the Pushover success message
 
   Scenario: Activate email on push service
     When I visit project "Shop" services page
     And I click email on push service link
     And I fill email on push settings
-    Then I should see email on push service settings saved
+    Then I should see the Emails on push success message
 
   Scenario: Activate JIRA service
     When I visit project "Shop" services page
     And I click jira service link
     And I fill jira settings
-    Then I should see jira service settings saved
+    Then I should see the JIRA success message
 
   Scenario: Activate Irker (IRC Gateway) service
     When I visit project "Shop" services page
     And I click Irker service link
     And I fill Irker settings
-    Then I should see Irker service settings saved
+    Then I should see the Irker success message
 
   Scenario: Activate Atlassian Bamboo CI service
     When I visit project "Shop" services page
     And I click Atlassian Bamboo CI service link
     And I fill Atlassian Bamboo CI settings
-    Then I should see Atlassian Bamboo CI service settings saved
+    Then I should see the Bamboo success message
     And I should see empty field Change Password
 
   Scenario: Activate jetBrains TeamCity CI service
     When I visit project "Shop" services page
     And I click jetBrains TeamCity CI service link
     And I fill jetBrains TeamCity CI settings
-    Then I should see jetBrains TeamCity CI service settings saved
+    Then I should see the JetBrains success message
 
   Scenario: Activate Asana service
     When I visit project "Shop" services page
     And I click Asana service link
     And I fill Asana settings
-    Then I should see Asana service settings saved
+    Then I should see the Asana success message
diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb
index 66368a159ec..6bac4df16f8 100644
--- a/features/steps/project/services.rb
+++ b/features/steps/project/services.rb
@@ -34,8 +34,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see hipchat service settings saved' do
-    expect(find_field('Room').value).to eq 'gitlab'
+  step 'I should see the Hipchat success message' do
+    expect(page).to have_content 'HipChat activated.'
   end
 
   step 'I fill hipchat settings with custom server' do
@@ -46,10 +46,6 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see hipchat service settings with custom server saved' do
-    expect(find_field('Server').value).to eq 'https://chat.example.com'
-  end
-
   step 'I click pivotaltracker service link' do
     click_link 'PivotalTracker'
   end
@@ -60,8 +56,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see pivotaltracker service settings saved' do
-    expect(find_field('Token').value).to eq 'verySecret'
+  step 'I should see the Pivotaltracker success message' do
+    expect(page).to have_content 'PivotalTracker activated.'
   end
 
   step 'I click Flowdock service link' do
@@ -74,8 +70,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Flowdock service settings saved' do
-    expect(find_field('Token').value).to eq 'verySecret'
+  step 'I should see the Flowdock success message' do
+    expect(page).to have_content 'Flowdock activated.'
   end
 
   step 'I click Assembla service link' do
@@ -88,8 +84,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Assembla service settings saved' do
-    expect(find_field('Token').value).to eq 'verySecret'
+  step 'I should see the Assembla success message' do
+    expect(page).to have_content 'Assembla activated.'
   end
 
   step 'I click Asana service link' do
@@ -103,9 +99,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Asana service settings saved' do
-    expect(find_field('Api key').value).to eq 'verySecret'
-    expect(find_field('Restrict to branch').value).to eq 'master'
+  step 'I should see the Asana success message' do
+    expect(page).to have_content 'Asana activated.'
   end
 
   step 'I click email on push service link' do
@@ -113,12 +108,13 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
   end
 
   step 'I fill email on push settings' do
+    check 'Active'
     fill_in 'Recipients', with: 'qa@company.name'
     click_button 'Save'
   end
 
-  step 'I should see email on push service settings saved' do
-    expect(find_field('Recipients').value).to eq 'qa@company.name'
+  step 'I should see the Emails on push success message' do
+    expect(page).to have_content 'Emails on push activated.'
   end
 
   step 'I click Irker service link' do
@@ -132,9 +128,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Irker service settings saved' do
-    expect(find_field('Recipients').value).to eq 'irc://chat.freenode.net/#commits'
-    expect(find_field('Colorize messages').value).to eq '1'
+  step 'I should see the Irker success message' do
+    expect(page).to have_content 'Irker (IRC gateway) activated.'
   end
 
   step 'I click Slack notifications service link' do
@@ -147,8 +142,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Slack Notifications service settings saved' do
-    expect(find_field('Webhook').value).to eq 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685'
+  step 'I should see the Slack notifications success message' do
+    expect(page).to have_content 'Slack notifications activated.'
   end
 
   step 'I click Pushover service link' do
@@ -165,12 +160,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Pushover service settings saved' do
-    expect(find_field('Api key').value).to eq 'verySecret'
-    expect(find_field('User key').value).to eq 'verySecret'
-    expect(find_field('Device').value).to eq 'myDevice'
-    expect(find_field('Priority').find('option[selected]').value).to eq '1'
-    expect(find_field('Sound').find('option[selected]').value).to eq 'bike'
+  step 'I should see the Pushover success message' do
+    expect(page).to have_content 'Pushover activated.'
   end
 
   step 'I click jira service link' do
@@ -178,6 +169,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
   end
 
   step 'I fill jira settings' do
+    check 'Active'
+
     fill_in 'Web URL', with: 'http://jira.example'
     fill_in 'JIRA API URL', with: 'http://jira.example/api'
     fill_in 'Username', with: 'gitlab'
@@ -186,11 +179,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see jira service settings saved' do
-    expect(find_field('Web URL').value).to eq 'http://jira.example'
-    expect(find_field('JIRA API URL').value).to eq 'http://jira.example/api'
-    expect(find_field('Username').value).to eq 'gitlab'
-    expect(find_field('Project Key').value).to eq 'GITLAB'
+  step 'I should see the JIRA success message' do
+    expect(page).to have_content 'JIRA activated.'
   end
 
   step 'I click Atlassian Bamboo CI service link' do
@@ -206,13 +196,13 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see Atlassian Bamboo CI service settings saved' do
-    expect(find_field('Bamboo url').value).to eq 'http://bamboo.example.com'
-    expect(find_field('Build key').value).to eq 'KEY'
-    expect(find_field('Username').value).to eq 'user'
+  step 'I should see the Bamboo success message' do
+    expect(page).to have_content 'Atlassian Bamboo CI activated.'
   end
 
   step 'I should see empty field Change Password' do
+    click_link 'Atlassian Bamboo CI'
+
     expect(find_field('Enter new password').value).to be_nil
   end
 
@@ -229,9 +219,7 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps
     click_button 'Save'
   end
 
-  step 'I should see JetBrains TeamCity CI service settings saved' do
-    expect(find_field('Teamcity url').value).to eq 'http://teamcity.example.com'
-    expect(find_field('Build type').value).to eq 'GitlabTest_Build'
-    expect(find_field('Username').value).to eq 'user'
+  step 'I should see the JetBrains success message' do
+    expect(page).to have_content 'JetBrains TeamCity CI activated.'
   end
 end
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index 2d892f4a2b7..7aa2492edf6 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -3,7 +3,9 @@ require 'spec_helper'
 describe Projects::ServicesController do
   let(:project) { create(:project, :repository) }
   let(:user)    { create(:user) }
-  let(:service) { create(:service, project: project) }
+  let(:service) { create(:hipchat_service, project: project) }
+  let(:hipchat_client) { { '#room' => double(send: true) } }
+  let(:service_params) { { token: 'hipchat_token_p', room: '#room' } }
 
   before do
     sign_in(user)
@@ -13,97 +15,84 @@ describe Projects::ServicesController do
     controller.instance_variable_set(:@service, service)
   end
 
-  shared_examples_for 'services controller' do |referrer|
-    before do
-      request.env["HTTP_REFERER"] = referrer
-    end
-
-    describe "#test" do
-      context 'when can_test? returns false' do
-        it 'renders 404' do
-          allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
+  describe '#test' do
+    context 'when can_test? returns false' do
+      it 'renders 404' do
+        allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
 
-          get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
 
-          expect(response).to have_http_status(404)
-        end
+        expect(response).to have_http_status(404)
       end
+    end
 
-      context 'success' do
-        context 'with empty project' do
-          let(:project) { create(:empty_project) }
-
-          context 'with chat notification service' do
-            let(:service) { project.create_microsoft_teams_service(webhook: 'http://webhook.com') }
-
-            it 'redirects and show success message' do
-              allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
-
-              get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+    context 'success' do
+      context 'with empty project' do
+        let(:project) { create(:empty_project) }
 
-              expect(response).to redirect_to(root_path)
-              expect(flash[:notice]).to eq('We sent a request to the provided URL')
-            end
-          end
+        context 'with chat notification service' do
+          let(:service) { project.create_microsoft_teams_service(webhook: 'http://webhook.com') }
 
-          it 'redirects and show success message' do
-            expect(service).to receive(:test).and_return(success: true, result: 'done')
+          it 'returns success' do
+            allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
 
-            get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+            put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
 
-            expect(response).to redirect_to(root_path)
-            expect(flash[:notice]).to eq('We sent a request to the provided URL')
+            expect(response.status).to eq(200)
           end
         end
 
-        it "redirects and show success message" do
-          expect(service).to receive(:test).and_return(success: true, result: 'done')
+        it 'returns success' do
+          expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-          get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+          put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
 
-          expect(response).to redirect_to(root_path)
-          expect(flash[:notice]).to eq('We sent a request to the provided URL')
+          expect(response.status).to eq(200)
         end
       end
 
-      context 'failure' do
-        it "redirects and show failure message" do
-          expect(service).to receive(:test).and_return(success: false, result: 'Bad test')
+      it 'returns success' do
+        expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
 
-          get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
 
-          expect(response).to redirect_to(root_path)
-          expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test')
-        end
+        expect(response.status).to eq(200)
       end
-    end
-  end
 
-  describe 'referrer defined' do
-    it_should_behave_like 'services controller' do
-      let!(:referrer) { "/" }
+      def built_service
+      end
     end
-  end
 
-  describe 'referrer undefined' do
-    it_should_behave_like 'services controller' do
-      let!(:referrer) { nil }
+    context 'failure' do
+      it 'returns 500 status code and the error message' do
+        expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_raise('Bad test')
+
+        put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+
+        expect(response.status).to eq(200)
+        expect(JSON.parse(response.body)).
+          to eq('error' => true, 'message' => 'Test failed', 'service_response' => 'Bad test')
+      end
     end
   end
 
   describe 'PUT #update' do
-    context 'on successful update' do
-      it 'sets the flash' do
-        expect(service).to receive(:to_param).and_return('hipchat')
-        expect(service).to receive(:event_names).and_return(HipchatService.event_names)
+    context 'when param `active` is set to true' do
+      it 'activates the service and redirects to integrations paths' do
+        put :update,
+          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: true }
+
+        expect(response).to redirect_to(namespace_project_settings_integrations_path(project.namespace, project))
+        expect(flash[:notice]).to eq 'HipChat activated.'
+      end
+    end
 
+    context 'when param `active` is set to false' do
+      it 'does not  activate the service but saves the settings' do
         put :update,
-          namespace_id: project.namespace.id,
-          project_id: project.id,
-          id: service.id,
-          service: { active: false }
+          namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: false }
 
-        expect(flash[:notice]).to eq 'Successfully updated.'
+        expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
       end
     end
   end
diff --git a/spec/factories/services.rb b/spec/factories/services.rb
index 3fad4d2d658..e7366a7fd1c 100644
--- a/spec/factories/services.rb
+++ b/spec/factories/services.rb
@@ -33,4 +33,10 @@ FactoryGirl.define do
       project_key: 'jira-key'
     )
   end
+
+  factory :hipchat_service do
+    project factory: :empty_project
+    type 'HipchatService'
+    token 'test_token'
+  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 dc3854262e7..bb12f64a552 100644
--- a/spec/features/projects/services/mattermost_slash_command_spec.rb
+++ b/spec/features/projects/services/mattermost_slash_command_spec.rb
@@ -24,15 +24,25 @@ feature 'Setup Mattermost slash commands', :feature, :js do
       expect(token_placeholder).to eq('XXxxXXxxXXxxXXxxXXxxXXxx')
     end
 
-    it 'shows the token after saving' do
+    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_on 'Save'
 
-      value = find_field('service_token').value
+      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.')
+    end
+
+    it 'redirects to the integrations page after activating' do
+      token = ('a'..'z').to_a.join
+
+      fill_in 'service_token', with: token
+      check 'service_active'
+      click_on 'Save'
 
-      expect(value).to eq(token)
+      expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project))
+      expect(page).to have_content(' Mattermost slash commands activated.')
     end
 
     it 'shows the add to mattermost button' do
diff --git a/spec/features/projects/services/slack_slash_command_spec.rb b/spec/features/projects/services/slack_slash_command_spec.rb
index db903a0c8f0..f53b820c460 100644
--- a/spec/features/projects/services/slack_slash_command_spec.rb
+++ b/spec/features/projects/services/slack_slash_command_spec.rb
@@ -21,13 +21,21 @@ feature 'Slack slash commands', feature: true do
     expect(page).to have_content('This service allows users to perform common')
   end
 
-  it 'shows the token after saving' do
+  it 'redirects to the integrations page after saving but not activating' do
     fill_in 'service_token', with: 'token'
     click_on 'Save'
 
-    value = find_field('service_token').value
+    expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project))
+    expect(page).to have_content('Slack slash commands settings saved, but not activated.')
+  end
+
+  it 'redirects to the integrations page after activating' do
+    fill_in 'service_token', with: 'token'
+    check 'service_active'
+    click_on 'Save'
 
-    expect(value).to eq('token')
+    expect(current_path).to eq(namespace_project_settings_integrations_path(project.namespace, project))
+    expect(page).to have_content('Slack slash commands activated.')
   end
 
   it 'shows the correct trigger url' do