Add shortcuts for adding users to a project team with a specific rolers-project-team-helpers

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

7 files changed, 90 insertions, 62 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index f9bcc547c36..facaddb3aa4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -158,6 +158,7 @@ class Project < ActiveRecord::Base
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :members, to: :team, prefix: true
   delegate :add_user, to: :team
+  delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
 
   # Validations
   validates :creator, presence: true, on: :create
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index a6e911df9bd..65549d8cd08 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -21,6 +21,22 @@ class ProjectTeam
     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)
     member = project.members.find_by(user_id: user_id)
 
diff --git a/changelogs/unreleased/rs-project-team-helpers.yml b/changelogs/unreleased/rs-project-team-helpers.yml
new file mode 100644
index 00000000000..79abcbce1e3
--- /dev/null
+++ b/changelogs/unreleased/rs-project-team-helpers.yml
@@ -0,0 +1,4 @@
+---
+title: Add shortcuts for adding users to a project team with a specific role
+merge_request:
+author: Nikolay Ponomarev and Dino M
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb
index a121cb2fc97..d9a86346c81 100644
--- a/spec/controllers/autocomplete_controller_spec.rb
+++ b/spec/controllers/autocomplete_controller_spec.rb
@@ -11,7 +11,7 @@ describe AutocompleteController do
     context 'project members' do
       before do
         sign_in(user)
-        project.team << [user, :master]
+        project.add_master(user)
       end
 
       describe 'GET #users with project ID' do
@@ -69,7 +69,7 @@ describe AutocompleteController do
 
       before do
         sign_in(non_member)
-        project.team << [user, :master]
+        project.add_master(user)
       end
 
       let(:body) { JSON.parse(response.body) }
@@ -103,7 +103,7 @@ describe AutocompleteController do
 
       describe 'GET #users with public project' do
         before do
-          public_project.team << [user, :guest]
+          public_project.add_guest(user)
           get(:users, project_id: public_project.id)
         end
 
@@ -129,7 +129,7 @@ describe AutocompleteController do
 
       describe 'GET #users with inaccessible group' do
         before do
-          project.team << [user, :guest]
+          project.add_guest(user)
           get(:users, group_id: user.namespace.id)
         end
 
@@ -186,12 +186,12 @@ describe AutocompleteController do
 
     before do
       sign_in(user)
-      project.team << [user, :master]
+      project.add_master(user)
     end
 
     context 'authorized projects' do
       before do
-        authorized_project.team << [user, :master]
+        authorized_project.add_master(user)
       end
 
       describe 'GET #projects with project ID' do
@@ -216,8 +216,8 @@ describe AutocompleteController do
 
     context 'authorized projects and search' do
       before do
-        authorized_project.team << [user, :master]
-        authorized_search_project.team << [user, :master]
+        authorized_project.add_master(user)
+        authorized_search_project.add_master(user)
       end
 
       describe 'GET #projects with project ID and search' do
@@ -242,9 +242,9 @@ describe AutocompleteController do
         authorized_project2 = create(:project)
         authorized_project3 = create(:project)
 
-        authorized_project.team << [user, :master]
-        authorized_project2.team << [user, :master]
-        authorized_project3.team << [user, :master]
+        authorized_project.add_master(user)
+        authorized_project2.add_master(user)
+        authorized_project3.add_master(user)
 
         stub_const 'MoveToProjectFinder::PAGE_SIZE', 2
       end
@@ -268,9 +268,9 @@ describe AutocompleteController do
         authorized_project2 = create(:project)
         authorized_project3 = create(:project)
 
-        authorized_project.team << [user, :master]
-        authorized_project2.team << [user, :master]
-        authorized_project3.team << [user, :master]
+        authorized_project.add_master(user)
+        authorized_project2.add_master(user)
+        authorized_project3.add_master(user)
       end
 
       describe 'GET #projects with project ID and offset_id' do
@@ -289,7 +289,7 @@ describe AutocompleteController do
 
     context 'authorized projects without admin_issue ability' do
       before(:each) do
-        authorized_project.team << [user, :guest]
+        authorized_project.add_guest(user)
 
         expect(user.can?(:admin_issue, authorized_project)).to eq(false)
       end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 46fa00a79c4..2fae3dde58c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -241,6 +241,13 @@ describe Project, models: true do
     it { is_expected.to respond_to(:path_with_namespace) }
   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
     let(:project) { build_stubbed(:empty_project) }
 
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 12228425579..eb6b009c7cf 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -10,9 +10,9 @@ describe ProjectTeam, models: true do
     let(:project) { create(:empty_project) }
 
     before do
-      project.team << [master, :master]
-      project.team << [reporter, :reporter]
-      project.team << [guest, :guest]
+      project.add_master(master)
+      project.add_reporter(reporter)
+      project.add_guest(guest)
     end
 
     describe 'members collection' do
@@ -47,8 +47,8 @@ describe ProjectTeam, models: true do
       # 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
       # to this project to test highest access
-      project.team << [guest, :master]
-      project.team << [master, :guest]
+      project.add_master(guest)
+      project.add_guest(master)
     end
 
     describe 'members collection' do
@@ -79,14 +79,14 @@ describe ProjectTeam, models: true do
 
       it 'returns project members' do
         user = create(:user)
-        project.team << [user, :guest]
+        project.add_guest(user)
 
         expect(project.team.members).to contain_exactly(user)
       end
 
       it 'returns project members of a specified level' do
         user = create(:user)
-        project.team << [user, :reporter]
+        project.add_reporter(user)
 
         expect(project.team.guests).to be_empty
         expect(project.team.reporters).to contain_exactly(user)
