Merge branch 'mattermost-api-v4' into 'master'

Updated Mattermost integration to use Mattermost API v4

Closes #41631

See merge request gitlab-org/gitlab-ce!19043

app/views/projects/mattermosts/_team_selection.html.haml

@@ -13,9 +13,9 @@
   = f.hidden_field(:team_id, value: selected_id, required: true) if @teams.one?
   .form-text.text-muted
     - if @teams.one?
-      This is the only available team.
+      This is the only available team that you are a member of.
     - else
-      The list shows all available teams.
+      The list shows all available teams that you are a member of.
     To create a team,
     = link_to "#{Gitlab.config.mattermost.host}/create_team" do
       use Mattermost's interface

changelogs/unreleased/mattermost-api-v4.yml

+---
+title: Updated Mattermost integration to use API v4 and only allow creation of Mattermost slash commands in the current user's teams
+merge_request: 19043
+author: Harrison Healey
+type: changed

lib/mattermost/command.rb

 module Mattermost
   class Command < Client
     def create(params)
-      response = session_post("/api/v3/teams/#{params[:team_id]}/commands/create",
+      response = session_post('/api/v4/commands',
         body: params.to_json)
 
       response['token']

lib/mattermost/session.rb

@@ -112,7 +112,7 @@ module Mattermost
     end
 
     def destroy
-      post('/api/v3/users/logout')
+      post('/api/v4/users/logout')
     end
 
     def oauth_uri
@@ -120,7 +120,7 @@ module Mattermost
 
       @oauth_uri = nil
 
-      response = get("/api/v3/oauth/gitlab/login", follow_redirects: false)
+      response = get('/oauth/gitlab/login', follow_redirects: false, format: 'text/html')
       return unless (300...400) === response.code
 
       redirect_uri = response.headers['location']

lib/mattermost/team.rb

 module Mattermost
   class Team < Client
-    # Returns **all** teams for an admin
+    # Returns all teams that the current user is a member of
     def all
-      session_get('/api/v3/teams/all').values
+      session_get("/api/v4/users/me/teams")
     end
 
     # Creates a team on the linked Mattermost instance, the team admin will be the
     # `current_user` passed to the Mattermost::Client instance
     def create(name:, display_name:, type:)
-      session_post('/api/v3/teams/create', body: {
+      session_post('/api/v4/teams', body: {
         name: name,
         display_name: display_name,
         type: type

spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb

@@ -64,7 +64,7 @@ feature 'Setup Mattermost slash commands', :js do
       click_link 'Add to Mattermost'
 
       expect(page).to have_content('The team where the slash commands will be used in')
-      expect(page).to have_content('This is the only available team.')
+      expect(page).to have_content('This is the only available team that you are a member of.')
     end
 
     it 'shows a disabled prefilled select if user is a member of 1 team' do
@@ -94,7 +94,7 @@ feature 'Setup Mattermost slash commands', :js do
       click_link 'Add to Mattermost'
 
       expect(page).to have_content('Select the team where the slash commands will be used in')
-      expect(page).to have_content('The list shows all available teams.')
+      expect(page).to have_content('The list shows all available teams that you are a member of.')
     end
 
     it 'shows a select with team options user is a member of multiple teams' do

spec/lib/mattermost/command_spec.rb

@@ -21,13 +21,13 @@ describe Mattermost::Command do
 
     context 'for valid trigger word' do
       before do
-        stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create')
+        stub_request(:post, 'http://mattermost.example.com/api/v4/commands')
           .with(body: {
             team_id: 'abc',
             trigger: 'gitlab'
           }.to_json)
           .to_return(
-            status: 200,
+            status: 201,
             headers: { 'Content-Type' => 'application/json' },
             body: { token: 'token' }.to_json
           )
@@ -40,16 +40,16 @@ describe Mattermost::Command do
 
     context 'for error message' do
       before do
-        stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create')
+        stub_request(:post, 'http://mattermost.example.com/api/v4/commands')
           .to_return(
-            status: 500,
+            status: 400,
             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
+              status_code: 400
             }.to_json
           )
       end

spec/lib/mattermost/session_spec.rb

@@ -22,8 +22,8 @@ describe Mattermost::Session, type: :request do
     let(:location) { 'http://location.tld' }
     let(:cookie_header) {'MMOAUTH=taskik8az7rq8k6rkpuas7htia; Path=/;'}
     let!(:stub) do
-      WebMock.stub_request(:get, "#{mattermost_url}/api/v3/oauth/gitlab/login")
-        .to_return(headers: { 'location' => location, 'Set-Cookie' => cookie_header }, status: 307)
+      WebMock.stub_request(:get, "#{mattermost_url}/oauth/gitlab/login")
+        .to_return(headers: { 'location' => location, 'Set-Cookie' => cookie_header }, status: 302)
     end
 
     context 'without oauth uri' do
@@ -76,7 +76,7 @@ describe Mattermost::Session, type: :request do
               end
             end
 
-          WebMock.stub_request(:post, "#{mattermost_url}/api/v3/users/logout")
+          WebMock.stub_request(:post, "#{mattermost_url}/api/v4/users/logout")
             .to_return(headers: { Authorization: 'token thisworksnow' }, status: 200)
         end
 

spec/lib/mattermost/team_spec.rb

@@ -12,26 +12,28 @@ describe Mattermost::Team do
   describe '#all' do
     subject { described_class.new(nil).all }
 
+    let(:test_team) 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
+
     context 'for valid request' do
-      let(:response) do
-        { "xiyro8huptfhdndadpz8r3wnbo" => {
-          "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
+      let(:response) { [test_team] }
 
       before do
-        stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all')
+        stub_request(:get, 'http://mattermost.example.com/api/v4/users/me/teams')
           .to_return(
             status: 200,
             headers: { 'Content-Type' => 'application/json' },
@@ -39,14 +41,14 @@ describe Mattermost::Team do
           )
       end
 
-      it 'returns a token' do
-        is_expected.to eq(response.values)
+      it 'returns teams' do
+        is_expected.to eq(response)
       end
     end
 
     context 'for error message' do
       before do
-        stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all')
+        stub_request(:get, 'http://mattermost.example.com/api/v4/users/me/teams')
           .to_return(
             status: 500,
             headers: { 'Content-Type' => 'application/json' },
@@ -89,9 +91,9 @@ describe Mattermost::Team do
       end
 
       before do
-        stub_request(:post, "http://mattermost.example.com/api/v3/teams/create")
+        stub_request(:post, "http://mattermost.example.com/api/v4/teams")
           .to_return(
-            status: 200,
+            status: 201,
             body: response.to_json,
             headers: { 'Content-Type' => 'application/json' }
           )
@@ -104,7 +106,7 @@ describe Mattermost::Team do
 
     context 'for existing team' do
       before do
-        stub_request(:post, 'http://mattermost.example.com/api/v3/teams/create')
+        stub_request(:post, 'http://mattermost.example.com/api/v4/teams')
           .to_return(
             status: 400,
             headers: { 'Content-Type' => 'application/json' },

spec/models/project_services/mattermost_slash_commands_service_spec.rb

@@ -25,7 +25,7 @@ describe MattermostSlashCommandsService do
 
       context 'the requests succeeds' do
         before do
-          stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create')
+          stub_request(:post, 'http://mattermost.example.com/api/v4/commands')
             .with(body: {
               team_id: 'abc',
               trigger: 'gitlab',
@@ -59,7 +59,7 @@ describe MattermostSlashCommandsService do
 
       context 'an error is received' do
         before do
-          stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create')
+          stub_request(:post, 'http://mattermost.example.com/api/v4/commands')
             .to_return(
               status: 500,
               headers: { 'Content-Type' => 'application/json' },
@@ -89,11 +89,11 @@ describe MattermostSlashCommandsService do
 
       context 'the requests succeeds' do
         before do
-          stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all')
+          stub_request(:get, 'http://mattermost.example.com/api/v4/users/me/teams')
             .to_return(
               status: 200,
               headers: { 'Content-Type' => 'application/json' },
-              body: { 'list' => true }.to_json
+              body: [{ id: 'test_team_id' }].to_json
             )
         end
 
@@ -104,7 +104,7 @@ describe MattermostSlashCommandsService do
 
       context 'an error is received' do
         before do
-          stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all')
+          stub_request(:get, 'http://mattermost.example.com/api/v4/users/me/teams')
             .to_return(
               status: 500,
               headers: { 'Content-Type' => 'application/json' },