Merge branch 'rs-project-team-helpers-ee' into 'master'

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

See merge request !888

app/models/project.rb

@@ -167,6 +167,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

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)
 

changelogs/unreleased/rs-project-team-helpers.yml

+---
+title: Add shortcuts for adding users to a project team with a specific role
+merge_request:
+author: Nikolay Ponomarev and Dino M

spec/controllers/autocomplete_controller_spec.rb

@@ -11,8 +11,8 @@ describe AutocompleteController do
     context 'project members' do
       before do
         sign_in(user)
-        project.team << [user, :master]
-        project.team << [user2, :developer]
+        project.add_master(user)
+        project.add_developer(user2)
       end
 
       describe 'GET #users with project ID' do
@@ -97,7 +97,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) }
@@ -131,7 +131,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
 
@@ -157,7 +157,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
 
@@ -214,12 +214,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
@@ -244,8 +244,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
@@ -270,9 +270,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
@@ -296,9 +296,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
@@ -317,7 +317,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

spec/models/project_spec.rb

@@ -256,6 +256,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) }
 

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)

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)
@@ -1245,7 +1245,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
@@ -1268,9 +1268,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
@@ -1332,15 +1332,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)
@@ -1374,10 +1374,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)