Merge branch 'zj-mattermost-slash-config' of gitlab.com:gitlab-org/gitlab-ce into zj-mattermost-slash-config

8 files changed, 92 insertions, 43 deletions

diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb
index 4c2b7d64f1f..fc1e7d79d08 100644
--- a/app/models/project_services/mattermost_slash_commands_service.rb
+++ b/app/models/project_services/mattermost_slash_commands_service.rb
@@ -24,13 +24,13 @@ class MattermostSlashCommandsService < ChatSlashCommandsService
       create(command(params))
 
     update(active: true, token: token) if token
-  rescue => Mattermost::Error => e
-    false, e.message
+  rescue Mattermost::Error => e
+    [false, e.message]
   end
 
   def list_teams(user)
     Mattermost::Team.new(user).all
-  rescue => Mattermost::Error => e
+  rescue Mattermost::Error
     []
   end
 
diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb
index 6eaaed34063..fa3c9fa27bd 100644
--- a/lib/mattermost/client.rb
+++ b/lib/mattermost/client.rb
@@ -34,7 +34,7 @@ module Mattermost
       end
 
       json_response
-    rescue JSON::JSONError => e
+    rescue JSON::JSONError
       raise ClientError('Cannot parse response')
     end
   end
diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb
index 38a71061097..8f14ed306b8 100644
--- a/lib/mattermost/session.rb
+++ b/lib/mattermost/session.rb
@@ -75,9 +75,9 @@ module Mattermost
     def get(path, options = {})
       self.class.get(path, options.merge(headers: @headers))
     rescue HTTParty::Error => e
-      raise ConnectionError(e.message)
+      raise Mattermost::ConnectionError.new(e.message)
     rescue Errno::ECONNREFUSED => e
-      raise ConnectionError(e.message)
+      raise Mattermost::ConnectionError.new(e.message)
     end
 
     def post(path, options = {})
diff --git a/spec/controllers/projects/mattermosts_controller_spec.rb b/spec/controllers/projects/mattermosts_controller_spec.rb
new file mode 100644
index 00000000000..3f9482b0cde
--- /dev/null
+++ b/spec/controllers/projects/mattermosts_controller_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe Projects::MattermostsController do
+  let!(:project) { create(:empty_project) }
+  let!(:user) { create(:user) }
+
+  before do
+    project.team << [user, :master]
+    sign_in(user)
+  end
+
+  describe 'GET #new' do
+    before do
+      get(:new,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param)
+    end
+
+    it 'accepts the request' do
+      expect(response).to have_http_status(200)
+    end
+  end
+
+  describe 'POST #create' do
+    let(:mattermost_params) { { trigger: 'http://localhost:3000/trigger', team_id: 'abc' } }
+
+    subject do
+      post(:create,
+           namespace_id: project.namespace.to_param,
+           project_id: project.to_param,
+           mattermost: mattermost_params)
+    end
+
+    context 'no request can be made to mattermost' do
+      it 'shows the error' do
+        expect(controller).to set_flash[:alert].to /\AFailed to open TCP connection to/
+      end
+    end
+
+    context 'the request is succesull' do
+      before do
+        allow_any_instance_of(Mattermost::Command).to receive(:create).and_return('token')
+      end
+
+      it 'redirects to the new page' do
+        subject
+        service = project.services.last
+
+        expect(subject).to redirect_to(edit_namespace_project_service_url(project.namespace, project, service))
+      end
+    end
+  end
+end
diff --git a/spec/features/projects/services/mattermost_slash_command_spec.rb b/spec/features/projects/services/mattermost_slash_command_spec.rb
index 521eedeae9e..274d50e7ce4 100644
--- a/spec/features/projects/services/mattermost_slash_command_spec.rb
+++ b/spec/features/projects/services/mattermost_slash_command_spec.rb
@@ -4,10 +4,12 @@ feature 'Setup Mattermost slash commands', feature: true do
   include WaitForAjax
 
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:empty_project) }
   let(:service) { project.create_mattermost_slash_commands_service }
+  let(:mattermost_enabled) { true }
 
   before do
+    Settings.mattermost['enabled'] = mattermost_enabled
     project.team << [user, :master]
     login_as(user)
     visit edit_namespace_project_service_path(project.namespace, project, service)
@@ -32,20 +34,13 @@ feature 'Setup Mattermost slash commands', feature: true do
     end
 
     describe 'mattermost service is enabled' do
