diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index fc1e7d79d087dfdb573f096918774a8edfaa5fb1..6c78c0af71c0e201832c932b36e3a6fc463afc54 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -30,8 +30,8 @@ class MattermostSlashCommandsService < ChatSlashCommandsService def list_teams(user) Mattermost::Team.new(user).all - rescue Mattermost::Error - [] + rescue Mattermost::Error => e + [[], e.message] end private @@ -44,7 +44,7 @@ class MattermostSlashCommandsService < ChatSlashCommandsService auto_complete_desc: "Perform common operations on: #{pretty_project_name}", auto_complete_hint: '[help]', description: "Perform common operations on: #{pretty_project_name}", - display_name: "GitLab / #{pretty_project_name}", + display_name: "GitLab / #{pretty_project_name}", method: 'P', user_name: 'GitLab') end diff --git a/changelogs/unreleased/mattermost-slash-auto-config.yml b/changelogs/unreleased/mattermost-slash-auto-config.yml new file mode 100644 index 0000000000000000000000000000000000000000..43014d38769af8667b932e28402675acedfd6dc8 --- /dev/null +++ b/changelogs/unreleased/mattermost-slash-auto-config.yml @@ -0,0 +1,4 @@ +--- +title: Allow to auto-configure Mattermost +merge_request: 8070 +author: diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb index ddfeb88a71f74a3aa792465540a54837ba93cb84..377cb7b10211f4b8101d923d6fb99c0f0500cb32 100644 --- a/lib/mattermost/session.rb +++ b/lib/mattermost/session.rb @@ -1,11 +1,11 @@ module Mattermost - class NoSessionError < Error + class NoSessionError < Mattermost::Error def message 'No session could be set up, is Mattermost configured with Single Sign On?' end end - class ConnectionError < Error; end + class ConnectionError < Mattermost::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. @@ -36,12 +36,12 @@ module Mattermost def with_session with_lease do - raise NoSessionError unless create + raise Mattermost::NoSessionError unless create begin yield self rescue Errno::ECONNREFUSED - raise NoSessionError + raise Mattermost::NoSessionError ensure destroy end @@ -71,19 +71,15 @@ module Mattermost end def get(path, options = {}) - self.class.get(path, options.merge(headers: @headers)) - rescue HTTParty::Error => e - raise Mattermost::ConnectionError.new(e.message) - rescue Errno::ECONNREFUSED => e - raise Mattermost::ConnectionError.new(e.message) + handle_exceptions do + self.class.get(path, options.merge(headers: @headers)) + end end def post(path, options = {}) - self.class.post(path, options.merge(headers: @headers)) - rescue HTTParty::Error => e - raise Mattermost::ConnectionError.new(e.message) - rescue Errno::ECONNREFUSED - raise Mattermost::ConnectionError.new(e.message) + handle_exceptions do + self.class.post(path, options.merge(headers: @headers)) + end end private @@ -152,5 +148,13 @@ module Mattermost lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) lease.try_obtain end + + def handle_exceptions + yield + rescue HTTParty::Error => e + raise Mattermost::ConnectionError.new(e.message) + rescue Errno::ECONNREFUSED + raise Mattermost::ConnectionError.new(e.message) + end end end diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb index f70aee7f3e515b6203ee69d5fbf6ee7db76b3f09..5ccf11008980a827f48a146fe31f22f92a87b9b1 100644 --- a/spec/lib/mattermost/command_spec.rb +++ b/spec/lib/mattermost/command_spec.rb @@ -2,21 +2,60 @@ require 'spec_helper' describe Mattermost::Command do let(:params) { { 'token' => 'token', team_id: 'abc' } } - let(:user) { build(:user) } before do - Mattermost::Session.base_uri("http://mattermost.example.com") - end + Mattermost::Session.base_uri('http://mattermost.example.com') - subject { described_class.new(user) } + allow_any_instance_of(Mattermost::Client).to receive(:with_session). + and_yield(Mattermost::Session.new(nil)) + end describe '#create' do - 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') + let(:params) do + { team_id: 'abc', + trigger: 'gitlab' + } + end + + subject { described_class.new(nil).create(params) } + + context 'for valid trigger word' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + with(body: { + team_id: 'abc', + trigger: 'gitlab' }.to_json). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: { token: 'token' }.to_json + ) + end + + it 'returns a token' do + is_expected.to eq('token') + end + end + + context 'for error message' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + id: 'api.command.duplicate_trigger.app_error', + message: 'This trigger word is already in use. Please choose another word.', + detailed_error: '', + request_id: 'obc374man7bx5r3dbc1q5qhf3r', + status_code: 500 + }.to_json + ) + end - subject.create(params) + it 'raises an error with message' do + expect { subject }.to raise_error(Mattermost::Error, 'This trigger word is already in use. Please choose another word.') + end end end end diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb index 704579f0f48ab44e0566f6044e471b9f006f192b..2d14be6bcc2d277b017f9ff8ec48923633e33a71 100644 --- a/spec/lib/mattermost/team_spec.rb +++ b/spec/lib/mattermost/team_spec.rb @@ -1,32 +1,66 @@ require 'spec_helper' describe Mattermost::Team do + before do + Mattermost::Session.base_uri('http://mattermost.example.com') + + allow_any_instance_of(Mattermost::Client).to receive(:with_session). + and_yield(Mattermost::Session.new(nil)) + end + describe '#all' do - let(:user) { build(:user) } - let(:response) do - [{ - "id" => "xiyro8huptfhdndadpz8r3wnbo", - "create_at" => 1482174222155, - "update_at" => 1482174222155, - "delete_at" => 0, - "display_name" => "chatops", - "name" => "chatops", - "email" => "admin@example.com", - "type" => "O", - "company_name" => "", - "allowed_domains" => "", - "invite_id" => "o4utakb9jtb7imctdfzbf9r5ro", - "allow_open_invite" => false }] - end + subject { described_class.new(nil).all } + + context 'for valid request' do + let(:response) do + [{ + "id" => "xiyro8huptfhdndadpz8r3wnbo", + "create_at" => 1482174222155, + "update_at" => 1482174222155, + "delete_at" => 0, + "display_name" => "chatops", + "name" => "chatops", + "email" => "admin@example.com", + "type" => "O", + "company_name" => "", + "allowed_domains" => "", + "invite_id" => "o4utakb9jtb7imctdfzbf9r5ro", + "allow_open_invite" => false }] + end - subject { described_class.new(user) } + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: response.to_json + ) + end - before do - allow(subject).to receive(:json_get).and_return(response) + it 'returns a token' do + is_expected.to eq(response) + end end - it 'gets the teams' do - expect(subject.all.count).to be(1) + context 'for error message' do + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + id: 'api.team.list.app_error', + message: 'Cannot list teams.', + detailed_error: '', + request_id: 'obc374man7bx5r3dbc1q5qhf3r', + status_code: 500 + }.to_json + ) + end + + it 'raises an error with message' do + expect { subject }.to raise_error(Mattermost::Error, 'Cannot list teams.') + end 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 850ca45ddd8581c30e9f05b285a2ce123ebe43fe..d6f4fbd7265421c63f6332bd868eb504a901fe85 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -3,40 +3,121 @@ require 'spec_helper' describe MattermostSlashCommandsService, :models do it_behaves_like "chat slash commands service" - describe '#configure' do + context 'Mattermost API' do let(:project) { create(:empty_project) } let(:service) { project.build_mattermost_slash_commands_service } let(:user) { create(:user)} - subject do - service.configure(user, team_id: 'abc', - trigger: 'gitlab', url: 'http://trigger.url', - icon_url: 'http://icon.url/icon.png') + before do + Mattermost::Session.base_uri("http://mattermost.example.com") + + allow_any_instance_of(Mattermost::Client).to receive(:with_session). + and_yield(Mattermost::Session.new(nil)) end - context 'the requests succeeds' do - before do - allow_any_instance_of(Mattermost::Command). - to receive(:json_post).and_return('token' => 'token') + describe '#configure' do + subject do + service.configure(user, team_id: 'abc', + trigger: 'gitlab', url: 'http://trigger.url', + icon_url: 'http://icon.url/icon.png') end - it 'saves the service' do - expect { subject }.to change { project.services.count }.by(1) + context 'the requests succeeds' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + with(body: { + team_id: 'abc', + trigger: 'gitlab', + url: 'http://trigger.url', + icon_url: 'http://icon.url/icon.png', + auto_complete: true, + auto_complete_desc: "Perform common operations on: #{project.name_with_namespace}", + auto_complete_hint: '[help]', + description: "Perform common operations on: #{project.name_with_namespace}", + display_name: "GitLab / #{project.name_with_namespace}", + method: 'P', + user_name: 'GitLab' }.to_json). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: { token: 'token' }.to_json + ) + end + + it 'saves the service' do + expect { subject }.to change { project.services.count }.by(1) + end + + it 'saves the token' do + subject + + expect(service.reload.token).to eq('token') + end end - it 'saves the token' do - subject + context 'an error is received' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + id: 'api.command.duplicate_trigger.app_error', + message: 'This trigger word is already in use. Please choose another word.', + detailed_error: '', + request_id: 'obc374man7bx5r3dbc1q5qhf3r', + status_code: 500 + }.to_json + ) + end + + it 'shows error messages' do + succeeded, message = subject - expect(service.reload.token).to eq('token') + expect(succeeded).to be(false) + expect(message).to eq('This trigger word is already in use. Please choose another word.') + end end end - context 'an error is received' do - it 'shows error messages' do - succeeded, message = subject + describe '#list_teams' do + subject do + service.list_teams(user) + end + + context 'the requests succeeds' do + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: ['list'].to_json + ) + end + + it 'returns a list of teams' do + expect(subject).not_to be_empty + end + end + + context 'an error is received' do + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + message: 'Failed to get team list.' + }.to_json + ) + end + + it 'shows error messages' do + teams, message = subject - expect(succeeded).to be(false) - expect(message).to start_with("Failed to open TCP connection to") + expect(teams).to be_empty + expect(message).to eq('Failed to get team list.') + end end end end