diff options
| -rw-r--r-- | app/controllers/groups_controller.rb | 1 | ||||
| -rw-r--r-- | app/models/chat_team.rb | 5 | ||||
| -rw-r--r-- | app/services/groups/create_service.rb | 4 | ||||
| -rw-r--r-- | app/workers/mattermost/create_team_worker.rb | 25 | ||||
| -rw-r--r-- | lib/mattermost/client.rb | 2 | ||||
| -rw-r--r-- | lib/mattermost/team.rb | 18 | ||||
| -rw-r--r-- | spec/models/chat_team_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/groups/create_service_spec.rb | 17 | ||||
| -rw-r--r-- | spec/workers/mattermost/create_team_worker_spec.rb | 29 |
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 |