Commit eef36091 authored by Robert Speicher's avatar Robert Speicher

Add shortcuts for adding users to a project team with a specific role

This also updates _some_ specs to use these new methods, just to serve
as an example for others going forward, but by no means is this
exhaustive.

Original implementations at !5992 and !6012.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/20944
parent 688ff26d
...@@ -158,6 +158,7 @@ class Project < ActiveRecord::Base ...@@ -158,6 +158,7 @@ class Project < ActiveRecord::Base
delegate :name, to: :owner, allow_nil: true, prefix: true delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :members, to: :team, prefix: true delegate :members, to: :team, prefix: true
delegate :add_user, to: :team delegate :add_user, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
# Validations # Validations
validates :creator, presence: true, on: :create validates :creator, presence: true, on: :create
......
...@@ -21,6 +21,22 @@ class ProjectTeam ...@@ -21,6 +21,22 @@ class ProjectTeam
end end
end end
def add_guest(user, current_user: nil)
self << [user, :guest, current_user]
end
def add_reporter(user, current_user: nil)
self << [user, :reporter, current_user]
end
def add_developer(user, current_user: nil)
self << [user, :developer, current_user]
end
def add_master(user, current_user: nil)
self << [user, :master, current_user]
end
def find_member(user_id) def find_member(user_id)
member = project.members.find_by(user_id: user_id) member = project.members.find_by(user_id: user_id)
......
---
title: Add shortcuts for adding users to a project team with a specific role
merge_request:
author: Nikolay Ponomarev and Dino M
...@@ -11,7 +11,7 @@ describe AutocompleteController do ...@@ -11,7 +11,7 @@ describe AutocompleteController do
context 'project members' do context 'project members' do
before do before do
sign_in(user) sign_in(user)
project.team << [user, :master] project.add_master(user)
end end
describe 'GET #users with project ID' do describe 'GET #users with project ID' do
...@@ -69,7 +69,7 @@ describe AutocompleteController do ...@@ -69,7 +69,7 @@ describe AutocompleteController do
before do before do
sign_in(non_member) sign_in(non_member)
project.team << [user, :master] project.add_master(user)
end end
let(:body) { JSON.parse(response.body) } let(:body) { JSON.parse(response.body) }
...@@ -103,7 +103,7 @@ describe AutocompleteController do ...@@ -103,7 +103,7 @@ describe AutocompleteController do
describe 'GET #users with public project' do describe 'GET #users with public project' do
before do before do
public_project.team << [user, :guest] public_project.add_guest(user)
get(:users, project_id: public_project.id) get(:users, project_id: public_project.id)
end end
...@@ -129,7 +129,7 @@ describe AutocompleteController do ...@@ -129,7 +129,7 @@ describe AutocompleteController do
describe 'GET #users with inaccessible group' do describe 'GET #users with inaccessible group' do
before do before do
project.team << [user, :guest] project.add_guest(user)
get(:users, group_id: user.namespace.id) get(:users, group_id: user.namespace.id)
end end
...@@ -186,12 +186,12 @@ describe AutocompleteController do ...@@ -186,12 +186,12 @@ describe AutocompleteController do
before do before do
sign_in(user) sign_in(user)
project.team << [user, :master] project.add_master(user)
end end
context 'authorized projects' do context 'authorized projects' do
before do before do
authorized_project.team << [user, :master] authorized_project.add_master(user)
end end
describe 'GET #projects with project ID' do describe 'GET #projects with project ID' do
...@@ -216,8 +216,8 @@ describe AutocompleteController do ...@@ -216,8 +216,8 @@ describe AutocompleteController do
context 'authorized projects and search' do context 'authorized projects and search' do
before do before do
authorized_project.team << [user, :master] authorized_project.add_master(user)
authorized_search_project.team << [user, :master] authorized_search_project.add_master(user)
end end
describe 'GET #projects with project ID and search' do describe 'GET #projects with project ID and search' do
...@@ -242,9 +242,9 @@ describe AutocompleteController do ...@@ -242,9 +242,9 @@ describe AutocompleteController do
authorized_project2 = create(:project) authorized_project2 = create(:project)
authorized_project3 = create(:project) authorized_project3 = create(:project)
authorized_project.team << [user, :master] authorized_project.add_master(user)
authorized_project2.team << [user, :master] authorized_project2.add_master(user)
authorized_project3.team << [user, :master] authorized_project3.add_master(user)
stub_const 'MoveToProjectFinder::PAGE_SIZE', 2 stub_const 'MoveToProjectFinder::PAGE_SIZE', 2
end end
...@@ -268,9 +268,9 @@ describe AutocompleteController do ...@@ -268,9 +268,9 @@ describe AutocompleteController do
authorized_project2 = create(:project) authorized_project2 = create(:project)
authorized_project3 = create(:project) authorized_project3 = create(:project)
authorized_project.team << [user, :master] authorized_project.add_master(user)
authorized_project2.team << [user, :master] authorized_project2.add_master(user)
authorized_project3.team << [user, :master] authorized_project3.add_master(user)
end end
describe 'GET #projects with project ID and offset_id' do describe 'GET #projects with project ID and offset_id' do
...@@ -289,7 +289,7 @@ describe AutocompleteController do ...@@ -289,7 +289,7 @@ describe AutocompleteController do
context 'authorized projects without admin_issue ability' do context 'authorized projects without admin_issue ability' do
before(:each) do before(:each) do
authorized_project.team << [user, :guest] authorized_project.add_guest(user)
expect(user.can?(:admin_issue, authorized_project)).to eq(false) expect(user.can?(:admin_issue, authorized_project)).to eq(false)
end end
......
...@@ -241,6 +241,13 @@ describe Project, models: true do ...@@ -241,6 +241,13 @@ describe Project, models: true do
it { is_expected.to respond_to(:path_with_namespace) } it { is_expected.to respond_to(:path_with_namespace) }
end end
describe 'delegation' do
it { is_expected.to delegate_method(:add_guest).to(:team) }
it { is_expected.to delegate_method(:add_reporter).to(:team) }
it { is_expected.to delegate_method(:add_developer).to(:team) }
it { is_expected.to delegate_method(:add_master).to(:team) }
end
describe '#name_with_namespace' do describe '#name_with_namespace' do
let(:project) { build_stubbed(:empty_project) } let(:project) { build_stubbed(:empty_project) }
......
...@@ -10,9 +10,9 @@ describe ProjectTeam, models: true do ...@@ -10,9 +10,9 @@ describe ProjectTeam, models: true do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
before do before do
project.team << [master, :master] project.add_master(master)
project.team << [reporter, :reporter] project.add_reporter(reporter)
project.team << [guest, :guest] project.add_guest(guest)
end end
describe 'members collection' do describe 'members collection' do
...@@ -47,8 +47,8 @@ describe ProjectTeam, models: true do ...@@ -47,8 +47,8 @@ describe ProjectTeam, models: true do
# If user is a group and a project member - GitLab uses highest permission # If user is a group and a project member - GitLab uses highest permission
# So we add group guest as master and add group master as guest # So we add group guest as master and add group master as guest
# to this project to test highest access # to this project to test highest access
project.team << [guest, :master] project.add_master(guest)
project.team << [master, :guest] project.add_guest(master)
end end
describe 'members collection' do describe 'members collection' do
...@@ -79,14 +79,14 @@ describe ProjectTeam, models: true do ...@@ -79,14 +79,14 @@ describe ProjectTeam, models: true do
it 'returns project members' do it 'returns project members' do
user = create(:user) user = create(:user)
project.team << [user, :guest] project.add_guest(user)
expect(project.team.members).to contain_exactly(user) expect(project.team.members).to contain_exactly(user)
end end
it 'returns project members of a specified level' do it 'returns project members of a specified level' do
user = create(:user) user = create(:user)
project.team << [user, :reporter] project.add_reporter(user)
expect(project.team.guests).to be_empty expect(project.team.guests).to be_empty
expect(project.team.reporters).to contain_exactly(user) expect(project.team.reporters).to contain_exactly(user)
...@@ -141,9 +141,9 @@ describe ProjectTeam, models: true do ...@@ -141,9 +141,9 @@ describe ProjectTeam, models: true do
let(:requester) { create(:user) } let(:requester) { create(:user) }
before do before do
project.team << [master, :master] project.add_master(master)
project.team << [reporter, :reporter] project.add_reporter(reporter)
project.team << [guest, :guest] project.add_guest(guest)
project.request_access(requester) project.request_access(requester)
end end
...@@ -204,9 +204,9 @@ describe ProjectTeam, models: true do ...@@ -204,9 +204,9 @@ describe ProjectTeam, models: true do
context 'when project is not shared with group' do context 'when project is not shared with group' do
before do before do
project.team << [master, :master] project.add_master(master)
project.team << [reporter, :reporter] project.add_reporter(reporter)
project.team << [guest, :guest] project.add_guest(guest)
project.request_access(requester) project.request_access(requester)
end end
...@@ -281,10 +281,10 @@ describe ProjectTeam, models: true do ...@@ -281,10 +281,10 @@ describe ProjectTeam, models: true do
guest = create(:user) guest = create(:user)
project = create(:project) project = create(:project)
project.team << [master, :master] project.add_master(master)
project.team << [reporter, :reporter] project.add_reporter(reporter)
project.team << [promoted_guest, :guest] project.add_guest(promoted_guest)
project.team << [guest, :guest] project.add_guest(guest)
group = create(:group) group = create(:group)
group_developer = create(:user) group_developer = create(:user)
......
...@@ -64,9 +64,9 @@ describe NotificationService, services: true do ...@@ -64,9 +64,9 @@ describe NotificationService, services: true do
before do before do
build_team(note.project) build_team(note.project)
project.team << [issue.author, :master] project.add_master(issue.author)
project.team << [issue.assignee, :master] project.add_master(issue.assignee)
project.team << [note.author, :master] project.add_master(note.author)
create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy')
update_custom_notification(:new_note, @u_guest_custom, project) update_custom_notification(:new_note, @u_guest_custom, project)
update_custom_notification(:new_note, @u_custom_global) update_custom_notification(:new_note, @u_custom_global)
...@@ -168,8 +168,8 @@ describe NotificationService, services: true do ...@@ -168,8 +168,8 @@ describe NotificationService, services: true do
let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") } let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
it 'filters out users that can not read the issue' do it 'filters out users that can not read the issue' do
project.team << [member, :developer] project.add_developer(member)
project.team << [guest, :guest] project.add_guest(guest)
expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times
...@@ -195,7 +195,7 @@ describe NotificationService, services: true do ...@@ -195,7 +195,7 @@ describe NotificationService, services: true do
before do before do
build_team(note.project) build_team(note.project)
note.project.team << [note.author, :master] note.project.add_master(note.author)
reset_delivered_emails! reset_delivered_emails!
end end
...@@ -237,7 +237,7 @@ describe NotificationService, services: true do ...@@ -237,7 +237,7 @@ describe NotificationService, services: true do
before do before do
build_team(note.project) build_team(note.project)
note.project.team << [note.author, :master] note.project.add_master(note.author)
reset_delivered_emails! reset_delivered_emails!
end end
...@@ -324,8 +324,8 @@ describe NotificationService, services: true do ...@@ -324,8 +324,8 @@ describe NotificationService, services: true do
before do before do
build_team(note.project) build_team(note.project)
project.team << [merge_request.author, :master] project.add_master(merge_request.author)
project.team << [merge_request.assignee, :master] project.add_master(merge_request.assignee)
end end
describe '#new_note' do describe '#new_note' do
...@@ -409,8 +409,8 @@ describe NotificationService, services: true do ...@@ -409,8 +409,8 @@ describe NotificationService, services: true do
let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) } let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
it "emails subscribers of the issue's labels that can read the issue" do it "emails subscribers of the issue's labels that can read the issue" do
project.team << [member, :developer] project.add_developer(member)
project.team << [guest, :guest] project.add_guest(guest)
label = create(:label, project: project, issues: [confidential_issue]) label = create(:label, project: project, issues: [confidential_issue])
confidential_issue.reload confidential_issue.reload
...@@ -621,8 +621,8 @@ describe NotificationService, services: true do ...@@ -621,8 +621,8 @@ describe NotificationService, services: true do
let!(:label_2) { create(:label, project: project) } let!(:label_2) { create(:label, project: project) }
it "emails subscribers of the issue's labels that can read the issue" do it "emails subscribers of the issue's labels that can read the issue" do
project.team << [member, :developer] project.add_developer(member)
project.team << [guest, :guest] project.add_guest(guest)
label_2.toggle_subscription(non_member, project) label_2.toggle_subscription(non_member, project)
label_2.toggle_subscription(author, project) label_2.toggle_subscription(author, project)
...@@ -1210,7 +1210,7 @@ describe NotificationService, services: true do ...@@ -1210,7 +1210,7 @@ describe NotificationService, services: true do
let(:member) { create(:user) } let(:member) { create(:user) }
before(:each) do before(:each) do
project.team << [member, :developer, project.owner] project.add_developer(member, current_user: project.owner)
end end
it do it do
...@@ -1233,9 +1233,9 @@ describe NotificationService, services: true do ...@@ -1233,9 +1233,9 @@ describe NotificationService, services: true do
let(:note) { create(:note, noteable: merge_request, project: private_project) } let(:note) { create(:note, noteable: merge_request, project: private_project) }
before do before do
private_project.team << [assignee, :developer] private_project.add_developer(assignee)
private_project.team << [developer, :developer] private_project.add_developer(developer)
private_project.team << [guest, :guest] private_project.add_guest(guest)
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
end end
...@@ -1297,15 +1297,15 @@ describe NotificationService, services: true do ...@@ -1297,15 +1297,15 @@ describe NotificationService, services: true do
@u_guest_watcher = create_user_with_notification(:watch, 'guest_watching') @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
@u_guest_custom = create_user_with_notification(:custom, 'guest_custom') @u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
project.team << [@u_watcher, :master] project.add_master(@u_watcher)
project.team << [@u_participating, :master] project.add_master(@u_participating)
project.team << [@u_participant_mentioned, :master] project.add_master(@u_participant_mentioned)
project.team << [@u_disabled, :master] project.add_master(@u_disabled)
project.team << [@u_mentioned, :master] project.add_master(@u_mentioned)
project.team << [@u_committer, :master] project.add_master(@u_committer)
project.team << [@u_not_mentioned, :master] project.add_master(@u_not_mentioned)
project.team << [@u_lazy_participant, :master] project.add_master(@u_lazy_participant)
project.team << [@u_custom_global, :master] project.add_master(@u_custom_global)
end end
def create_global_setting_for(user, level) def create_global_setting_for(user, level)
...@@ -1339,10 +1339,10 @@ describe NotificationService, services: true do ...@@ -1339,10 +1339,10 @@ describe NotificationService, services: true do
@subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating) @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
@watcher_and_subscriber = create_global_setting_for(create(:user), :watch) @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
project.team << [@subscribed_participant, :master] project.add_master(@subscribed_participant)
project.team << [@subscriber, :master] project.add_master(@subscriber)
project.team << [@unsubscriber, :master] project.add_master(@unsubscriber)
project.team << [@watcher_and_subscriber, :master] project.add_master(@watcher_and_subscriber)
issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true) issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true) issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment