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