Automatic merge of gitlab-org/gitlab-ce master

app/controllers/projects/services_controller.rb

@@ -18,10 +18,23 @@ class Projects::ServicesController < Projects::ApplicationController
   def update
     @service.attributes = service_params[:service]
 
-    if @service.save(context: :manual_change)
-      redirect_to(project_settings_integrations_path(@project), notice: success_message)
-    else
-      render 'edit'
+    saved = @service.save(context: :manual_change)
+
+    respond_to do |format|
+      format.html do
+        if saved
+          redirect_to project_settings_integrations_path(@project),
+            notice: success_message
+        else
+          render 'edit'
+        end
+      end
+
+      format.json do
+        status = saved ? :ok : :unprocessable_entity
+
+        render json: serialize_as_json, status: status
+      end
     end
   end
 
@@ -67,4 +80,10 @@ class Projects::ServicesController < Projects::ApplicationController
   def ensure_service_enabled
     render_404 unless service
   end
+
+  def serialize_as_json
+    @service
+      .as_json(only: @service.json_fields)
+      .merge(errors: @service.errors.as_json)
+  end
 end

app/models/service.rb

@@ -107,6 +107,13 @@ class Service < ApplicationRecord
     []
   end
 
+  # Expose a list of fields in the JSON endpoint.
+  #
+  # This list is used in `Service#as_json(only: json_fields)`.
+  def json_fields
+    %w(active)
+  end
+
   def test_data(project, user)
     Gitlab::DataBuilder::Push.build_sample(project, user)
   end

changelogs/unreleased/pl-project-service-json.yml

+---
+title: Expose update project service endpoint JSON
+merge_request: 32759
+author:
+type: added

spec/controllers/projects/services_controller_spec.rb

@@ -19,9 +19,9 @@ describe Projects::ServicesController do
       it 'renders 404' do
         allow_any_instance_of(Service).to receive(:can_test?).and_return(false)
 
-        put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param }
+        put :test, params: project_params
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
@@ -29,11 +29,11 @@ describe Projects::ServicesController do
       let(:service_params) { { active: 'true', url: '' } }
 
       it 'returns error messages in JSON response' do
-        put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
+        put :test, params: project_params(service: service_params)
 
-        expect(json_response['message']).to eq "Validations failed."
+        expect(json_response['message']).to eq 'Validations failed.'
         expect(json_response['service_response']).to include "Url can't be blank"
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to be_successful
       end
     end
 
@@ -47,9 +47,9 @@ describe Projects::ServicesController do
           it 'returns success' do
             allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true)
 
-            put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param }
+            put :test, params: project_params
 
-            expect(response.status).to eq(200)
+            expect(response).to be_successful
           end
         end
 
@@ -57,11 +57,11 @@ describe Projects::ServicesController do
           stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
             .to_return(status: 200, body: '{}')
 
-          expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+          expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
-          put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
+          put :test, params: project_params(service: service_params)
 
-          expect(response.status).to eq(200)
+          expect(response).to be_successful
         end
       end
 
@@ -69,14 +69,23 @@ describe Projects::ServicesController do
         stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
           .to_return(status: 200, body: '{}')
 
-        expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+        expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
-        put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
+        put :test, params: project_params(service: service_params)
 
-        expect(response.status).to eq(200)
+        expect(response).to be_successful
       end
 
       context 'when service is configured for the first time' do
+        let(:service_params) do
+          {
+            'active' => '1',
+            'push_events' => '1',
+            'token' => 'token',
+            'project_url' => 'http://test.com'
+          }
+        end
+
         before do
           allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true)
         end
@@ -84,7 +93,7 @@ describe Projects::ServicesController do
         it 'persist the object' do
           do_put
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to be_successful
           expect(json_response).to be_empty
           expect(BuildkiteService.first).to be_present
         end
@@ -92,18 +101,14 @@ describe Projects::ServicesController do
         it 'creates the ServiceHook object' do
           do_put
 