-      before do
-        allow(Gitlab.config.mattermost).to receive(:enabled).and_return(true)
-      end
-
       it 'shows the add to mattermost button' do
         expect(page).to have_link 'Add to Mattermost'
       end
     end
 
-
     describe 'mattermost service is not enabled' do
-      before do
-        allow(Gitlab.config.mattermost).to receive(:enabled).and_return(false)
-      end
+      let(:mattermost_enabled) { false }
 
       it 'shows the correct trigger url' do
         value = find_field('request_url').value
diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb
index ecf336e3199..f70aee7f3e5 100644
--- a/spec/lib/mattermost/command_spec.rb
+++ b/spec/lib/mattermost/command_spec.rb
@@ -1,17 +1,22 @@
 require 'spec_helper'
 
 describe Mattermost::Command do
-  let(:hash) { { 'token' => 'token' } }
-  let(:user) { create(:user) }
+  let(:params) { { 'token' => 'token', team_id: 'abc' } }
+  let(:user) { build(:user) }
 
   before do
     Mattermost::Session.base_uri("http://mattermost.example.com")
   end
 
+  subject { described_class.new(user) }
+
   describe '#create' do
-    it 'creates a command' do
-      described_class.new(user).
-        create(team_id: 'abc', url: 'http://trigger.com')
+    it 'interpolates the team id' do
+      allow(subject).to receive(:json_post).
+        with('/api/v3/teams/abc/commands/create', body: params.to_json).
+        and_return('token' => 'token')
+
+      subject.create(params)
     end
   end
 end
diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb
index 5d1fc6fc603..ef39c456d5f 100644
--- a/spec/lib/mattermost/team_spec.rb
+++ b/spec/lib/mattermost/team_spec.rb
@@ -2,7 +2,9 @@ require 'spec_helper'
 
 describe Mattermost::Team do
   describe '#all' do
-    let(:session) { double("session") }
+    let(:user) { build(:user) }
+
+    subject { described_class.new(user) }
 
     let(:response) do
       [{
@@ -20,22 +22,13 @@ describe Mattermost::Team do
         "allow_open_invite" => false }]
     end
 
-    let(:json) { nil }
 
     before do
-      allow(session).to receive(:get).with('/api/v3/teams/all').
-        and_return(json)
-      allow(json).to receive(:parsed_response).and_return(response)
+      allow(subject).to receive(:json_get).and_return(response)
     end
 
     it 'gets the teams' do
-      expect(described_class.all(session).count).to be(1)
-    end
-
-    it 'filters on being team admin' do
-      ids = described_class.all(session).map { |team| team['id'] }
-
-      expect(ids).to include("xiyro8huptfhdndadpz8r3wnbo")
+      expect(subject.all.count).to be(1)
     end
   end
 end
diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
index 9fb6132d171..850ca45ddd8 100644
--- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb
+++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
@@ -3,23 +3,23 @@ require 'spec_helper'
 describe MattermostSlashCommandsService, :models do
   it_behaves_like "chat slash commands service"
 
-  describe '#configure!' do
+  describe '#configure' do
     let(:project) { create(:empty_project) }
     let(:service) { project.build_mattermost_slash_commands_service }
     let(:user) { create(:user)}
 
-    before do
-      allow_any_instance_of(Mattermost::Session).to
-        receive(:with_session).and_yield
-    end
-
     subject do
-      service.configure!(user, team_id: 'abc',
-        trigger: 'gitlab', url: 'http://trigger.url',
-        icon_url: 'http://icon.url/icon.png')
+      service.configure(user, team_id: 'abc',
+                               trigger: 'gitlab', url: 'http://trigger.url',
+                               icon_url: 'http://icon.url/icon.png')
     end
 
     context 'the requests succeeds' do
+      before do
+        allow_any_instance_of(Mattermost::Command).
+          to receive(:json_post).and_return('token' => 'token')
+      end
+
       it 'saves the service' do
         expect { subject }.to change { project.services.count }.by(1)
       end
@@ -27,13 +27,16 @@ describe MattermostSlashCommandsService, :models do
       it 'saves the token' do
         subject
 
-        expect(service.reload.token).to eq('mynewtoken')
+        expect(service.reload.token).to eq('token')
       end
     end
 
     context 'an error is received' do
       it 'shows error messages' do
-        expect(subject).to raise_error("Error")
+        succeeded, message = subject
+
+        expect(succeeded).to be(false)
+        expect(message).to start_with("Failed to open TCP connection to")
       end
     end
   end