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) + result, message = @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) + 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 + before do + Mattermost::Session.base_uri("http://mattermost.example.com") + end - it 'gets the teams' do - expect(session).to receive(:post) - - 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