diff options
author | Rémy Coutable <remy@rymai.me> | 2016-06-27 17:20:57 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-01 18:44:46 +0300 |
commit | bd78f5733ca546bf940438b84aefa2fa3abacb36 (patch) | |
tree | 9d9ac648a594623489e628e025bde48f7ef2b2f9 /spec | |
parent | 557ca2b31ff503b36a4b65af2641fcd0f5682d5b (diff) |
Exclude requesters from Project#members, Group#members and User#members
And create new Project#requesters, Group#requesters scopes.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/groups/group_members_controller_spec.rb | 6 | ||||
-rw-r--r-- | spec/controllers/projects/project_members_controller_spec.rb | 6 | ||||
-rw-r--r-- | spec/features/groups/members/owner_manages_access_requests_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/groups/members/user_requests_access_spec.rb | 8 | ||||
-rw-r--r-- | spec/features/projects/members/master_manages_access_requests_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/projects/members/user_requests_access_spec.rb | 8 | ||||
-rw-r--r-- | spec/helpers/members_helper_spec.rb | 66 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/concerns/access_requestable_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/group_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/member_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 30 |
12 files changed, 150 insertions, 41 deletions
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index ddc54108a7b..c34475976c6 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -133,7 +133,7 @@ describe Groups::GroupMembersController do expect(response).to set_flash.to 'Your access request to the group has been withdrawn.' expect(response).to redirect_to(group_path(group)) - expect(group.members.request).to be_empty + expect(group.requesters).to be_empty expect(group.users).not_to include user end end @@ -153,7 +153,7 @@ describe Groups::GroupMembersController do expect(response).to set_flash.to 'Your request for access has been queued for review.' expect(response).to redirect_to(group_path(group)) - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(group.users).not_to include user end end @@ -175,7 +175,7 @@ describe Groups::GroupMembersController do let(:group_requester) { create(:user) } let(:member) do group.request_access(group_requester) - group.members.request.find_by(user_id: group_requester) + group.requesters.find_by(user_id: group_requester) end context 'when user does not have enough rights' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 29aaceb2302..5e2a8cf3849 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -187,7 +187,7 @@ describe Projects::ProjectMembersController do expect(response).to set_flash.to 'Your access request to the project has been withdrawn.' expect(response).to redirect_to(namespace_project_path(project.namespace, project)) - expect(project.members.request).to be_empty + expect(project.requesters).to be_empty expect(project.users).not_to include user end end @@ -210,7 +210,7 @@ describe Projects::ProjectMembersController do expect(response).to redirect_to( namespace_project_path(project.namespace, project) ) - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(project.users).not_to include user end end @@ -233,7 +233,7 @@ describe Projects::ProjectMembersController do let(:team_requester) { create(:user) } let(:member) do project.request_access(team_requester) - project.members.request.find_by(user_id: team_requester.id) + project.requesters.find_by(user_id: team_requester.id) end context 'when user does not have enough rights' do diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb index 321c9bad7d0..2aae3e00261 100644 --- a/spec/features/groups/members/owner_manages_access_requests_spec.rb +++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb @@ -41,7 +41,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do def expect_visible_access_request(group, user) - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content "#{group.name} access requests 1" expect(page).to have_content user.name end diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb index 4944301c938..d1a6a98ab72 100644 --- a/spec/features/groups/members/user_requests_access_spec.rb +++ b/spec/features/groups/members/user_requests_access_spec.rb @@ -18,7 +18,7 @@ feature 'Groups > Members > User requests access', feature: true do expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email] expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group" - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Withdraw Access Request' @@ -42,7 +42,7 @@ feature 'Groups > Members > User requests access', feature: true do scenario 'user is not listed in the group members page' do click_link 'Request Access' - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy click_link 'Members' @@ -54,11 +54,11 @@ feature 'Groups > Members > User requests access', feature: true do scenario 'user can withdraw its request for access' do click_link 'Request Access' - expect(group.members.request.exists?(user_id: user)).to be_truthy + expect(group.requesters.exists?(user_id: user)).to be_truthy click_link 'Withdraw Access Request' - expect(group.members.request.exists?(user_id: user)).to be_falsey + expect(group.requesters.exists?(user_id: user)).to be_falsey expect(page).to have_content 'Your access request to the group has been withdrawn.' end end diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb index aa2d906fa2e..f7fcd9b6731 100644 --- a/spec/features/projects/members/master_manages_access_requests_spec.rb +++ b/spec/features/projects/members/master_manages_access_requests_spec.rb @@ -40,7 +40,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do end def expect_visible_access_request(project, user) - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content "#{project.name} access requests 1" expect(page).to have_content user.name end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index af420c170ef..f2fe3ef364d 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -17,7 +17,7 @@ feature 'Projects > Members > User requests access', feature: true do expect(ActionMailer::Base.deliveries.last.to).to eq [master.notification_email] expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.name_with_namespace} project" - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy expect(page).to have_content 'Your request for access has been queued for review.' expect(page).to have_content 'Withdraw Access Request' @@ -27,7 +27,7 @@ feature 'Projects > Members > User requests access', feature: true do scenario 'user is not listed in the project members page' do click_link 'Request Access' - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy open_project_settings_menu click_link 'Members' @@ -41,11 +41,11 @@ feature 'Projects > Members > User requests access', feature: true do scenario 'user can withdraw its request for access' do click_link 'Request Access' - expect(project.members.request.exists?(user_id: user)).to be_truthy + expect(project.requesters.exists?(user_id: user)).to be_truthy click_link 'Withdraw Access Request' - expect(project.members.request.exists?(user_id: user)).to be_falsey + expect(project.requesters.exists?(user_id: user)).to be_falsey expect(page).to have_content 'Your access request to the project has been withdrawn.' end diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index f75fdb739f6..7b2155e9a4e 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -57,6 +57,72 @@ describe MembersHelper do end end + describe '#can_see_request_access_button?' do + let(:user) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'source is a group' do + context 'current_user is not a member' do + it 'returns true' do + expect(helper.can_see_request_access_button?(group)).to be_truthy + end + end + + context 'current_user is a member' do + it 'returns false' do + group.add_owner(user) + + expect(helper.can_see_request_access_button?(group)).to be_falsy + end + end + + context 'current_user is a requester' do + it 'returns true' do + group.request_access(user) + + expect(helper.can_see_request_access_button?(group)).to be_truthy + end + end + end + + context 'source is a project' do + context 'current_user is not a member' do + it 'returns true' do + expect(helper.can_see_request_access_button?(project)).to be_truthy + end + end + + context 'current_user is a group member' do + it 'returns false' do + group.add_owner(user) + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + + context 'current_user is a group requester' do + it 'returns false' do + group.request_access(user) + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + + context 'current_user is a member' do + it 'returns false' do + project.team << [user, :master] + + expect(helper.can_see_request_access_button?(project)).to be_falsy + end + end + end + end + describe '#remove_member_message' do let(:requester) { build(:user) } let(:project) { create(:project) } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index ae55a01ebea..e527d649e34 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -406,7 +406,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('project', project_member.id) } @@ -433,7 +433,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('project', project_member.id) } @@ -459,7 +459,7 @@ describe Notify do let(:user) { create(:user) } let(:project_member) do project.request_access(user) - project.members.request.find_by(user_id: user.id) + project.requesters.find_by(user_id: user.id) end subject { Notify.member_access_denied_email('project', project.id, user.id) } @@ -684,7 +684,7 @@ describe Notify do let(:user) { create(:user) } let(:group_member) do group.request_access(user) - group.members.request.find_by(user_id: user.id) + group.requesters.find_by(user_id: user.id) end subject { Notify.member_access_requested_email('group', group_member.id) } @@ -705,7 +705,7 @@ describe Notify do let(:user) { create(:user) } let(:group_member) do group.request_access(user) - group.members.request.find_by(user_id: user.id) + group.requesters.find_by(user_id: user.id) end subject { Notify.member_access_denied_email('group', group.id, user.id) } diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb index 98307876962..96eee0e8bdd 100644 --- a/spec/models/concerns/access_requestable_spec.rb +++ b/spec/models/concerns/access_requestable_spec.rb @@ -16,7 +16,7 @@ describe AccessRequestable do before { group.request_access(user) } - it { expect(group.members.request.exists?(user_id: user)).to be_truthy } + it { expect(group.requesters.exists?(user_id: user)).to be_truthy } end end @@ -34,7 +34,7 @@ describe AccessRequestable do before { project.request_access(user) } - it { expect(project.members.request.exists?(user_id: user)).to be_truthy } + it { expect(project.requesters.exists?(user_id: user)).to be_truthy } end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 2c19aa3f67f..a878ff1b227 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -7,9 +7,38 @@ describe Group, models: true do it { is_expected.to have_many :projects } it { is_expected.to have_many(:group_members).dependent(:destroy) } it { is_expected.to have_many(:users).through(:group_members) } + it { is_expected.to have_many(:owners).through(:group_members) } + it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:project_group_links).dependent(:destroy) } it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } + + describe '#members & #requesters' do + let(:requester) { create(:user) } + let(:developer) { create(:user) } + before do + group.request_access(requester) + group.add_developer(developer) + end + + describe '#members' do + it 'includes members and exclude requesters' do + member_user_ids = group.members.pluck(:user_id) + + expect(member_user_ids).to include(developer.id) + expect(member_user_ids).not_to include(requester.id) + end + end + + describe '#requesters' do + it 'does not include requesters' do + requester_user_ids = group.requesters.pluck(:user_id) + + expect(requester_user_ids).to include(requester.id) + expect(requester_user_ids).not_to include(developer.id) + end + end + end end describe 'modules' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index e9134a3d283..40181a8b906 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -73,10 +73,10 @@ describe Member, models: true do @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) } requested_user = create(:user).tap { |u| project.request_access(u) } - @requested_member = project.members.request.find_by(user_id: requested_user.id) + @requested_member = project.requesters.find_by(user_id: requested_user.id) accepted_request_user = create(:user).tap { |u| project.request_access(u) } - @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } + @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } end describe '.invite' do @@ -103,22 +103,6 @@ describe Member, models: true do it { expect(described_class.request).not_to include @accepted_request_member } end - describe '.non_request' do - it { expect(described_class.non_request).to include @master } - it { expect(described_class.non_request).to include @invited_member } - it { expect(described_class.non_request).to include @accepted_invite_member } - it { expect(described_class.non_request).not_to include @requested_member } - it { expect(described_class.non_request).to include @accepted_request_member } - end - - describe '.non_pending' do - it { expect(described_class.non_pending).to include @master } - it { expect(described_class.non_pending).not_to include @invited_member } - it { expect(described_class.non_pending).to include @accepted_invite_member } - it { expect(described_class.non_pending).not_to include @requested_member } - it { expect(described_class.non_pending).to include @accepted_request_member } - end - describe '.owners_and_masters' do it { expect(described_class.owners_and_masters).to include @owner } it { expect(described_class.owners_and_masters).to include @master } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 42308035d8c..a8c777d1e3e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -11,6 +11,8 @@ describe Project, models: true do it { is_expected.to have_many(:issues).dependent(:destroy) } it { is_expected.to have_many(:milestones).dependent(:destroy) } it { is_expected.to have_many(:project_members).dependent(:destroy) } + it { is_expected.to have_many(:users).through(:project_members) } + it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:snippets).class_name('ProjectSnippet').dependent(:destroy) } it { is_expected.to have_many(:deploy_keys_projects).dependent(:destroy) } @@ -31,6 +33,34 @@ describe Project, models: true do it { is_expected.to have_many(:environments).dependent(:destroy) } it { is_expected.to have_many(:deployments).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + + describe '#members & #requesters' do + let(:project) { create(:project) } + let(:requester) { create(:user) } + let(:developer) { create(:user) } + before do + project.request_access(requester) + project.team << [developer, :developer] + end + + describe '#members' do + it 'includes members and exclude requesters' do + member_user_ids = project.members.pluck(:user_id) + + expect(member_user_ids).to include(developer.id) + expect(member_user_ids).not_to include(requester.id) + end + end + + describe '#requesters' do + it 'does not include requesters' do + requester_user_ids = project.requesters.pluck(:user_id) + + expect(requester_user_ids).to include(requester.id) + expect(requester_user_ids).not_to include(developer.id) + end + end + end end describe 'modules' do |