diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 10bc75b4c5ea9294f2d789775afaea4e55927956..3fb8bba3cd079b4fa25386ab46aab7b09fb0d503 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -13,7 +13,13 @@ class Projects::ProjectMembersController < Projects::ApplicationController group = @project.group if group - group_members = group.group_members.where.not(user_id: @project_members.select(:user_id)) + # We need `.where.not(user_id: nil)` here otherwise when a group has an + # invitee, it would make the following query return 0 rows since a NULL + # user_id would be present in the subquery + # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values + # FIXME: This whole logic should be moved to a finder! + non_null_user_ids = @project_members.where.not(user_id: nil).select(:user_id) + group_members = group.group_members.where.not(user_id: non_null_user_ids) group_members = group_members.non_invite unless can?(current_user, :admin_group, @group) end @@ -29,13 +35,13 @@ class Projects::ProjectMembersController < Projects::ApplicationController @group_links = @project.project_group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id)) end - members_id = @project_members.pluck(:id) + member_ids = @project_members.pluck(:id) if group_members - members_id << group_members.pluck(:id) + member_ids += group_members.pluck(:id) end - @project_members = Member.where(id: members_id.flatten).order(access_level: :desc).page(params[:page]) + @project_members = Member.where(id: member_ids).order(access_level: :desc).page(params[:page]) @requesters = AccessRequestsFinder.new(@project).execute(current_user) diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb index 39235a1cd4fe8f8091a83c6ec50116c99169d2bb..7d0065ee2c4c6be39c8a22f5c1da5bcc24c40576 100644 --- a/spec/features/projects/members/group_members_spec.rb +++ b/spec/features/projects/members/group_members_spec.rb @@ -2,20 +2,89 @@ require 'spec_helper' feature 'Projects members', feature: true do let(:user) { create(:user) } - let(:group) { create(:group) } - let(:project) { create(:project, group: group) } + let(:developer) { create(:user) } + let(:group) { create(:group, :public, :access_requestable) } + let(:project) { create(:empty_project, :public, :access_requestable, creator: user, group: group) } + let(:project_invitee) { create(:project_member, project: project, invite_token: '123', invite_email: 'test1@abc.com', user: nil) } + let(:group_invitee) { create(:group_member, group: group, invite_token: '123', invite_email: 'test2@abc.com', user: nil) } + let(:project_requester) { create(:user) } + let(:group_requester) { create(:user) } background do + project.team << [developer, :developer] group.add_owner(user) login_as(user) - visit namespace_project_project_members_path(project.namespace, project) end - it 'shows group members in list' do - expect(page).to have_selector('.group_member') + context 'with a group invitee' do + before do + group_invitee + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'does not appear in the project members page' do + page.within first('.content-list') do + expect(page).not_to have_content('test2@abc.com') + end + end + end + + context 'with a group and a project invitee' do + before do + group_invitee + project_invitee + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'shows the project invitee, the project developer, and the group owner' do + page.within first('.content-list') do + expect(page).to have_content('test1@abc.com') + expect(page).not_to have_content('test2@abc.com') + + # Project developer + expect(page).to have_content(developer.name) + + # Group owner + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end + end + end + + context 'with a group requester' do + before do + group.request_access(group_requester) + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'does not appear in the project members page' do + page.within first('.content-list') do + expect(page).not_to have_content(group_requester.name) + end + end + end + + context 'with a group and a project requesters' do + before do + group.request_access(group_requester) + project.request_access(project_requester) + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'shows the project requester, the project developer, and the group owner' do + page.within first('.content-list') do + expect(page).to have_content(project_requester.name) + expect(page).not_to have_content(group_requester.name) + end + + page.within all('.content-list').last do + # Project developer + expect(page).to have_content(developer.name) - page.within first('.content-list .member') do - expect(page).to have_content(group.name) + # Group owner + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end end end end