diff options
| -rw-r--r-- | app/controllers/groups_controller.rb | 8 | ||||
| -rw-r--r-- | app/models/group.rb | 10 | ||||
| -rw-r--r-- | app/services/groups/base_service.rb | 6 | ||||
| -rw-r--r-- | app/services/groups/create_service.rb | 12 | ||||
| -rw-r--r-- | app/services/groups/update_service.rb | 14 | ||||
| -rw-r--r-- | app/services/mattermost/create_team_service.rb | 5 | ||||
| -rw-r--r-- | db/migrate/20170120131253_create_chat_teams.rb | 4 | ||||
| -rw-r--r-- | db/schema.rb | 14 | ||||
| -rw-r--r-- | lib/mattermost/team.rb | 20 | ||||
| -rw-r--r-- | spec/services/groups/create_service_spec.rb | 6 |
10 files changed, 47 insertions, 52 deletions
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 5b1898b0ee1..5a6ba3f2054 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -32,7 +32,13 @@ class GroupsController < Groups::ApplicationController @group = Groups::CreateService.new(current_user, group_params).execute if @group.persisted? - redirect_to @group, notice: "Group '#{@group.name}' was successfully created." + notice = if @group.chat_team.present? + "Group '#{@group.name}' and its Mattermost team were successfully created." + else + "Group '#{@group.name}' was successfully created." + end + + redirect_to @group, notice: notice else render action: "new" end diff --git a/app/models/group.rb b/app/models/group.rb index 240a17f1dc1..ff340859cb7 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -212,4 +212,14 @@ class Group < Namespace def users_with_parents User.where(id: members_with_parents.select(:user_id)) end + + def mattermost_team_params + max_length = 59 + + { + name: path[0..max_length], + display_name: name[0..max_length], + type: public? ? 'O' : 'I' # Open vs Invite-only + } + end end diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 47476aaa05e..a8fa098246a 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -5,11 +5,5 @@ module Groups def initialize(group, user, params = {}) @group, @current_user, @params = group, user, params.dup end - - private - - def create_chat_team? - @chat_team == true && Gitlab.config.mattermost.enabled - end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 4ed2cb7c8af..6ef78c3e677 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -23,7 +23,11 @@ module Groups @group.name ||= @group.path.dup if create_chat_team? - Mattermost::CreateTeamService.new(@group, current_user).execute + begin + response = Mattermost::CreateTeamService.new(@group, current_user).execute + + @group.build_chat_team(name: response['name'], team_id: response['id']) + end return @group if @group.errors.any? end @@ -32,5 +36,11 @@ module Groups @group.add_owner(current_user) @group end + + private + + def create_chat_team? + Gitlab.config.mattermost.enabled && @chat_team && group.chat_team.nil? + end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index e8ad6d41ed4..4e878ec556a 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -1,8 +1,6 @@ module Groups class UpdateService < Groups::BaseService def execute - @chat_team = params.delete(:create_chat_team) - # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] if new_visibility && new_visibility.to_i != group.visibility_level @@ -16,12 +14,6 @@ module Groups group.assign_attributes(params) - if create_chat_team? - Mattermost::CreateTeamService.new(group, current_user).execute - - return group if group.errors.any? - end - begin group.save rescue Gitlab::UpdatePathError => e @@ -30,11 +22,5 @@ module Groups false end end - - private - - def create_chat_team? - super && group.chat_team.nil? - end end end diff --git a/app/services/mattermost/create_team_service.rb b/app/services/mattermost/create_team_service.rb index 199d15aee92..e3206810f3a 100644 --- a/app/services/mattermost/create_team_service.rb +++ b/app/services/mattermost/create_team_service.rb @@ -6,10 +6,9 @@ module Mattermost def execute # The user that creates the team will be Team Admin - response = Mattermost::Team.new(current_user).create(@group) - @group.build_chat_team(name: response['name'], team_id: response['id']) + Mattermost::Team.new(current_user).create(@group.mattermost_team_params) rescue Mattermost::ClientError => e - @group.errors.add(:chat_team, e.message) + @group.errors.add(:mattermost_team, e.message) end end end diff --git a/db/migrate/20170120131253_create_chat_teams.rb b/db/migrate/20170120131253_create_chat_teams.rb index 2d9341d235f..699226d60c9 100644 --- a/db/migrate/20170120131253_create_chat_teams.rb +++ b/db/migrate/20170120131253_create_chat_teams.rb @@ -4,6 +4,8 @@ class CreateChatTeams < ActiveRecord::Migration DOWNTIME = true DOWNTIME_REASON = "Adding a foreign key" + disable_ddl_transaction! + def change create_table :chat_teams do |t| t.integer :namespace_id, index: true @@ -13,6 +15,6 @@ class CreateChatTeams < ActiveRecord::Migration t.timestamps null: false end - add_concurrent_foreign_key :chat_teams, :namespaces, on_delete: :cascade + add_concurrent_foreign_key :chat_teams, :namespaces, column: :namespace_id end end diff --git a/db/schema.rb b/db/schema.rb index 39c767a5595..34aa12814c0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170216141440) do +ActiveRecord::Schema.define(version: 20170217151947) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -61,7 +61,6 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.boolean "shared_runners_enabled", default: true, null: false t.integer "max_artifacts_size", default: 100, null: false t.string "runners_registration_token" - t.integer "max_pages_size", default: 100, null: false t.boolean "require_two_factor_authentication", default: false t.integer "two_factor_grace_period", default: 48 t.boolean "metrics_enabled", default: false @@ -110,7 +109,9 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.boolean "html_emails_enabled", default: true t.string "plantuml_url" t.boolean "plantuml_enabled" + t.integer "max_pages_size", default: 100, null: false t.integer "terminal_max_session_time", default: 0, null: false + t.string "default_artifacts_expire_in", default: "0", null: false end create_table "audit_events", force: :cascade do |t| @@ -698,7 +699,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.integer "updated_by_id" t.text "merge_error" t.text "merge_params" - t.boolean "merge_when_build_succeeds", default: false, null: false + t.boolean "merge_when_pipeline_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" @@ -765,8 +766,8 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.integer "visibility_level", default: 20, null: false t.boolean "request_access_enabled", default: false, null: false t.datetime "deleted_at" - t.text "description_html" t.boolean "lfs_enabled" + t.text "description_html" t.integer "parent_id" end @@ -981,7 +982,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.boolean "last_repository_check_failed" t.datetime "last_repository_check_at" t.boolean "container_registry_enabled" - t.boolean "only_allow_merge_if_build_succeeds", default: false, null: false + t.boolean "only_allow_merge_if_pipeline_succeeds", default: false, null: false t.boolean "has_external_issue_tracker" t.string "repository_storage", default: "default", null: false t.boolean "request_access_enabled", default: false, null: false @@ -1291,6 +1292,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.string "organization" t.boolean "authorized_projects_populated" t.boolean "notified_of_own_activity", default: false, null: false + t.boolean "ghost" end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree @@ -1341,7 +1343,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "boards", "projects" - add_foreign_key "chat_teams", "namespaces", on_delete: :cascade + add_foreign_key "chat_teams", "namespaces", name: "fk_3b543909cb", on_delete: :cascade add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb index 52486fd1a54..69b0a3edf6f 100644 --- a/lib/mattermost/team.rb +++ b/lib/mattermost/team.rb @@ -7,20 +7,12 @@ module Mattermost # 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) - session_post('/api/v3/teams/create', body: new_team_params(group).to_json) - end - - private - - MATTERMOST_TEAM_LENGTH_MAX = 59 - - def new_team_params(group) - { - 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 - } + def create(name:, display_name:, type:) + session_post('/api/v3/teams/create', body: { + name: name, + display_name: display_name, + type: type + }.to_json) end end end diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index b11483fb407..ec89b540e6a 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -47,12 +47,6 @@ describe Groups::CreateService, '#execute', services: true do Settings.mattermost['enabled'] = true end - it 'triggers the service' do - expect_any_instance_of(Mattermost::CreateTeamService).to receive(:execute) - - subject - end - it 'create the chat team with the group' do allow_any_instance_of(Mattermost::Team).to receive(:create) .and_return({ 'name' => 'tanuki', 'id' => 'lskdjfwlekfjsdifjj' }) |