From eef360912320ecfcb427684edf35672b643a87c0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 18 Nov 2016 14:52:39 +0200 Subject: [PATCH] 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 --- app/models/project.rb | 1 + app/models/project_team.rb | 16 +++++ .../unreleased/rs-project-team-helpers.yml | 4 ++ .../autocomplete_controller_spec.rb | 30 +++++----- spec/models/project_spec.rb | 7 +++ spec/models/project_team_spec.rb | 34 +++++------ spec/services/notification_service_spec.rb | 60 +++++++++---------- 7 files changed, 90 insertions(+), 62 deletions(-) create mode 100644 changelogs/unreleased/rs-project-team-helpers.yml 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) -- GitLab