diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-08-13 11:43:44 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-08-13 11:43:44 +0000 |
commit | 00a33d67aa5dfcfbc63182106162041d2c60f4de (patch) | |
tree | 73035cff0dc392f1bce5f7a3388294afa686a457 | |
parent | 167ab75365b35b9ff10c399e1c3b97f3b3aeb351 (diff) | |
parent | 23790570026ce78e3b4cbbf1b2f32ada992c5f40 (diff) | |
download | gitlab-ce-00a33d67aa5dfcfbc63182106162041d2c60f4de.tar.gz |
Merge branch 'improve-hipchat-service-test' into 'master'
Provide more feedback what went wrong if HipChat service failed test
This MR adds support for displaying the error message during HipChat service test. Before an Error 500 would be displayed with no helpful remarks.
Screenshot:
![image](https://gitlab.com/stanhu/gitlab-ce/uploads/10f82eb02db138f9680e69cdb3d3ed82/image.png)
Issue gitlab-com/support-forum#213
See merge request !1144
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/projects/services_controller.rb | 7 | ||||
-rw-r--r-- | app/models/project_services/hipchat_service.rb | 10 | ||||
-rw-r--r-- | app/models/service.rb | 8 | ||||
-rw-r--r-- | spec/controllers/projects/services_controller_spec.rb | 35 | ||||
-rw-r--r-- | spec/models/project_services/hipchat_service_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/service_spec.rb | 10 |
7 files changed, 76 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index 8a378670c41..bbbee1d929a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.14.0 (unreleased) + - Provide more feedback what went wrong if HipChat service failed test (Stan Hu) - Disable turbolinks when linking to Bitbucket import status (Stan Hu) - Fix broken code import and display error messages if something went wrong with creating project (Stan Hu) - Fix corrupted binary files when using API files endpoint (Stan Hu) diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 1e435be8275..01105532479 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -39,10 +39,13 @@ class Projects::ServicesController < Projects::ApplicationController def test data = Gitlab::PushDataBuilder.build_sample(project, current_user) - if @service.execute(data) + outcome = @service.test(data) + if outcome[:success] message = { notice: 'We sent a request to the provided URL' } else - message = { alert: 'We tried to send a request to the provided URL but an error occured' } + 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 } end redirect_to :back, message diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 6761f00183e..7a15a861abc 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -60,6 +60,16 @@ class HipchatService < Service gate[room].send('GitLab', message, message_options) end + def test(data) + begin + result = execute(data) + rescue StandardError => error + return { success: false, result: error } + end + + { success: true, result: result } + end + private def gate diff --git a/app/models/service.rb b/app/models/service.rb index 818a6808db5..dcef2866c3b 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -87,10 +87,16 @@ class Service < ActiveRecord::Base %w(push tag_push issue merge_request) end - def execute + def execute(data) # implement inside child end + def test(data) + # default implementation + result = execute(data) + { success: result.present?, result: result } + end + def can_test? !project.empty_repo? end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb new file mode 100644 index 00000000000..d4ecd98e12d --- /dev/null +++ b/spec/controllers/projects/services_controller_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Projects::ServicesController do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:service) { create(:service, project: project) } + + before do + sign_in(user) + project.team << [user, :master] + controller.instance_variable_set(:@project, project) + controller.instance_variable_set(:@service, service) + request.env["HTTP_REFERER"] = "/" + end + + describe "#test" do + context 'success' do + it "should redirect and show success message" do + expect(service).to receive(:test).and_return({ success: true, result: 'done' }) + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + expect(response.status).to redirect_to('/') + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + + context 'failure' do + it "should redirect and show failure message" do + expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + expect(response.status).to redirect_to('/') + expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') + end + end + end +end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 4707673269a..65d16beef91 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -47,6 +47,14 @@ describe HipchatService do WebMock.stub_request(:post, api_url) end + it 'should test and return errors' do + allow(hipchat).to receive(:execute).and_raise(StandardError, 'no such room') + result = hipchat.test(push_sample_data) + + expect(result[:success]).to be_falsey + expect(result[:result].to_s).to eq('no such room') + end + it 'should use v1 if version is provided' do allow(hipchat).to receive(:api_version).and_return('v1') expect(HipChat::Client).to receive(:new). diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index ca11758ee06..a213ffe6c4b 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -46,6 +46,16 @@ describe Service do describe :can_test do it { expect(@testable).to eq(true) } end + + describe :test do + let(:data) { 'test' } + + it 'test runs execute' do + expect(@service).to receive(:execute).with(data) + + @service.test(data) + end + end end describe "With commits" do |