Merge branch 'ce-jej/github-project-service-for-ci' into 'master'

app/controllers/projects/services_controller.rb

@@ -3,7 +3,8 @@ class Projects::ServicesController < Projects::ApplicationController
 
   # Authorize
   before_action :authorize_admin_project!
-  before_action :service, only: [:edit, :update, :test]
+  before_action :ensure_service_enabled
+  before_action :service
 
   respond_to :html
 
@@ -23,26 +24,30 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def test
-    message = {}
+    if @service.can_test?
+      render json: service_test_response, status: :ok
+    else
+      render json: {}, status: :not_found
+    end
+  end
 
-    if @service.can_test? && @service.update_attributes(service_params[:service])
+  private
+
+  def service_test_response
+    if @service.update_attributes(service_params[:service])
       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 }
+      if outcome[:success]
+        {}
+      else
+        { error: true, message: 'Test failed.', service_response: outcome[:result].to_s }
       end
-
-      status = :ok
     else
-      status = :not_found
+      { error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(',') }
     end
-
-    render json: message, status: status
   end
 
-  private
-
   def success_message
     if @service.active?
       "#{@service.title} activated."
@@ -54,4 +59,8 @@ class Projects::ServicesController < Projects::ApplicationController
   def service
     @service ||= @project.find_or_initialize_service(params[:id])
   end
+
+  def ensure_service_enabled
+    render_404 unless service
+  end
 end

app/models/service.rb

@@ -129,6 +129,17 @@ class Service < ActiveRecord::Base
     fields
   end
 
+  def configurable_events
+    events = self.class.supported_events
+
+    # No need to disable individual triggers when there is only one
+    if events.count == 1
+      []
+    else
+      events
+    end
+  end
+
   def supported_events
     self.class.supported_events
   end

app/views/projects/services/_form.html.haml

@@ -5,6 +5,9 @@
       = boolean_to_icon @service.activated?
 
     %p= @service.description
+
+    - if @service.respond_to?(:detailed_description)
+      %p= @service.detailed_description
   .col-lg-9
     = form_for(@service, as: :service, url: project_service_path(@project, @service.to_param), method: :put, html: { class: 'gl-show-field-errors form-horizontal integration-settings-form js-integration-settings-form', data: { 'can-test' => @service.can_test?, 'test-url' => test_project_service_path(@project, @service) } }) do |form|
       = render 'shared/service_settings', form: form, subject: @service

app/views/shared/_service_settings.html.haml

@@ -13,12 +13,12 @@
       .col-sm-10
         = form.check_box :active, disabled: disable_fields_service?(@service)
 
-  - if @service.supported_events.present?
+  - if @service.configurable_events.present?
     .form-group
       = form.label :url, "Trigger", class: 'control-label'
 
       .col-sm-10
-        - @service.supported_events.each do |event|
+        - @service.configurable_events.each do |event|
           %div
             = form.check_box service_event_field_name(event), class: 'pull-left'
             .prepend-left-20

changelogs/unreleased/ce-jej-github-project-service-for-ci.yml

+---
+title: Hook data for pipelines includes detailed_status
+merge_request: 17607
+author:
+type: changed

changelogs/unreleased/ce-jej-integrations-can-hide-trigger-checkboxes.yml

+---
+title: Avoid showing unnecessary Trigger checkboxes for project Integrations with
+  only one event
+merge_request: 17607
+author:
+type: changed

config/routes/project.rb

@@ -69,7 +69,7 @@ constraints(ProjectUrlConstrainer.new) do
         end
       end
 
-      resources :services, constraints: { id: %r{[^/]+} }, only: [:index, :edit, :update] do
+      resources :services, constraints: { id: %r{[^/]+} }, only: [:edit, :update] do
         member do
           put :test
         end

lib/gitlab/data_builder/pipeline.rb

@@ -22,6 +22,7 @@ module Gitlab
           sha: pipeline.sha,
           before_sha: pipeline.before_sha,
           status: pipeline.status,
+          detailed_status: pipeline.detailed_status(nil).label,
           stages: pipeline.stages_names,
           created_at: pipeline.created_at,
           finished_at: pipeline.finished_at,

spec/controllers/projects/services_controller_spec.rb

@@ -23,6 +23,18 @@ describe Projects::ServicesController do
       end
     end
 
+    context 'when validations fail' do
+      let(:service_params) { { active: 'true', token: '' } }
+
+      it 'returns error messages in JSON response' do
+        put :test, namespace_id: project.namespace, project_id: project, id: :hipchat, service: service_params
+
+        expect(json_response['message']).to eq "Validations failed."
+        expect(json_response['service_response']).to eq "Token can't be blank"
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
     context 'success' do
       context 'with empty project' do
         let(:project) { create(:project) }

spec/features/projects/services/disable_triggers_spec.rb

+require 'spec_helper'
+
+describe 'Disable individual triggers' do
+  let(:project) { create(:project) }
+  let(:user) { project.owner }
+  let(:checkbox_selector) { 'input[type=checkbox][id$=_events]' }
+
+  before do
+    sign_in(user)
+
+    visit(project_settings_integrations_path(project))
+
+    click_link(service_name)
+  end
+
+  context 'service has multiple supported events' do
+    let(:service_name) { 'HipChat' }
+
+    it 'shows trigger checkboxes' do
+      event_count = HipchatService.supported_events.count
+
+      expect(page).to have_content "Trigger"
+      expect(page).to have_css(checkbox_selector, count: event_count)
+    end
+  end
+
+  context 'services only has one supported event' do
+    let(:service_name) { 'Asana' }
+
+    it "doesn't show unnecessary Trigger checkboxes" do
+      expect(page).not_to have_content "Trigger"
+      expect(page).not_to have_css(checkbox_selector)
+    end
+  end
+end

spec/lib/gitlab/data_builder/pipeline_spec.rb

@@ -26,6 +26,7 @@ describe Gitlab::DataBuilder::Pipeline do
     it { expect(attributes[:tag]).to eq(pipeline.tag) }
     it { expect(attributes[:id]).to eq(pipeline.id) }
     it { expect(attributes[:status]).to eq(pipeline.status) }
+    it { expect(attributes[:detailed_status]).to eq('passed') }
 
     it { expect(build_data).to be_a(Hash) }
     it { expect(build_data[:id]).to eq(build.id) }