@@ -141,9 +141,9 @@ describe ProjectTeam, models: true do
       let(:requester) { create(:user) }
 
       before do
-        project.team << [master, :master]
-        project.team << [reporter, :reporter]
-        project.team << [guest, :guest]
+        project.add_master(master)
+        project.add_reporter(reporter)
+        project.add_guest(guest)
         project.request_access(requester)
       end
 
@@ -204,9 +204,9 @@ describe ProjectTeam, models: true do
 
       context 'when project is not shared with group' do
         before do
-          project.team << [master, :master]
-          project.team << [reporter, :reporter]
-          project.team << [guest, :guest]
+          project.add_master(master)
+          project.add_reporter(reporter)
+          project.add_guest(guest)
           project.request_access(requester)
         end
 
@@ -281,10 +281,10 @@ describe ProjectTeam, models: true do
         guest = create(:user)
         project = create(:project)
 
-        project.team << [master, :master]
-        project.team << [reporter, :reporter]
-        project.team << [promoted_guest, :guest]
-        project.team << [guest, :guest]
+        project.add_master(master)
+        project.add_reporter(reporter)
+        project.add_guest(promoted_guest)
+        project.add_guest(guest)
 
         group = create(:group)
         group_developer = create(:user)
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 8726c9eaa55..08ae61708a5 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -64,9 +64,9 @@ describe NotificationService, services: true do
 
       before do
         build_team(note.project)
-        project.team << [issue.author, :master]
-        project.team << [issue.assignee, :master]
-        project.team << [note.author, :master]
+        project.add_master(issue.author)
+        project.add_master(issue.assignee)
+        project.add_master(note.author)
         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_custom_global)
@@ -168,8 +168,8 @@ describe NotificationService, services: true do
       let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
 
       it 'filters out users that can not read the issue' do
-        project.team << [member, :developer]
-        project.team << [guest, :guest]
+        project.add_developer(member)
+        project.add_guest(guest)
 
         expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times
 
@@ -195,7 +195,7 @@ describe NotificationService, services: true do
 
       before do
         build_team(note.project)
-        note.project.team << [note.author, :master]
+        note.project.add_master(note.author)
         reset_delivered_emails!
       end
 
@@ -237,7 +237,7 @@ describe NotificationService, services: true do
 
       before do
         build_team(note.project)
-        note.project.team << [note.author, :master]
+        note.project.add_master(note.author)
         reset_delivered_emails!
       end
 
@@ -324,8 +324,8 @@ describe NotificationService, services: true do
 
       before do
         build_team(note.project)
-        project.team << [merge_request.author, :master]
-        project.team << [merge_request.assignee, :master]
+        project.add_master(merge_request.author)
+        project.add_master(merge_request.assignee)
       end
 
       describe '#new_note' 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) }
 
         it "emails subscribers of the issue's labels that can read the issue" do
-          project.team << [member, :developer]
-          project.team << [guest, :guest]
+          project.add_developer(member)
+          project.add_guest(guest)
 
           label = create(:label, project: project, issues: [confidential_issue])
           confidential_issue.reload
@@ -621,8 +621,8 @@ describe NotificationService, services: true do
         let!(:label_2) { create(:label, project: project) }
 
         it "emails subscribers of the issue's labels that can read the issue" do
-          project.team << [member, :developer]
-          project.team << [guest, :guest]
+          project.add_developer(member)
+          project.add_guest(guest)
 
           label_2.toggle_subscription(non_member, project)
           label_2.toggle_subscription(author, project)
@@ -1210,7 +1210,7 @@ describe NotificationService, services: true do
       let(:member) { create(:user) }
 
       before(:each) do
-        project.team << [member, :developer, project.owner]
+        project.add_developer(member, current_user: project.owner)
       end
 
       it do
@@ -1233,9 +1233,9 @@ describe NotificationService, services: true do
     let(:note) { create(:note, noteable: merge_request, project: private_project) }
 
     before do
-      private_project.team << [assignee, :developer]
-      private_project.team << [developer, :developer]
-      private_project.team << [guest, :guest]
+      private_project.add_developer(assignee)
+      private_project.add_developer(developer)
+      private_project.add_guest(guest)
 
       ActionMailer::Base.deliveries.clear
     end
@@ -1297,15 +1297,15 @@ describe NotificationService, services: true do
     @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
     @u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
 
-    project.team << [@u_watcher, :master]
-    project.team << [@u_participating, :master]
-    project.team << [@u_participant_mentioned, :master]
-    project.team << [@u_disabled, :master]
-    project.team << [@u_mentioned, :master]
-    project.team << [@u_committer, :master]
-    project.team << [@u_not_mentioned, :master]
-    project.team << [@u_lazy_participant, :master]
-    project.team << [@u_custom_global, :master]
+    project.add_master(@u_watcher)
+    project.add_master(@u_participating)
+    project.add_master(@u_participant_mentioned)
+    project.add_master(@u_disabled)
+    project.add_master(@u_mentioned)
+    project.add_master(@u_committer)
+    project.add_master(@u_not_mentioned)
+    project.add_master(@u_lazy_participant)
+    project.add_master(@u_custom_global)
   end
 
   def create_global_setting_for(user, level)
@@ -1339,10 +1339,10 @@ describe NotificationService, services: true do
     @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
     @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
 
-    project.team << [@subscribed_participant, :master]
-    project.team << [@subscriber, :master]
-    project.team << [@unsubscriber, :master]
-    project.team << [@watcher_and_subscriber, :master]
+    project.add_master(@subscribed_participant)
+    project.add_master(@subscriber)
+    project.add_master(@unsubscriber)
+    project.add_master(@watcher_and_subscriber)
 
     issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
     issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)