diff options
author | Imre Farkas <ifarkas@gitlab.com> | 2018-09-02 09:08:55 +0200 |
---|---|---|
committer | Imre Farkas <ifarkas@gitlab.com> | 2018-09-08 13:08:13 +0200 |
commit | 6b6b928c3465746f73be989a928875c089332da6 (patch) | |
tree | bceb8b05742cfb7df0e8b8dd806e37c30cd7fcb3 | |
parent | 623ef392e562a7d8d6db2599227c89f0a0799037 (diff) | |
download | gitlab-ce-if-33054-share_groups_with_groups.tar.gz |
Share groups with groupsif-33054-share_groups_with_groups
-rw-r--r-- | app/controllers/groups/group_links_controller.rb | 19 | ||||
-rw-r--r-- | app/models/group.rb | 2 | ||||
-rw-r--r-- | app/models/group_group_link.rb | 22 | ||||
-rw-r--r-- | app/services/groups/group_links/create_service.rb | 18 | ||||
-rw-r--r-- | app/views/groups/group_members/_new_shared_group.html.haml | 21 | ||||
-rw-r--r-- | app/views/groups/group_members/index.html.haml | 81 | ||||
-rw-r--r-- | config/routes/group.rb | 2 | ||||
-rw-r--r-- | db/migrate/20180902070406_create_group_group_links.rb | 41 | ||||
-rw-r--r-- | db/schema.rb | 15 | ||||
-rw-r--r-- | spec/controllers/groups/group_links_controller_spec.rb | 40 | ||||
-rw-r--r-- | spec/factories/group_group_links.rb | 8 | ||||
-rw-r--r-- | spec/models/group_group_link_spec.rb | 27 | ||||
-rw-r--r-- | spec/services/groups/group_links/create_service_spec.rb | 22 |
13 files changed, 286 insertions, 32 deletions
diff --git a/app/controllers/groups/group_links_controller.rb b/app/controllers/groups/group_links_controller.rb new file mode 100644 index 00000000000..b12c24e347c --- /dev/null +++ b/app/controllers/groups/group_links_controller.rb @@ -0,0 +1,19 @@ +class Groups::GroupLinksController < Groups::ApplicationController + # before_action :authorize_admin_project! + # before_action :authorize_admin_project_member!, only: [:update] + before_action :group + + def create + shared_group = Group.find(params[:shared_group_id]) if params[:shared_group_id].present? + + Groups::GroupLinks::CreateService.new(group, current_user, group_link_create_params).execute(shared_group) + + redirect_to group_group_members_path(group) + end + + protected + + def group_link_create_params + params.permit(:shared_group_access, :expires_at) + end +end diff --git a/app/models/group.rb b/app/models/group.rb index 106a1f4a94c..df42a569350 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -27,6 +27,8 @@ class Group < Namespace has_many :members_and_requesters, as: :source, class_name: 'GroupMember' has_many :milestones + has_many :group_group_links, foreign_key: :shared_with_group_id + has_many :shared_groups, through: :group_group_links, source: :shared_group has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :shared_projects, through: :project_group_links, source: :project diff --git a/app/models/group_group_link.rb b/app/models/group_group_link.rb new file mode 100644 index 00000000000..8f4deea9beb --- /dev/null +++ b/app/models/group_group_link.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class GroupGroupLink < ActiveRecord::Base + include Expirable + + belongs_to :shared_group, class_name: 'Group', foreign_key: :shared_group_id + belongs_to :shared_with_group, class_name: 'Group', foreign_key: :shared_with_group_id + + validates :shared_group, presence: true + validates :shared_group_id, uniqueness: { scope: [:shared_with_group_id], message: 'already shared with this group' } + validates :shared_with_group, presence: true + validates :group_access, presence: true + validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true + + def self.access_options + Gitlab::Access.options + end + + def self.default_access + Gitlab::Access::DEVELOPER + end +end diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb new file mode 100644 index 00000000000..602530e9de2 --- /dev/null +++ b/app/services/groups/group_links/create_service.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Groups + module GroupLinks + class CreateService < BaseService + def execute(shared_group) + return false unless shared_group + + group.group_group_links.create( + shared_group: shared_group, + shared_with_group: group, + group_access: params[:shared_group_access], + expires_at: params[:expires_at] + ) + end + end + end +end diff --git a/app/views/groups/group_members/_new_shared_group.html.haml b/app/views/groups/group_members/_new_shared_group.html.haml new file mode 100644 index 00000000000..fc81b1ef991 --- /dev/null +++ b/app/views/groups/group_members/_new_shared_group.html.haml @@ -0,0 +1,21 @@ +.row + .col-sm-12 + = form_tag group_group_links_path(@group), class: 'js-requires-input', method: :post do + .form-group + = label_tag :shared_group_id, 'Select a group to share with', class: 'label-bold' + -#= groups_select_tag(:link_group_id, data: { skip_groups: [7] }, class: "input-clamp", required: true) + = text_field_tag :shared_group_id, nil, class: 'form-control', id: 'foo' + .form-group + = label_tag :shared_group_access, 'Max access level', class: 'label-bold' + .select-wrapper + = select_tag :share_group_access, options_for_select(GroupGroupLink.access_options, GroupGroupLink.default_access), class: 'form-control select-control' + = icon('chevron-down') + .form-text.text-muted.append-bottom-10 + = link_to 'Read more', help_page_path('user/permissions'), class: 'vlink' + about role permissions + .form-group + = label_tag :expires_at, 'Access expiration date', class: 'label-bold' + .clearable-input + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date-groups', placeholder: 'Expiration date', id: 'expires_at_groups' + %i.clear-icon.js-clear-input + = submit_tag 'Share', class: 'btn btn-create' diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 13d584f5f1d..f594dfc0e6b 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -1,37 +1,56 @@ - page_title "Members" - can_manage_members = can?(current_user, :admin_group_member, @group) -.project-members-page.prepend-top-default - %h4 - Members - %hr - - if can_manage_members - .project-members-new.append-bottom-default - %p.clearfix - Add new member to +.row.prepend-top-default + .col-lg-12 + %h4 + Group members + - if can_manage_members + %p + You can add a new member to %strong= @group.name - = render "new_group_member" + or share it with another group. + - else + %p + Members can be added by project + %i Maintainers + or + %i Owners + .light + - if can_manage_members + %ul.nav-links.nav.nav-tabs.gitlab-tabs{ role: 'tablist' } + %li.nav-tab{ role: 'presentation' } + %a.nav-link.active{ href: '#add-member-pane', id: 'add-member-tab', data: { toggle: 'tab' }, role: 'tab' } Add member + - #if @project.allowed_to_share_with_group? + %li.nav-tab{ role: 'presentation' } + %a.nav-link{ href: '#share-with-group-pane', id: 'share-with-group-tab', data: { toggle: 'tab' }, role: 'tab' } Share with group - = render 'shared/members/requests', membership_source: @group, requesters: @requesters + .tab-content.gitlab-tab-content + .tab-pane.active{ id: 'add-member-pane', role: 'tabpanel' } + = render 'groups/group_members/new_group_member', tab_title: 'Add member' + .tab-pane{ id: 'share-with-group-pane', role: 'tabpanel' } + = render 'groups/group_members/new_shared_group', tab_title: 'Share with group' - .clearfix - %h5.member.existing-title - Existing members - .card - .card-header.flex-project-members-panel - %span.flex-project-title - Members with access to - %strong= @group.name - %span.badge= @members.total_count - = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form flex-project-members-form' do - .form-group - .position-relative.append-right-8 - = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } - %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" } - = icon("search") - - if can_manage_members - = render 'shared/members/filter_2fa_dropdown' - = render 'shared/members/sort_dropdown' - %ul.content-list.members-list - = render partial: 'shared/members/member', collection: @members, as: :member - = paginate @members, theme: 'gitlab' + = render 'shared/members/requests', membership_source: @group, requesters: @requesters + + .clearfix + %h5.member.existing-title + Existing members + .card + .card-header.flex-project-members-panel + %span.flex-project-title + Members with access to + %strong= @group.name + %span.badge= @members.total_count + = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form flex-project-members-form' do + .form-group + .position-relative.append-right-8 + = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } + %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" } + = icon("search") + - if can_manage_members + = render 'shared/members/filter_2fa_dropdown' + = render 'shared/members/sort_dropdown' + %ul.content-list.members-list + = render partial: 'shared/members/member', collection: @members, as: :member + = paginate @members, theme: 'gitlab' diff --git a/config/routes/group.rb b/config/routes/group.rb index 343865cc50c..ef975adc73e 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -51,6 +51,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do delete :leave, on: :collection end + resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ } + resources :uploads, only: [:create] do collection do get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} } diff --git a/db/migrate/20180902070406_create_group_group_links.rb b/db/migrate/20180902070406_create_group_group_links.rb new file mode 100644 index 00000000000..ad559e2fd2b --- /dev/null +++ b/db/migrate/20180902070406_create_group_group_links.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class CreateGroupGroupLinks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + create_table :group_group_links do |t| + t.integer :shared_group_id + t.integer :shared_with_group_id + t.integer :group_access, default: 30, null: false + t.date :expires_at + end + + add_concurrent_foreign_key :group_group_links, + :namespaces, + column: :shared_group_id, + on_delete: :cascade + add_concurrent_index :group_group_links, :shared_group_id + change_column_null :group_group_links, :shared_group_id, false + + add_concurrent_foreign_key :group_group_links, + :namespaces, + column: :shared_with_group_id, + on_delete: :cascade + add_concurrent_index :group_group_links, :shared_with_group_id + change_column_null :group_group_links, :shared_with_group_id, false + + add_concurrent_index :group_group_links, + [:shared_group_id, :shared_with_group_id], + unique: true, + name: 'index_group_group_links_on_shared_group_and_shared_with_group' + end + + def down + drop_table :group_group_links + end +end diff --git a/db/schema.rb b/db/schema.rb index 56c7265119d..bbe921390e7 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: 20180826111825) do +ActiveRecord::Schema.define(version: 20180902070406) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -949,6 +949,17 @@ ActiveRecord::Schema.define(version: 20180826111825) do add_index "group_custom_attributes", ["group_id", "key"], name: "index_group_custom_attributes_on_group_id_and_key", unique: true, using: :btree add_index "group_custom_attributes", ["key", "value"], name: "index_group_custom_attributes_on_key_and_value", using: :btree + create_table "group_group_links", force: :cascade do |t| + t.integer "shared_group_id", null: false + t.integer "shared_with_group_id", null: false + t.integer "group_access", default: 30, null: false + t.date "expires_at" + end + + add_index "group_group_links", ["shared_group_id", "shared_with_group_id"], name: "index_group_group_links_on_shared_group_and_shared_with_group", unique: true, using: :btree + add_index "group_group_links", ["shared_group_id"], name: "index_group_group_links_on_shared_group_id", using: :btree + add_index "group_group_links", ["shared_with_group_id"], name: "index_group_group_links_on_shared_with_group_id", using: :btree + create_table "identities", force: :cascade do |t| t.string "extern_uid" t.string "provider" @@ -2318,6 +2329,8 @@ ActiveRecord::Schema.define(version: 20180826111825) do add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify add_foreign_key "gpg_signatures", "projects", on_delete: :cascade add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade + add_foreign_key "group_group_links", "namespaces", column: "shared_group_id", name: "fk_d3a0488427", on_delete: :cascade + add_foreign_key "group_group_links", "namespaces", column: "shared_with_group_id", name: "fk_2b2353ca49", on_delete: :cascade add_foreign_key "import_export_uploads", "projects", on_delete: :cascade add_foreign_key "internal_ids", "namespaces", name: "fk_162941d509", on_delete: :cascade add_foreign_key "internal_ids", "projects", on_delete: :cascade diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb new file mode 100644 index 00000000000..ccef6a8333d --- /dev/null +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Groups::GroupLinksController do + let(:group) { create(:group, :public) } + # TODO(ifarkas): why does it need to be public? + let(:shared_group) { create(:group, :public) } + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#create' do + subject do + post(:create, + group_id: group, + shared_group_id: shared_group.id, + shared_group_access: GroupGroupLink.default_access) + end + + context 'when user has access to group he want to link another group to' do + before do + group.add_developer(user) + shared_group.add_developer(user) + end + + it 'links group with selected group' do + subject + + expect(group.shared_groups).to include shared_group + end + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(group))) + end + end + end +end diff --git a/spec/factories/group_group_links.rb b/spec/factories/group_group_links.rb new file mode 100644 index 00000000000..ffa201f891e --- /dev/null +++ b/spec/factories/group_group_links.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :group_group_link do + shared_group { group } + shared_with_group { group } + access_level { GroupMember::DEVELOPER } + expires_at nil + end +end diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb new file mode 100644 index 00000000000..7b8bd9895f2 --- /dev/null +++ b/spec/models/group_group_link_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe GroupGroupLink do + describe 'validation' do + let(:group) { create(:group) } + let(:shared_group) { create(:group) } + let!(:group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) } + + it { is_expected.to validate_presence_of(:shared_group) } + it { is_expected.to validate_uniqueness_of(:shared_group_id).scoped_to(:shared_with_group_id).with_message(/already shared/) } + it { is_expected.to validate_presence_of(:shared_group) } + it { is_expected.to validate_presence_of(:group_access) } + it { is_expected.to validate_inclusion_of(:group_access).in_array(Gitlab::Access.values) } + + # it 'does not allow a project to be shared with the group it is in' do + # project_group_link.group = group + # + # expect(project_group_link).not_to be_valid + # end + # + # it 'doesn not allow a project to be shared with an ancestor of the group it is in', :nested_groups do + # project_group_link.group = parent_group + # + # expect(project_group_link).not_to be_valid + # end + end +end diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb new file mode 100644 index 00000000000..1902a04acfe --- /dev/null +++ b/spec/services/groups/group_links/create_service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Groups::GroupLinks::CreateService, '#execute' do + let(:user) { create :user } + let(:group) { create :group } + let(:shared_group) { create :group } + let(:opts) do + { + shared_group_access: Gitlab::Access::DEVELOPER, + expires_at: nil + } + end + let(:subject) { described_class.new(group, user, opts) } + + it 'adds group to another group' do + expect { subject.execute(group) }.to change { group.group_group_links.count }.from(0).to(1) + end + + it 'returns false if shared group is blank' do + expect { subject.execute(nil) }.not_to change { group.group_group_links.count } + end +end |