Commit 52c4a786 authored by Z.J. van de Weg's avatar Z.J. van de Weg

Improve UX

parent f6247600
......@@ -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
......
......@@ -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
......@@ -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
......@@ -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
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
......@@ -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
......@@ -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
......@@ -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
......
......@@ -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
......@@ -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' })
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment