Commit 62d0ce19 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix-hipchat-default-api-version' into 'master'

Allow HipChat API version to be blank and default to v2

### What does this MR do?

This MR fixes a regression introduced in v7.11 that requires a HipChat API version to be specified when it is supposed to be optional.

### Why was this MR needed?

The "optional" HipChat API version in 57c72455 passed in a blank `api_version` when nothing was specified, and the code was not tested. This would cause a 500 Error.

### What are the relevant issue numbers?

Closes #772

See merge request !718
parents 8044b4d2 0c946317
...@@ -4,6 +4,7 @@ v 7.12.0 (unreleased) ...@@ -4,6 +4,7 @@ v 7.12.0 (unreleased)
- Refactor permission checks with issues and merge requests project settings (Stan Hu) - Refactor permission checks with issues and merge requests project settings (Stan Hu)
- Fix Markdown preview not working in Edit Milestone page (Stan Hu) - Fix Markdown preview not working in Edit Milestone page (Stan Hu)
- Fix Zen Mode not closing with ESC key (Stan Hu) - Fix Zen Mode not closing with ESC key (Stan Hu)
- Allow HipChat API version to be blank and default to v2 (Stan Hu)
- Add web hook support for note events (Stan Hu) - Add web hook support for note events (Stan Hu)
- Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu) - Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu)
- Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu) - Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu)
......
...@@ -63,7 +63,7 @@ class HipchatService < Service ...@@ -63,7 +63,7 @@ class HipchatService < Service
private private
def gate def gate
options = { api_version: api_version || 'v2' } options = { api_version: api_version.present? ? api_version : 'v2' }
options[:server_url] = server unless server.blank? options[:server_url] = server unless server.blank?
@gate ||= HipChat::Client.new(token, options) @gate ||= HipChat::Client.new(token, options)
end end
......
...@@ -32,21 +32,44 @@ describe HipchatService do ...@@ -32,21 +32,44 @@ describe HipchatService do
let(:project) { create(:project, name: 'project') } let(:project) { create(:project, name: 'project') }
let(:api_url) { 'https://hipchat.example.com/v2/room/123456/notification?auth_token=verySecret' } let(:api_url) { 'https://hipchat.example.com/v2/room/123456/notification?auth_token=verySecret' }
let(:project_name) { project.name_with_namespace.gsub(/\s/, '') } let(:project_name) { project.name_with_namespace.gsub(/\s/, '') }
let(:token) { 'verySecret' }
let(:server_url) { 'https://hipchat.example.com'}
let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
before(:each) do before(:each) do
hipchat.stub( hipchat.stub(
project_id: project.id, project_id: project.id,
project: project, project: project,
room: 123456, room: 123456,
server: 'https://hipchat.example.com', server: server_url,
token: 'verySecret' token: token
) )
WebMock.stub_request(:post, api_url) WebMock.stub_request(:post, api_url)
end end
context 'push events' do it 'should use v1 if version is provided' do
let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } hipchat.stub(api_version: 'v1')
expect(HipChat::Client).to receive(:new).
with(token,
api_version: 'v1',
server_url: server_url).
and_return(
double(:hipchat_service).as_null_object)
hipchat.execute(push_sample_data)
end
it 'should use v2 as the version when nothing is provided' do
hipchat.stub(api_version: '')
expect(HipChat::Client).to receive(:new).
with(token,
api_version: 'v2',
server_url: server_url).
and_return(
double(:hipchat_service).as_null_object)
hipchat.execute(push_sample_data)
end
context 'push events' do
it "should call Hipchat API for push events" do it "should call Hipchat API for push events" do
hipchat.execute(push_sample_data) hipchat.execute(push_sample_data)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment