summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-08-23 08:49:35 +0000
committerDouwe Maan <douwe@gitlab.com>2017-08-23 08:49:35 +0000
commit459631acdbbfabe1eadda8feb45d49d4a0c3e412 (patch)
tree41e852bdd5710a8c3ed5369b73f5e112d44a2db9
parent0f74ba967296cfb2e2ae65328f93170f453ab687 (diff)
parente6880ebc7d399627d6b77fb3483b8e7157932313 (diff)
downloadgitlab-ce-459631acdbbfabe1eadda8feb45d49d4a0c3e412.tar.gz
Merge branch 'fix-broken-testing-of-some-integrations' into 'master'
Fix inability to test some project integrations Closes gitlab-ee#3194 See merge request !13729
-rw-r--r--app/controllers/projects/services_controller.rb9
-rw-r--r--changelogs/unreleased/rd-fix-broken-configuration-for-some-integrations.yml5
-rw-r--r--spec/controllers/projects/services_controller_spec.rb42
3 files changed, 40 insertions, 16 deletions
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index d54a1111f11..daa5c88aae0 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -4,7 +4,6 @@ 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
@@ -14,6 +13,8 @@ class Projects::ServicesController < Projects::ApplicationController
end
def update
+ @service.attributes = service_params[:service]
+
if @service.save(context: :manual_change)
redirect_to(project_settings_integrations_path(@project), notice: success_message)
else
@@ -24,7 +25,7 @@ class Projects::ServicesController < Projects::ApplicationController
def test
message = {}
- if @service.can_test?
+ if @service.can_test? && @service.update_attributes(service_params[:service])
data = @service.test_data(project, current_user)
outcome = @service.test(data)
@@ -50,10 +51,6 @@ class Projects::ServicesController < Projects::ApplicationController
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/changelogs/unreleased/rd-fix-broken-configuration-for-some-integrations.yml b/changelogs/unreleased/rd-fix-broken-configuration-for-some-integrations.yml
new file mode 100644
index 00000000000..00528397d64
--- /dev/null
+++ b/changelogs/unreleased/rd-fix-broken-configuration-for-some-integrations.yml
@@ -0,0 +1,5 @@
+---
+title: Testing of some integrations were broken due to missing ServiceHook record.
+merge_request:
+author:
+type: fixed
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index 4e9b0c09ff2..efba9cc7306 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -10,9 +10,6 @@ describe Projects::ServicesController do
before do
sign_in(user)
project.team << [user, :master]
-
- controller.instance_variable_set(:@project, project)
- controller.instance_variable_set(:@service, service)
end
describe '#test' do
@@ -20,7 +17,7 @@ describe Projects::ServicesController do
it 'renders 404' do
allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
- put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+ put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
expect(response).to have_http_status(404)
end
@@ -36,7 +33,7 @@ describe Projects::ServicesController do
it 'returns success' do
allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
- put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id
+ put :test, namespace_id: project.namespace, project_id: project, id: service.to_param
expect(response.status).to eq(200)
end
@@ -45,7 +42,7 @@ describe Projects::ServicesController do
it 'returns success' do
expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
- put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+ put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
expect(response.status).to eq(200)
end
@@ -54,17 +51,42 @@ describe Projects::ServicesController do
it 'returns success' do
expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client)
- put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params
+ put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
expect(response.status).to eq(200)
end
+
+ context 'when service is configured for the first time' do
+ before do
+ allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true)
+ end
+
+ it 'persist the object' do
+ do_put
+
+ expect(BuildkiteService.first).to be_present
+ end
+
+ it 'creates the ServiceHook object' do
+ do_put
+
+ expect(BuildkiteService.first.service_hook).to be_present
+ end
+
+ def do_put
+ put :test, namespace_id: project.namespace,
+ project_id: project,
+ id: 'buildkite',
+ service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' }
+ end
+ end
end
context 'failure' do
it 'returns success 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
+ put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params
expect(response.status).to eq(200)
expect(JSON.parse(response.body))
@@ -77,7 +99,7 @@ describe Projects::ServicesController do
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 }
+ namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true }
expect(response).to redirect_to(project_settings_integrations_path(project))
expect(flash[:notice]).to eq 'HipChat activated.'
@@ -87,7 +109,7 @@ describe Projects::ServicesController do
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, project_id: project, id: service.to_param, service: { active: false }
expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.'
end