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 57c724558 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

3 files changed, 29 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index df598502820..0b096612d28 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 7.12.0 (unreleased)
   - Refactor permission checks with issues and merge requests project settings (Stan Hu)
   - Fix Markdown preview not working in Edit Milestone page (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)
   - 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)
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 38cb64f8c48..6761f00183e 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -63,7 +63,7 @@ class HipchatService < Service
   private
 
   def gate
-    options = { api_version: api_version || 'v2' }
+    options = { api_version: api_version.present? ? api_version : 'v2' }
     options[:server_url] = server unless server.blank?
     @gate ||= HipChat::Client.new(token, options)
   end
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index bbaf54488be..e88615e1a2e 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -32,21 +32,44 @@ describe HipchatService do
     let(:project) { create(:project, name: 'project') }
     let(:api_url) { 'https://hipchat.example.com/v2/room/123456/notification?auth_token=verySecret' }
     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
       hipchat.stub(
         project_id: project.id,
         project: project,
         room: 123456,
-        server: 'https://hipchat.example.com',
-        token: 'verySecret'
+        server: server_url,
+        token: token
       )
       WebMock.stub_request(:post, api_url)
     end
 
-    context 'push events' do
-      let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    it 'should use v1 if version is provided' do
+      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
         hipchat.execute(push_sample_data)