Improve API specs

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

changelogs/unreleased/mattermost-slash-auto-config.yml

@@ -0,0 +1,4 @@
+---
+title: Allow to auto-configure Mattermost
+merge_request: 8070
+author:

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

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")
+    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
 
-  subject { described_class.new(user) }
-
   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.create(params)
+    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
+
+      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

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 }]
+    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
+
+      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
+
+      it 'returns a token' do
+        is_expected.to eq(response)
+      end
     end
 
-    subject { described_class.new(user) }
+    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
 
-    before do
-      allow(subject).to receive(:json_get).and_return(response)
-    end
-
-    it 'gets the teams' do
-      expect(subject.all.count).to be(1)
+      it 'raises an error with message' do
+        expect { subject }.to raise_error(Mattermost::Error, 'Cannot list teams.')
+      end
     end
   end
 end

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
 
-        expect(service.reload.token).to eq('token')
+        it 'shows error messages' do
+          succeeded, message = subject
+
+          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
 
-        expect(succeeded).to be(false)
-        expect(message).to start_with("Failed to open TCP connection to")
+      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(teams).to be_empty
+          expect(message).to eq('Failed to get team list.')
+        end
       end
     end
   end