diff options
25 files changed, 446 insertions, 12 deletions
diff --git a/app/assets/javascripts/behaviors/bind_in_out.js b/app/assets/javascripts/behaviors/bind_in_out.js new file mode 100644 index 00000000000..886f127b06b --- /dev/null +++ b/app/assets/javascripts/behaviors/bind_in_out.js @@ -0,0 +1,47 @@ +class BindInOut { + constructor(bindIn, bindOut) { + this.in = bindIn; + this.out = bindOut; + + this.eventWrapper = {}; + this.eventType = /(INPUT|TEXTAREA)/.test(bindIn.tagName) ? 'keyup' : 'change'; + } + + addEvents() { + this.eventWrapper.updateOut = this.updateOut.bind(this); + + this.in.addEventListener(this.eventType, this.eventWrapper.updateOut); + + return this; + } + + updateOut() { + this.out.textContent = this.in.value; + + return this; + } + + removeEvents() { + this.in.removeEventListener(this.eventType, this.eventWrapper.updateOut); + + return this; + } + + static initAll() { + const ins = document.querySelectorAll('*[data-bind-in]'); + + return [].map.call(ins, anIn => BindInOut.init(anIn)); + } + + static init(anIn, anOut) { + const out = anOut || document.querySelector(`*[data-bind-out="${anIn.dataset.bindIn}"]`); + + if (!out) return null; + + const bindInOut = new BindInOut(anIn, out); + + return bindInOut.addEvents().updateOut(); + } +} + +export default BindInOut; diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index a7181904ac9..0726c6c9636 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -21,8 +21,7 @@ // %a.js-toggle-button // %div.js-toggle-content // - $('body').on('click', '.js-toggle-button', function(e) { - e.preventDefault(); + $('body').on('click', '.js-toggle-button', function() { toggleContainer($(this).closest('.js-toggle-container')); }); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index ef5785b5532..31f10f89245 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -35,6 +35,7 @@ /* global Labels */ /* global Shortcuts */ +import BindInOut from './behaviors/bind_in_out'; import GroupsList from './groups_list'; import ProjectsList from './projects_list'; @@ -229,9 +230,14 @@ const UserCallout = require('./user_callout'); new UsersSelect(); break; case 'groups:new': + case 'admin:groups:new': + case 'groups:create': + case 'admin:groups:create': + BindInOut.initAll(); + case 'groups:new': + case 'admin:groups:new': case 'groups:edit': case 'admin:groups:edit': - case 'admin:groups:new': new GroupAvatar(); break; case 'projects:tree:show': diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 798553c16ac..6ac659fd290 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -41,6 +41,7 @@ require('./behaviors/details_behavior'); require('./behaviors/quick_submit'); require('./behaviors/requires_input'); require('./behaviors/toggler_behavior'); +require('./behaviors/bind_in_out'); // blob require('./blob/blob_ci_yaml'); diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss index d377526e655..84d21e48463 100644 --- a/app/assets/stylesheets/pages/groups.scss +++ b/app/assets/stylesheets/pages/groups.scss @@ -73,3 +73,19 @@ } } } + +.mattermost-icon svg { + width: 16px; + height: 16px; + vertical-align: text-bottom; +} + +.mattermost-team-name { + color: $gl-text-color-secondary; +} + +.mattermost-info { + display: block; + color: $gl-text-color-secondary; + margin-top: 10px; +} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 15db5b7762d..4663b6e7fc6 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 @@ -142,7 +148,9 @@ class GroupsController < Groups::ApplicationController :request_access_enabled, :share_with_group_lock, :visibility_level, - :parent_id + :parent_id, + :create_chat_team, + :chat_team_name ] 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/models/group.rb b/app/models/group.rb index 7d23f655225..e1ac7b63c6b 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/models/namespace.rb b/app/models/namespace.rb index 3137dd32f93..d350f1d6770 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -20,6 +20,7 @@ class Namespace < ActiveRecord::Base belongs_to :parent, class_name: "Namespace" has_many :children, class_name: "Namespace", foreign_key: :parent_id + has_one :chat_team, dependent: :destroy validates :owner, presence: true, unless: ->(n) { n.type == "Group" } validates :name, diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index febeb661fb5..c4e9b8fd8e0 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -2,6 +2,7 @@ module Groups class CreateService < Groups::BaseService def initialize(user, params = {}) @current_user, @params = user, params.dup + @chat_team = @params.delete(:create_chat_team) end def execute @@ -20,9 +21,23 @@ module Groups end @group.name ||= @group.path.dup + + if create_chat_team? + response = Mattermost::CreateTeamService.new(@group, current_user).execute + return @group if @group.errors.any? + + @group.build_chat_team(name: response['name'], team_id: response['id']) + end + @group.save @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/mattermost/create_team_service.rb b/app/services/mattermost/create_team_service.rb new file mode 100644 index 00000000000..e3206810f3a --- /dev/null +++ b/app/services/mattermost/create_team_service.rb @@ -0,0 +1,14 @@ +module Mattermost + class CreateTeamService < ::BaseService + def initialize(group, current_user) + @group, @current_user = group, current_user + end + + def execute + # The user that creates the team will be Team Admin + Mattermost::Team.new(current_user).create(@group.mattermost_team_params) + rescue Mattermost::ClientError => e + @group.errors.add(:mattermost_team, e.message) + end + end +end diff --git a/app/views/groups/_create_chat_team.html.haml b/app/views/groups/_create_chat_team.html.haml new file mode 100644 index 00000000000..20de1b4c973 --- /dev/null +++ b/app/views/groups/_create_chat_team.html.haml @@ -0,0 +1,16 @@ +.form-group + = f.label :create_chat_team, class: 'control-label' do + %span.mattermost-icon + = custom_icon('icon_mattermost') + Mattermost + .col-sm-10 + .checkbox.js-toggle-container + = f.label :create_chat_team do + .js-toggle-button= f.check_box(:create_chat_team, { checked: true }, true, false) + Create a Mattermost team for this group + %br + %small.light.js-toggle-content + Mattermost URL: + = Settings.mattermost.host + %span> / + %span{ "data-bind-out" => "create_chat_team" } diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 38d63fd9acc..000c7af2326 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -16,6 +16,8 @@ = render 'shared/visibility_level', f: f, visibility_level: default_group_visibility, can_change_visibility_level: true, form_model: @group + = render 'create_chat_team', f: f if Gitlab.config.mattermost.enabled + .form-group .col-sm-offset-2.col-sm-10 = render 'shared/group_tips' diff --git a/app/views/shared/_group_form.html.haml b/app/views/shared/_group_form.html.haml index 02b7b2447ed..c2d9ac87b20 100644 --- a/app/views/shared/_group_form.html.haml +++ b/app/views/shared/_group_form.html.haml @@ -18,7 +18,8 @@ = f.text_field :path, placeholder: 'open-source', class: 'form-control', autofocus: local_assigns[:autofocus] || false, required: true, pattern: Gitlab::Regex::NAMESPACE_REGEX_STR_JS, - title: 'Please choose a group name with no special characters.' + title: 'Please choose a group name with no special characters.', + "data-bind-in" => "#{'create_chat_team' if Gitlab.config.mattermost.enabled}" - if parent = f.hidden_field :parent_id, value: parent.id diff --git a/app/views/shared/icons/_icon_mattermost.svg b/app/views/shared/icons/_icon_mattermost.svg new file mode 100644 index 00000000000..d1c541523ab --- /dev/null +++ b/app/views/shared/icons/_icon_mattermost.svg @@ -0,0 +1 @@ +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 500 500"><path d="M250.05 34c1.9.04 3.8.11 5.6.2l-29.79 35.51c-.07.01-.15.03-.23.04C149.26 84.1 98.22 146.5 98.22 222.97c0 41.56 23.07 90.5 59.75 119.1 28.61 22.32 64.29 36.9 101.21 36.9 93.4 0 160.15-68.61 160.15-156 0-34.91-15.99-72.77-41.76-100.76l-1.63-47.39c54.45 39.15 89.95 103.02 90.06 175.17v.01c0 119.29-96.7 216-216 216-119.29 0-216-96.71-216-216S130.71 34 250 34h.05zm64.1 20.29c.66-.04 1.32.03 1.96.25 3.01 1 3.85 3.57 3.93 6.45l3.84 146.88c.76 28.66-17.16 68.44-60.39 68.56-30.97.08-63.68-20.83-63.68-60.13.01-14.73 5.61-31.26 19.25-48.11l90.03-111.18c1.15-1.42 3.08-2.58 5.06-2.72z"/></svg> diff --git a/db/migrate/20170120131253_create_chat_teams.rb b/db/migrate/20170120131253_create_chat_teams.rb new file mode 100644 index 00000000000..7995d383986 --- /dev/null +++ b/db/migrate/20170120131253_create_chat_teams.rb @@ -0,0 +1,18 @@ +class CreateChatTeams < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + DOWNTIME_REASON = "Adding a foreign key" + + disable_ddl_transaction! + + def change + create_table :chat_teams do |t| + t.references :namespace, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.string :team_id + t.string :name + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f7dd3e4768..94fd64bc561 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -175,6 +175,16 @@ ActiveRecord::Schema.define(version: 20170306170512) do add_index "chat_names", ["service_id", "team_id", "chat_id"], name: "index_chat_names_on_service_id_and_team_id_and_chat_id", unique: true, using: :btree add_index "chat_names", ["user_id", "service_id"], name: "index_chat_names_on_user_id_and_service_id", unique: true, using: :btree + create_table "chat_teams", force: :cascade do |t| + t.integer "namespace_id", null: false + t.string "team_id" + t.string "name" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "chat_teams", ["namespace_id"], name: "index_chat_teams_on_namespace_id", unique: true, using: :btree + create_table "ci_application_settings", force: :cascade do |t| t.boolean "all_broken_builds" t.boolean "add_pusher" @@ -1338,6 +1348,7 @@ ActiveRecord::Schema.define(version: 20170306170512) 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 "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb index ad6df246091..3d60618006c 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/session.rb b/lib/mattermost/session.rb index 5388966605d..688a79c0441 100644 --- a/lib/mattermost/session.rb +++ b/lib/mattermost/session.rb @@ -153,7 +153,7 @@ module Mattermost yield rescue HTTParty::Error => e raise Mattermost::ConnectionError.new(e.message) - rescue Errno::ECONNREFUSED + rescue Errno::ECONNREFUSED => e raise Mattermost::ConnectionError.new(e.message) end end diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb index afc152aa02e..2cdbbdece16 100644 --- a/lib/mattermost/team.rb +++ b/lib/mattermost/team.rb @@ -1,7 +1,18 @@ module Mattermost class Team < Client + # Returns **all** teams for an admin def all session_get('/api/v3/teams/all').values 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: { + name: name, + display_name: display_name, + type: type + }.to_json) + end end end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index 37b7c20239f..9f173dbc99d 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -43,6 +43,44 @@ feature 'Group', feature: true do expect(page).to have_namespace_error_message end end + + describe 'Mattermost team creation' do + before do + allow(Settings.mattermost).to receive_messages(enabled: mattermost_enabled) + + visit new_group_path + end + + context 'Mattermost enabled' do + let(:mattermost_enabled) { true } + + it 'displays a team creation checkbox' do + expect(page).to have_selector('#group_create_chat_team') + end + + it 'checks the checkbox by default' do + expect(find('#group_create_chat_team')['checked']).to eq(true) + end + + it 'updates the team URL on graph path update', :js do + out_span = find('span[data-bind-out="create_chat_team"]') + + expect(out_span.text).to be_empty + + fill_in('group_path', with: 'test-group') + + expect(out_span.text).to eq('test-group') + end + end + + context 'Mattermost disabled' do + let(:mattermost_enabled) { false } + + it 'doesnt show a team creation checkbox if Mattermost not enabled' do + expect(page).not_to have_selector('#group_create_chat_team') + end + end + end end describe 'create a nested group' do diff --git a/spec/javascripts/behaviors/bind_in_out_spec.js b/spec/javascripts/behaviors/bind_in_out_spec.js new file mode 100644 index 00000000000..dd9ab33289f --- /dev/null +++ b/spec/javascripts/behaviors/bind_in_out_spec.js @@ -0,0 +1,189 @@ +import BindInOut from '~/behaviors/bind_in_out'; +import ClassSpecHelper from '../helpers/class_spec_helper'; + +describe('BindInOut', function () { + describe('.constructor', function () { + beforeEach(function () { + this.in = {}; + this.out = {}; + + this.bindInOut = new BindInOut(this.in, this.out); + }); + + it('should set .in', function () { + expect(this.bindInOut.in).toBe(this.in); + }); + + it('should set .out', function () { + expect(this.bindInOut.out).toBe(this.out); + }); + + it('should set .eventWrapper', function () { + expect(this.bindInOut.eventWrapper).toEqual({}); + }); + + describe('if .in is an input', function () { + beforeEach(function () { + this.bindInOut = new BindInOut({ tagName: 'INPUT' }); + }); + + it('should set .eventType to keyup ', function () { + expect(this.bindInOut.eventType).toEqual('keyup'); + }); + }); + + describe('if .in is a textarea', function () { + beforeEach(function () { + this.bindInOut = new BindInOut({ tagName: 'TEXTAREA' }); + }); + + it('should set .eventType to keyup ', function () { + expect(this.bindInOut.eventType).toEqual('keyup'); + }); + }); + + describe('if .in is not an input or textarea', function () { + beforeEach(function () { + this.bindInOut = new BindInOut({ tagName: 'SELECT' }); + }); + + it('should set .eventType to change ', function () { + expect(this.bindInOut.eventType).toEqual('change'); + }); + }); + }); + + describe('.addEvents', function () { + beforeEach(function () { + this.in = jasmine.createSpyObj('in', ['addEventListener']); + + this.bindInOut = new BindInOut(this.in); + + this.addEvents = this.bindInOut.addEvents(); + }); + + it('should set .eventWrapper.updateOut', function () { + expect(this.bindInOut.eventWrapper.updateOut).toEqual(jasmine.any(Function)); + }); + + it('should call .addEventListener', function () { + expect(this.in.addEventListener) + .toHaveBeenCalledWith( + this.bindInOut.eventType, + this.bindInOut.eventWrapper.updateOut, + ); + }); + + it('should return the instance', function () { + expect(this.addEvents).toBe(this.bindInOut); + }); + }); + + describe('.updateOut', function () { + beforeEach(function () { + this.in = { value: 'the-value' }; + this.out = { textContent: 'not-the-value' }; + + this.bindInOut = new BindInOut(this.in, this.out); + + this.updateOut = this.bindInOut.updateOut(); + }); + + it('should set .out.textContent to .in.value', function () { + expect(this.out.textContent).toBe(this.in.value); + }); + + it('should return the instance', function () { + expect(this.updateOut).toBe(this.bindInOut); + }); + }); + + describe('.removeEvents', function () { + beforeEach(function () { + this.in = jasmine.createSpyObj('in', ['removeEventListener']); + this.updateOut = () => {}; + + this.bindInOut = new BindInOut(this.in); + this.bindInOut.eventWrapper.updateOut = this.updateOut; + + this.removeEvents = this.bindInOut.removeEvents(); + }); + + it('should call .removeEventListener', function () { + expect(this.in.removeEventListener) + .toHaveBeenCalledWith( + this.bindInOut.eventType, + this.updateOut, + ); + }); + + it('should return the instance', function () { + expect(this.removeEvents).toBe(this.bindInOut); + }); + }); + + describe('.initAll', function () { + beforeEach(function () { + this.ins = [0, 1, 2]; + this.instances = []; + + spyOn(document, 'querySelectorAll').and.returnValue(this.ins); + spyOn(Array.prototype, 'map').and.callThrough(); + spyOn(BindInOut, 'init'); + + this.initAll = BindInOut.initAll(); + }); + + ClassSpecHelper.itShouldBeAStaticMethod(BindInOut, 'initAll'); + + it('should call .querySelectorAll', function () { + expect(document.querySelectorAll).toHaveBeenCalledWith('*[data-bind-in]'); + }); + + it('should call .map', function () { + expect(Array.prototype.map).toHaveBeenCalledWith(jasmine.any(Function)); + }); + + it('should call .init for each element', function () { + expect(BindInOut.init.calls.count()).toEqual(3); + }); + + it('should return an array of instances', function () { + expect(this.initAll).toEqual(jasmine.any(Array)); + }); + }); + + describe('.init', function () { + beforeEach(function () { + spyOn(BindInOut.prototype, 'addEvents').and.callFake(function () { return this; }); + spyOn(BindInOut.prototype, 'updateOut').and.callFake(function () { return this; }); + + this.init = BindInOut.init({}, {}); + }); + + ClassSpecHelper.itShouldBeAStaticMethod(BindInOut, 'init'); + + it('should call .addEvents', function () { + expect(BindInOut.prototype.addEvents).toHaveBeenCalled(); + }); + + it('should call .updateOut', function () { + expect(BindInOut.prototype.updateOut).toHaveBeenCalled(); + }); + + describe('if no anOut is provided', function () { + beforeEach(function () { + this.anIn = { dataset: { bindIn: 'the-data-bind-in' } }; + + spyOn(document, 'querySelector'); + + BindInOut.init(this.anIn); + }); + + it('should call .querySelector', function () { + expect(document.querySelector) + .toHaveBeenCalledWith(`*[data-bind-out="${this.anIn.dataset.bindIn}"]`); + }); + }); + }); +}); diff --git a/spec/models/chat_team_spec.rb b/spec/models/chat_team_spec.rb new file mode 100644 index 00000000000..1aab161ec13 --- /dev/null +++ b/spec/models/chat_team_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' + +describe ChatTeam, type: :model do + # Associations + it { is_expected.to belong_to(:namespace) } + + # Fields + it { is_expected.to respond_to(:name) } + it { is_expected.to respond_to(:team_id) } +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a4e6eb4e3a6..8cfc2085458 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -13,6 +13,7 @@ describe Group, models: true do it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:labels).class_name('GroupLabel') } + it { is_expected.to have_one(:chat_team) } describe '#members & #requesters' do let(:requester) { create(:user) } diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index 14717a7455d..ec89b540e6a 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,20 @@ 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) } + + before do + Settings.mattermost['enabled'] = true + 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' }) + + expect { subject }.to change { ChatTeam.count }.from(0).to(1) + end + end end |
