Skip to content

Commit

Permalink
Merge branch '20614-show-member-roles-to-all-users' into 'master'
Browse files Browse the repository at this point in the history
Show member roles to all users on members page

Closes #20614 and https://gitlab.com/gitlab-org/gitlab-ee/issues/850

See merge request !5776
  • Loading branch information
Robert Speicher committed Aug 11, 2016
2 parents 5a33bc9 + 32b579e commit ded6029
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 84 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Expand Up @@ -23,6 +23,7 @@ v 8.11.0 (unreleased)
- Cache highlighted diff lines for merge requests
- Pre-create all builds for a Pipeline when the new Pipeline is created !5295
- Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
- Show member roles to all users on members page
- Fix awardable button mutuality loading spinners (ClemMakesApps)
- Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable
- Optimize maximum user access level lookup in loading of notes
Expand Down
6 changes: 0 additions & 6 deletions app/helpers/members_helper.rb
Expand Up @@ -6,12 +6,6 @@ def action_member_permission(action, member)
"#{action}_#{member.type.underscore}".to_sym
end

def default_show_roles(member)
can?(current_user, action_member_permission(:update, member), member) ||
can?(current_user, action_member_permission(:destroy, member), member) ||
can?(current_user, action_member_permission(:admin, member), member.source)
end

def remove_member_message(member, user: nil)
user = current_user if defined?(current_user)

Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/members/_member.html.haml
@@ -1,4 +1,4 @@
- show_roles = local_assigns.fetch(:show_roles, default_show_roles(member))
- show_roles = local_assigns.fetch(:show_roles, true)
- show_controls = local_assigns.fetch(:show_controls, true)
- user = member.user

Expand Down
25 changes: 0 additions & 25 deletions features/explore/groups.feature
Expand Up @@ -24,14 +24,6 @@ Feature: Explore Groups
Then I should see project "Internal" items
And I should not see project "Enterprise" items

Scenario: I should see group's members as user
Given group "TestGroup" has internal project "Internal"
And "John Doe" is owner of group "TestGroup"
When I sign in as a user
And I visit group "TestGroup" members page
Then I should see group member "John Doe"
And I should not see member roles

Scenario: I should see group with private, internal and public projects as visitor
Given group "TestGroup" has internal project "Internal"
Given group "TestGroup" has public project "Community"
Expand All @@ -56,14 +48,6 @@ Feature: Explore Groups
And I should not see project "Internal" items
And I should not see project "Enterprise" items

Scenario: I should see group's members as visitor
Given group "TestGroup" has internal project "Internal"
Given group "TestGroup" has public project "Community"
And "John Doe" is owner of group "TestGroup"
When I visit group "TestGroup" members page
Then I should see group member "John Doe"
And I should not see member roles

Scenario: I should see group with private, internal and public projects as user
Given group "TestGroup" has internal project "Internal"
Given group "TestGroup" has public project "Community"
Expand Down Expand Up @@ -91,15 +75,6 @@ Feature: Explore Groups
And I should see project "Internal" items
And I should not see project "Enterprise" items

Scenario: I should see group's members as user
Given group "TestGroup" has internal project "Internal"
Given group "TestGroup" has public project "Community"
And "John Doe" is owner of group "TestGroup"
When I sign in as a user
And I visit group "TestGroup" members page
Then I should see group member "John Doe"
And I should not see member roles

Scenario: I should see group with public project in public groups area
Given group "TestGroup" has public project "Community"
When I visit the public groups area
Expand Down
4 changes: 0 additions & 4 deletions features/steps/explore/groups.rb
Expand Up @@ -62,10 +62,6 @@ class Spinach::Features::ExploreGroups < Spinach::FeatureSteps
expect(page).to have_content "John Doe"
end

step 'I should not see member roles' do
expect(body).not_to match(%r{owner|developer|reporter|guest}i)
end

protected

def group_has_project(groupname, projectname, visibility_level)
Expand Down
48 changes: 0 additions & 48 deletions spec/helpers/members_helper_spec.rb
Expand Up @@ -9,54 +9,6 @@
it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
end

describe '#default_show_roles' do
let(:user) { double }
let(:member) { build(:project_member) }

before do
allow(helper).to receive(:current_user).and_return(user)
allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(false)
allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(false)
allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(false)
end

context 'when the current cannot update, destroy or admin the passed member' do
it 'returns false' do
expect(helper.default_show_roles(member)).to be_falsy
end
end

context 'when the current can update the passed member' do
before do
allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(true)
end

it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end

context 'when the current can destroy the passed member' do
before do
allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(true)
end

it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end

context 'when the current can admin the passed member source' do
before do
allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(true)
end

it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end
end

describe '#remove_member_message' do
let(:requester) { build(:user) }
let(:project) { create(:project) }
Expand Down

0 comments on commit ded6029

Please sign in to comment.