-          expect(response).to have_gitlab_http_status(200)
+          expect(response).to be_successful
           expect(json_response).to be_empty
           expect(BuildkiteService.first.service_hook).to be_present
         end
 
         def do_put
-          put :test, params: {
-                       namespace_id: project.namespace,
-                       project_id: project,
-                       id: 'buildkite',
-                       service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' }
-                     }
+          put :test, params: project_params(id: 'buildkite',
+                                            service: service_params)
         end
       end
     end
@@ -113,9 +118,9 @@ describe Projects::ServicesController do
         stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
           .to_return(status: 404)
 
-        put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
+        put :test, params: project_params(service: service_params)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to be_successful
         expect(json_response).to eq(
           'error' => true,
           'message' => 'Test failed.',
@@ -127,39 +132,70 @@ describe Projects::ServicesController do
   end
 
   describe 'PUT #update' do
-    context 'when param `active` is set to true' do
-      it 'activates the service and redirects to integrations paths' do
-        put :update,
-          params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true } }
+    describe 'as HTML' do
+      let(:service_params) { { active: true } }
 
-        expect(response).to redirect_to(project_settings_integrations_path(project))
-        expect(flash[:notice]).to eq 'Jira activated.'
+      before do
+        put :update, params: project_params(service: service_params)
+      end
+
+      context 'when param `active` is set to true' do
+        it 'activates the service and redirects to integrations paths' do
+          expect(response).to redirect_to(project_settings_integrations_path(project))
+          expect(flash[:notice]).to eq 'Jira activated.'
+        end
+      end
+
+      context 'when param `active` is set to false' do
+        let(:service_params) { { active: false } }
+
+        it 'does not activate the service but saves the settings' do
+          expect(flash[:notice]).to eq 'Jira settings saved, but not activated.'
+        end
       end
-    end
 
-    context 'when param `active` is set to false' do
-      it 'does not  activate the service but saves the settings' do
-        put :update,
-          params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: false } }
+      context 'when activating Jira service from a template' do
+        let(:service) do
+          create(:jira_service, project: project, template: true)
+        end
 
-        expect(flash[:notice]).to eq 'Jira settings saved, but not activated.'
+        it 'activate Jira service from template' do
+          expect(flash[:notice]).to eq 'Jira activated.'
+        end
       end
     end
 
-    context 'when activating Jira service from a template' do
-      let(:template_service) { create(:jira_service, project: project, template: true) }
+    describe 'as JSON' do
+      before do
+        put :update, params: project_params(service: service_params, format: :json)
+      end
+
+      context 'when update succeeds' do
+        let(:service_params) { { url: 'http://example.com' } }
+
+        it 'returns JSON response with no errors' do
+          expect(response).to be_successful
+          expect(json_response).to include('active' => true, 'errors' => {})
+        end
+      end
 
-      it 'activate Jira service from template' do
-        put :update, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true } }
+      context 'when update fails' do
+        let(:service_params) { { url: '' } }
 
-        expect(flash[:notice]).to eq 'Jira activated.'
+        it 'returns JSON response with errors' do
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
+          expect(json_response).to include(
+            'active' => true,
+            'errors' => { 'url' => ['must be a valid URL', %{can't be blank}] }
+          )
+        end
       end
     end
   end
 
-  describe "GET #edit" do
+  describe 'GET #edit' do
     before do
-      get :edit, params: { namespace_id: project.namespace, project_id: project, id: 'jira' }
+      get :edit, params: project_params(id: 'jira')
     end
 
     context 'with approved services' do
@@ -168,4 +204,14 @@ describe Projects::ServicesController do
       end
     end
   end
+
+  private
+
+  def project_params(opts = {})
+    opts.reverse_merge(
+      namespace_id: project.namespace,
+      project_id: project,
+      id: service.to_param
+    )
+  end
 end