summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/groups_controller.rb1
-rw-r--r--app/models/chat_team.rb5
-rw-r--r--app/services/groups/create_service.rb4
-rw-r--r--app/workers/mattermost/create_team_worker.rb25
-rw-r--r--lib/mattermost/client.rb2
-rw-r--r--lib/mattermost/team.rb18
-rw-r--r--spec/models/chat_team_spec.rb2
-rw-r--r--spec/services/groups/create_service_spec.rb17
-rw-r--r--spec/workers/mattermost/create_team_worker_spec.rb29
9 files changed, 72 insertions, 31 deletions
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 7ed54479599..63b37fb5eb3 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -143,6 +143,7 @@ class GroupsController < Groups::ApplicationController
:share_with_group_lock,
:visibility_level,
:parent_id
+ :create_chat_team
]
end
diff --git a/app/models/chat_team.rb b/app/models/chat_team.rb
new file mode 100644
index 00000000000..7952141a0d6
--- /dev/null
+++ b/app/models/chat_team.rb
@@ -0,0 +1,5 @@
+class ChatTeam < ActiveRecord::Base
+ validates :team_id, presence: true
+
+ belongs_to :namespace
+end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index 209fd15f481..aabd3c4bd05 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -5,6 +5,8 @@ module Groups
end
def execute
+ create_chat_team = params.delete(:create_chat_team)
+
@group = Group.new(params)
unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
@@ -23,7 +25,7 @@ module Groups
@group.save
@group.add_owner(current_user)
- if params[:create_chat_team] && Gitlab.config.mattermost.enabled
+ if create_chat_team && Gitlab.config.mattermost.enabled
Mattermost::CreateTeamWorker.perform_async(@group.id, current_user.id)
end
diff --git a/app/workers/mattermost/create_team_worker.rb b/app/workers/mattermost/create_team_worker.rb
index 69486569cbf..168bdc7454d 100644
--- a/app/workers/mattermost/create_team_worker.rb
+++ b/app/workers/mattermost/create_team_worker.rb
@@ -3,26 +3,21 @@ module Mattermost
include Sidekiq::Worker
include DedicatedSidekiqQueue
+ sidekiq_options retry: 5
+
+ # Add 5 seconds so the first retry isn't 1 second later
+ sidekiq_retry_in do |count|
+ 5 + 5 ** n
+ end
+
def perform(group_id, current_user_id, options = {})
- @group = Group.find(group_id)
+ group = Group.find(group_id)
current_user = User.find(current_user_id)
- options = team_params.merge(options)
-
# The user that creates the team will be Team Admin
- response = Mattermost::Team.new(current_user).create(options)
-
- ChatTeam.create!(namespace: @group, name: response['name'], team_id: response['id'])
- end
-
- private
+ response = Mattermost::Team.new(current_user).create(group, options)
- def team_params
- {
- name: @group.path[0..59],
- display_name: @group.name[0..59],
- type: @group.public? ? 'O' : 'I' # Open vs Invite-only
- }
+ ChatTeam.create(namespace: group, name: response['name'], team_id: response['id'])
end
end
end
diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb
index e55c0d6ac49..29b10e2afce 100644
--- a/lib/mattermost/client.rb
+++ b/lib/mattermost/client.rb
@@ -26,7 +26,7 @@ module Mattermost
def session_get(path, options = {})
with_session do |session|
- get(session, path, options)
+ get(session, path, options)
end
end
diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb
index 9e80f7f8571..d5648f7167f 100644
--- a/lib/mattermost/team.rb
+++ b/lib/mattermost/team.rb
@@ -5,8 +5,22 @@ module Mattermost
session_get('/api/v3/teams/all')
end
- def create(params)
- session_post('/api/v3/teams/create', body: params.to_json)
+ # Creates a team on the linked Mattermost instance, the team admin will be the
+ # `current_user` passed to the Mattermost::Client instance
+ def create(group, params)
+ session_post('/api/v3/teams/create', body: new_team_params(group, params).to_json)
+ end
+
+ private
+
+ MATTERMOST_TEAM_LENGTH_MAX = 59
+
+ def new_team_params(group, options)
+ {
+ name: group.path[0..MATTERMOST_TEAM_LENGTH_MAX],
+ display_name: group.name[0..MATTERMOST_TEAM_LENGTH_MAX],
+ type: group.public? ? 'O' : 'I' # Open vs Invite-only
+ }.merge(options)
end
end
end
diff --git a/spec/models/chat_team_spec.rb b/spec/models/chat_team_spec.rb
index 37a22f5cd6d..1aab161ec13 100644
--- a/spec/models/chat_team_spec.rb
+++ b/spec/models/chat_team_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe ChatTeam, type: :model do
# Associations
- it { is_expected.to belong_to(:group) }
+ it { is_expected.to belong_to(:namespace) }
# Fields
it { is_expected.to respond_to(:name) }
diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb
index 14717a7455d..235fa5c5188 100644
--- a/spec/services/groups/create_service_spec.rb
+++ b/spec/services/groups/create_service_spec.rb
@@ -4,11 +4,11 @@ describe Groups::CreateService, '#execute', services: true do
let!(:user) { create(:user) }
let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
+ subject { service.execute }
+
describe 'visibility level restrictions' do
let!(:service) { described_class.new(user, group_params) }
- subject { service.execute }
-
context "create groups without restricted visibility level" do
it { is_expected.to be_persisted }
end
@@ -24,8 +24,6 @@ describe Groups::CreateService, '#execute', services: true do
let!(:group) { create(:group) }
let!(:service) { described_class.new(user, group_params.merge(parent_id: group.id)) }
- subject { service.execute }
-
context 'as group owner' do
before { group.add_owner(user) }
@@ -40,4 +38,15 @@ describe Groups::CreateService, '#execute', services: true do
end
end
end
+
+ describe 'creating a mattermost team' do
+ let!(:params) { group_params.merge(create_chat_team: true) }
+ let!(:service) { described_class.new(user, params) }
+
+ it 'queues a background job' do
+ expect(Mattermost::CreateTeamWorker).to receive(:perform_async)
+
+ subject
+ end
+ end
end
diff --git a/spec/workers/mattermost/create_team_worker_spec.rb b/spec/workers/mattermost/create_team_worker_spec.rb
index a6b87967ca2..d3230f535c2 100644
--- a/spec/workers/mattermost/create_team_worker_spec.rb
+++ b/spec/workers/mattermost/create_team_worker_spec.rb
@@ -7,15 +7,30 @@ describe Mattermost::CreateTeamWorker do
describe '.perform' do
subject { described_class.new.perform(group.id, admin.id) }
- before do
- allow_any_instance_of(Mattermost::Team).
- to receive(:create).
- with(name: "path", display_name: "name", type: "O").
- and_return('name' => 'my team', 'id' => 'sjfkdlwkdjfwlkfjwf')
+ context 'succesfull request to mattermost' do
+ before do
+ allow_any_instance_of(Mattermost::Team).
+ to receive(:create).
+ with(group, {}).
+ and_return('name' => 'my team', 'id' => 'sjfkdlwkdjfwlkfjwf')
+ end
+
+ it 'creates a new chat team' do
+ expect { subject }.to change { ChatTeam.count }.from(0).to(1)
+ end
end
- it 'creates a new chat team' do
- expect { subject }.to change { ChatTeam.count }.from(0).to(1)
+ context 'connection trouble' do
+ before do
+ allow_any_instance_of(Mattermost::Team).
+ to receive(:create).
+ with(group, {}).
+ and_raise(Mattermost::ClientError.new('Undefined error'))
+ end
+
+ it 'does not rescue the error' do
+ expect { subject }.to raise_error(Mattermost::ClientError)
+ end
end
end
end