diff --git a/app/controllers/dashboard/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb index 71ba6153021fb1919044288de79a4a8a7e9f3e16..de6bc689bb7d6fa208c7692d9d0ffc4e1e56f05a 100644 --- a/app/controllers/dashboard/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -1,5 +1,5 @@ class Dashboard::GroupsController < Dashboard::ApplicationController def index - @group_members = current_user.group_members.page(params[:page]) + @group_members = current_user.group_members.includes(:source).page(params[:page]) end end diff --git a/app/models/group.rb b/app/models/group.rb index e66e04371b2c4a66cd805fdbe82ce26c0e35f91c..c70c719e33856d4e24c7d17b2ac876e27616788f 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -11,7 +11,7 @@ class Group < Namespace has_many :users, -> { where(members: { requested_at: nil }) }, through: :group_members has_many :owners, - -> { where(members: { access_level: Gitlab::Access::OWNER }) }, + -> { where(members: { requested_at: nil, access_level: Gitlab::Access::OWNER }) }, through: :group_members, source: :user diff --git a/app/models/user.rb b/app/models/user.rb index 04b220ee13c6e43f0246469062eeaa4a71185550..599b2fb1191852c629170d310cc7d5aa64ba0dd1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -57,7 +57,7 @@ class User < ActiveRecord::Base # Groups has_many :members, dependent: :destroy - has_many :group_members, dependent: :destroy, source: 'GroupMember' + has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, source: 'GroupMember' has_many :groups, through: :group_members has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group @@ -65,7 +65,7 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects - has_many :project_members, dependent: :destroy, class_name: 'ProjectMember' + has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, class_name: 'ProjectMember' has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy diff --git a/app/views/admin/users/groups.html.haml b/app/views/admin/users/groups.html.haml index b0a709a568a62ac73c42b56e888becc704922264..8f6d13b881a266931a40b8bf3432c085b84d4aaa 100644 --- a/app/views/admin/users/groups.html.haml +++ b/app/views/admin/users/groups.html.haml @@ -1,11 +1,12 @@ - page_title "Groups", @user.name, "Users" = render 'admin/users/head' -- if @user.group_members.present? +- group_members = @user.group_members.includes(:source) +- if group_members.any? .panel.panel-default .panel-heading Groups: %ul.well-list - - @user.group_members.each do |group_member| + - group_members.each do |group_member| - group = group_member.group %li.group_member %span{class: ("list-item-name" unless group_member.owner?)} diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb index 1ea607cbca065b1178ddfc704a20a4243a1925be..4944301c938c61dfdbcb93535488c3451222c831 100644 --- a/spec/features/groups/members/user_requests_access_spec.rb +++ b/spec/features/groups/members/user_requests_access_spec.rb @@ -4,6 +4,7 @@ feature 'Groups > Members > User requests access', feature: true do let(:user) { create(:user) } let(:owner) { create(:user) } let(:group) { create(:group, :public) } + let!(:project) { create(:project, :private, namespace: group) } background do group.add_owner(owner) @@ -24,6 +25,20 @@ feature 'Groups > Members > User requests access', feature: true do expect(page).not_to have_content 'Leave Group' end + scenario 'user does not see private projects' do + perform_enqueued_jobs { click_link 'Request Access' } + + expect(page).not_to have_content project.name + end + + scenario 'user does not see group in the Dashboard > Groups page' do + perform_enqueued_jobs { click_link 'Request Access' } + + visit dashboard_groups_path + + expect(page).not_to have_content group.name + end + scenario 'user is not listed in the group members page' do click_link 'Request Access' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 73bee535fe37ba1fc8530289efb3226f34045091..328254ed56b738256423bb22fb1abe2dc8087658 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -31,6 +31,26 @@ describe User, models: true do it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + + describe '#group_members' do + it 'does not include group memberships for which user is a requester' do + user = create(:user) + group = create(:group, :public) + group.request_access(user) + + expect(user.group_members).to be_empty + end + end + + describe '#project_members' do + it 'does not include project memberships for which user is a requester' do + user = create(:user) + project = create(:project, :public) + project.request_access(user) + + expect(user.project_members).to be_empty + end + end end describe 'validations' do