WIP

8 files changed, 40 insertions, 49 deletions

diff --git a/app/controllers/projects/mattermosts_controller.rb b/app/controllers/projects/mattermosts_controller.rb
index 0f939838306..d87dff2a80e 100644
--- a/app/controllers/projects/mattermosts_controller.rb
+++ b/app/controllers/projects/mattermosts_controller.rb
@@ -12,13 +12,17 @@ class Projects::MattermostsController < Projects::ApplicationController
   end
 
   def create
-    @service.configure!(current_user, configure_params)
-
-    flash[:notice] = 'This service is now configured'
-    redirect_to edit_namespace_project_service_path(@project.namespace, @project, service)
-  rescue => e
-    flash[:alert] = e.message
-    redirect_to new_namespace_project_mattermost_path(@project.namespace, @project)
+    result, message = @service.configure(current_user, configure_params)
+
+    if result
+      flash[:notice] = 'This service is now configured'
+      redirect_to edit_namespace_project_service_path(
+        @project.namespace, @project, service)
+    else
+      flash[:alert] = message || 'Failed to configure service'
+      redirect_to new_namespace_project_mattermost_path(
+        @project.namespace, @project)
+    end
   end
 
   private
@@ -31,9 +35,6 @@ class Projects::MattermostsController < Projects::ApplicationController
 
   def teams
     @teams ||= @service.list_teams(current_user)
-  rescue => e
-    @teams = []
-    flash[:alert] = e.message
   end
 
   def service
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index adb5eeee3e4..c816b616631 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -294,8 +294,4 @@ module ApplicationHelper
   def page_class
     "issue-boards-page" if current_controller?(:boards)
   end
-
-  def pretty_url(url)
-    url.gsub(/\A.*?:\/\//, '')
-  end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index bd2dcb08b3e..d2177f683a1 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -148,14 +148,6 @@ module ProjectsHelper
     ).html_safe
   end
 
-  def mattermost_teams_options(teams)
-    teams_options = teams.map do |id, options|
-      [options['display_name'] || options['name'], id]
-    end
-
-    teams_options.compact.unshift(['Select team...', '0'])
-  end
-
   private
 
   def repo_children_classes(field)
diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb
index 572a02b01d4..4c2b7d64f1f 100644
--- a/app/models/project_services/mattermost_slash_commands_service.rb
+++ b/app/models/project_services/mattermost_slash_commands_service.rb
@@ -19,15 +19,19 @@ class MattermostSlashCommandsService < ChatSlashCommandsService
     'mattermost_slash_commands'
   end
 
-  def configure!(user, params)
+  def configure(user, params)
     token = Mattermost::Command.new(user).
       create(command(params))
 
-    update!(active: true, token: token)
+    update(active: true, token: token) if token
+  rescue => Mattermost::Error => e
+    false, e.message
   end
 
   def list_teams(user)
     Mattermost::Team.new(user).all
+  rescue => Mattermost::Error => e
+    []
   end
 
   private
diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb
index d6759eac0d0..6eaaed34063 100644
--- a/lib/mattermost/client.rb
+++ b/lib/mattermost/client.rb
@@ -29,13 +29,13 @@ module Mattermost
     def json_response(response)
       json_response = JSON.parse(response.body)
 
-      if response.success?
-        json_response
-      elsif json_response['message']
-        raise ClientError(json_response['message'])
-      else
-        raise ClientError('Undefined error')
+      unless response.success?
+        raise ClientError(json_response['message'] || 'Undefined error')
       end
+
+      json_response
+    rescue JSON::JSONError => e
+      raise ClientError('Cannot parse response')
     end
   end
 end
diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb
index e36500d24a3..7a7912d02fc 100644
--- a/lib/mattermost/session.rb
+++ b/lib/mattermost/session.rb
@@ -3,15 +3,11 @@ module Mattermost
 
   class NoSessionError < Error
     def message
-      'No session could be set up, is Mattermost configured with Single Sign on?'
+      'No session could be set up, is Mattermost configured with Single Sign On?'
     end
   end
 
-  class ConnectionError < Error
-    def message
-      'Could not connect. Is Mattermost up?'
-    end
-  end
+  class ConnectionError < Error; end
 
   # This class' prime objective is to obtain a session token on a Mattermost
   # instance with SSO configured where this GitLab instance is the provider.
@@ -74,12 +70,16 @@ module Mattermost
 
     def get(path, options = {})
       self.class.get(path, options.merge(headers: @headers))
-    rescue Errno::ECONNREFUSED
-      raise ConnectionError
+    rescue HTTParty::Error => e
+      raise ConnectionError(e.message)
+    rescue Errno::ECONNREFUSED => e
+      raise ConnectionError(e.message)
     end
 
     def post(path, options = {})
       self.class.post(path, options.merge(headers: @headers))
+    rescue HTTParty::Error => e
+      raise ConnectionError(e.message)
     rescue Errno::ECONNREFUSED
       raise ConnectionError
     end
diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb
index f38bb273e7d..ecf336e3199 100644
--- a/spec/lib/mattermost/command_spec.rb
+++ b/spec/lib/mattermost/command_spec.rb
@@ -1,19 +1,17 @@
 require 'spec_helper'
 
 describe Mattermost::Command do
-  let(:session) { double("session") }
   let(:hash) { { 'token' => 'token' } }
+  let(:user) { create(:user) }
 
-  describe '.create' do
-    before do
-      allow(session).to receive(:post).and_return(hash)
-      allow(hash).to receive(:parsed_response).and_return(hash)
-    end
-
-    it 'gets the teams' do
-      expect(session).to receive(:post)
+  before do
+    Mattermost::Session.base_uri("http://mattermost.example.com")
+  end
 
-      described_class.create(session, 'abc', url: 'http://trigger.com')
+  describe '#create' do
+    it 'creates a command' do
+      described_class.new(user).
+        create(team_id: 'abc', url: 'http://trigger.com')
     end
   end
 end
diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb
index c208be3912b..5d1fc6fc603 100644
--- a/spec/lib/mattermost/team_spec.rb
+++ b/spec/lib/mattermost/team_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Mattermost::Team do
-  describe '.team_admin' do
+  describe '#all' do
     let(:session) { double("session") }
 
     let(:response) do