Merge branch 'fix-slack-channel-in-admin-settings' into 'master'

Fix bug where Slack service channel was not saved in admin template settings

Consolidate allowed parameters in one place to avoid these kinds of bugs in the future.

Closes https://github.com/gitlabhq/gitlabhq/issues/9181
Closes #1501

See merge request !574

5 files changed, 26 insertions, 22 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 58dde541429..92ae6767ea2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,7 +17,7 @@ v 7.11.0 (unreleased)
   - Include commit comments in MR from a forked project.
   -
   -
-  -
+  - Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu)
   - Move snippets UI to fluid layout
   - Improve UI for sidebar. Increase separation between navigation and content
   - Improve new project command options (Ben Bodenmiller)
diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index c1fdcd7fab6..a62170662e1 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -40,15 +40,6 @@ class Admin::ServicesController < Admin::ApplicationController
 
   def application_services_params
     params.permit(:id,
-      service: [
-        :title, :token, :type, :active, :api_key, :subdomain,
-        :room, :recipients, :project_url, :webhook,
-        :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
-        :build_key, :server, :teamcity_url, :build_type,
-        :description, :issues_url, :new_issue_url, :restrict_to_branch,
-        :send_from_committer_email, :disable_diffs,
-        :push_events, :tag_push_events, :note_events, :issues_events,
-        :merge_requests_events
-    ])
+      service: Projects::ServicesController::ALLOWED_PARAMS)
   end
 end
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index ae8146dca59..73031851734 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -1,4 +1,12 @@
 class Projects::ServicesController < Projects::ApplicationController
+  ALLOWED_PARAMS = [:title, :token, :type, :active, :api_key, :subdomain,
+                    :room, :recipients, :project_url, :webhook,
+                    :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
+                    :build_key, :server, :teamcity_url, :build_type,
+                    :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
+                    :colorize_messages, :channels,
+                    :push_events, :issues_events, :merge_requests_events, :tag_push_events,
+                    :note_events, :send_from_committer_email, :disable_diffs, :external_wiki_url]
   # Authorize
   before_action :authorize_admin_project!
   before_action :service, only: [:edit, :update, :test]
@@ -45,15 +53,6 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def service_params
-    params.require(:service).permit(
-      :title, :token, :type, :active, :api_key, :subdomain,
-      :room, :recipients, :project_url, :webhook,
-      :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
-      :build_key, :server, :teamcity_url, :build_type,
-      :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
-      :colorize_messages, :channels,
-      :push_events, :issues_events, :merge_requests_events, :tag_push_events,
-      :note_events, :send_from_committer_email, :disable_diffs, :external_wiki_url
-    )
+    params.require(:service).permit(ALLOWED_PARAMS)
   end
 end
diff --git a/features/admin/settings.feature b/features/admin/settings.feature
index 52e47307b23..e38eea2cfed 100644
--- a/features/admin/settings.feature
+++ b/features/admin/settings.feature
@@ -11,6 +11,9 @@ Feature: Admin Settings
   Scenario: Change Slack Service Template settings
     When I click on "Service Templates"
     And I click on "Slack" service
+    And I fill out Slack settings
     Then I check all events and submit form
     And I should see service template settings saved
+    Then I click on "Slack" service
     And I should see all checkboxes checked
+    And I should see Slack settings saved
diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb
index 87d4e969ff5..15ca0d80f1a 100644
--- a/features/steps/admin/settings.rb
+++ b/features/steps/admin/settings.rb
@@ -31,10 +31,15 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
     page.check('Comments')
     page.check('Issues events')
     page.check('Merge Request events')
-    fill_in 'Webhook', with: "http://localhost"
     click_on 'Save'
   end
 
+  step 'I fill out Slack settings' do
+    fill_in 'Webhook', with: 'http://localhost'
+    fill_in 'Username', with: 'test_user'
+    fill_in 'Channel', with: '#test_channel'
+  end
+
   step 'I should see service template settings saved' do
     page.should have_content 'Application settings saved successfully'
   end
@@ -44,4 +49,10 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
       checkbox.should be_checked
     end
   end
+
+  step 'I should see Slack settings saved' do
+    find_field('Webhook').value.should eq 'http://localhost'
+    find_field('Username').value.should eq 'test_user'
+    find_field('Channel').value.should eq '#test_channel'
+  end
 end