From 916332815e33c655f727a28457f655f6425306ae Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 18 Nov 2016 19:15:47 +0200 Subject: Drop Project#authorized_for_user? in favor of ProjectTeam#member? Closes #23938 --- app/models/issue.rb | 2 +- app/models/project.rb | 8 ------ app/models/project_team.rb | 8 ++++-- spec/helpers/members_helper_spec.rb | 4 +-- spec/models/issue_spec.rb | 2 +- spec/models/project_spec.rb | 51 ------------------------------------- spec/models/project_team_spec.rb | 51 +++++++++++++++++++++++++++++++++++++ 7 files changed, 61 insertions(+), 65 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 6e8f5d3c422..dd0cb75f9a8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -93,7 +93,7 @@ class Issue < ActiveRecord::Base # Check if we are scoped to a specific project's issues if owner_project - if owner_project.authorized_for_user?(user, Gitlab::Access::REPORTER) + if owner_project.team.member?(user, Gitlab::Access::REPORTER) # If the project is authorized for the user, they can see all issues in the project return all else diff --git a/app/models/project.rb b/app/models/project.rb index 76c1fc4945d..4408c29a54d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1293,14 +1293,6 @@ class Project < ActiveRecord::Base end end - # Checks if `user` is authorized for this project, with at least the - # `min_access_level` (if given). - def authorized_for_user?(user, min_access_level = nil) - return false unless user - - user.authorized_project?(self, min_access_level) - end - def append_or_update_attribute(name, value) old_values = public_send(name.to_s) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 65549d8cd08..5845203055a 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -141,8 +141,12 @@ class ProjectTeam max_member_access(user.id) == Gitlab::Access::MASTER end - def member?(user, min_member_access = Gitlab::Access::GUEST) - max_member_access(user.id) >= min_member_access + # Checks if `user` is authorized for this project, with at least the + # `min_access_level` (if given). + def member?(user, min_access_level = Gitlab::Access::GUEST) + return false unless user + + user.authorized_project?(project, min_access_level) end def human_max_access(user_id) diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index ffca1c94da1..33934cdf8b1 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -10,7 +10,7 @@ describe MembersHelper do end describe '#remove_member_message' do - let(:requester) { build(:user) } + let(:requester) { create(:user) } let(:project) { create(:empty_project, :public, :access_requestable) } let(:project_member) { build(:project_member, project: project) } let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } } @@ -31,7 +31,7 @@ describe MembersHelper do end describe '#remove_member_title' do - let(:requester) { build(:user) } + let(:requester) { create(:user) } let(:project) { create(:empty_project, :public, :access_requestable) } let(:project_member) { build(:project_member, project: project) } let(:project_member_request) { project.request_access(requester) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 300425767ed..89e93dce8c5 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -331,7 +331,7 @@ describe Issue, models: true do end context 'with a user' do - let(:user) { build(:user) } + let(:user) { create(:user) } let(:issue) { build(:issue) } it 'returns true when the issue is readable' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5ba741f40ec..da38254d1bc 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1507,57 +1507,6 @@ describe Project, models: true do end end - describe 'authorized_for_user' do - let(:group) { create(:group) } - let(:developer) { create(:user) } - let(:master) { create(:user) } - let(:personal_project) { create(:project, namespace: developer.namespace) } - let(:group_project) { create(:project, namespace: group) } - let(:members_project) { create(:project) } - let(:shared_project) { create(:project) } - - before do - group.add_master(master) - group.add_developer(developer) - - members_project.team << [developer, :developer] - members_project.team << [master, :master] - - create(:project_group_link, project: shared_project, group: group, group_access: Gitlab::Access::DEVELOPER) - end - - it 'returns false for no user' do - expect(personal_project.authorized_for_user?(nil)).to be(false) - end - - it 'returns true for personal projects of the user' do - expect(personal_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects of groups the user is a member of' do - expect(group_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects for which the user is a member of' do - expect(members_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects shared on a group the user is a member of' do - expect(shared_project.authorized_for_user?(developer)).to be(true) - end - - it 'checks for the correct minimum level access' do - expect(group_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(group_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true) - expect(members_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(members_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true) - expect(shared_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(shared_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(false) - expect(shared_project.authorized_for_user?(developer, Gitlab::Access::DEVELOPER)).to be(true) - expect(shared_project.authorized_for_user?(master, Gitlab::Access::DEVELOPER)).to be(true) - end - end - describe 'change_head' do let(:project) { create(:project) } diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index eb6b009c7cf..573da5e50d4 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -261,6 +261,57 @@ describe ProjectTeam, models: true do end end + describe '#member?' do + let(:group) { create(:group) } + let(:developer) { create(:user) } + let(:master) { create(:user) } + let(:personal_project) { create(:project, namespace: developer.namespace) } + let(:group_project) { create(:project, namespace: group) } + let(:members_project) { create(:project) } + let(:shared_project) { create(:project) } + + before do + group.add_master(master) + group.add_developer(developer) + + members_project.team << [developer, :developer] + members_project.team << [master, :master] + + create(:project_group_link, project: shared_project, group: group) + end + + it 'returns false for no user' do + expect(personal_project.team.member?(nil)).to be(false) + end + + it 'returns true for personal projects of the user' do + expect(personal_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects of groups the user is a member of' do + expect(group_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects for which the user is a member of' do + expect(members_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects shared on a group the user is a member of' do + expect(shared_project.team.member?(developer)).to be(true) + end + + it 'checks for the correct minimum level access' do + expect(group_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(group_project.team.member?(master, Gitlab::Access::MASTER)).to be(true) + expect(members_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(members_project.team.member?(master, Gitlab::Access::MASTER)).to be(true) + expect(shared_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(shared_project.team.member?(master, Gitlab::Access::MASTER)).to be(false) + expect(shared_project.team.member?(developer, Gitlab::Access::DEVELOPER)).to be(true) + expect(shared_project.team.member?(master, Gitlab::Access::DEVELOPER)).to be(true) + end + end + shared_examples_for "#max_member_access_for_users" do |enable_request_store| describe "#max_member_access_for_users" do before do -- cgit v1.2.1