diff options
Diffstat (limited to 'spec/services')
17 files changed, 144 insertions, 145 deletions
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index ca366cdf1df..363b7266940 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -76,14 +76,14 @@ describe Issuable::BulkUpdateService do end describe 'updating merge request assignee' do - let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignee: user) } + let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignees: [user]) } context 'when the new assignee ID is a valid user' do it 'succeeds' do new_assignee = create(:user) project.add_developer(new_assignee) - result = bulk_update(merge_request, assignee_id: new_assignee.id) + result = bulk_update(merge_request, assignee_ids: [user.id, new_assignee.id]) expect(result[:success]).to be_truthy expect(result[:count]).to eq(1) @@ -93,22 +93,22 @@ describe Issuable::BulkUpdateService do assignee = create(:user) project.add_developer(assignee) - expect { bulk_update(merge_request, assignee_id: assignee.id) } - .to change { merge_request.reload.assignee }.from(user).to(assignee) + expect { bulk_update(merge_request, assignee_ids: [assignee.id]) } + .to change { merge_request.reload.assignee_ids }.from([user.id]).to([assignee.id]) end end context "when the new assignee ID is #{IssuableFinder::NONE}" do it 'unassigns the issues' do - expect { bulk_update(merge_request, assignee_id: IssuableFinder::NONE) } - .to change { merge_request.reload.assignee }.to(nil) + expect { bulk_update(merge_request, assignee_ids: [IssuableFinder::NONE]) } + .to change { merge_request.reload.assignee_ids }.to([]) end end context 'when the new assignee ID is not present' do it 'does not unassign' do - expect { bulk_update(merge_request, assignee_id: nil) } - .not_to change { merge_request.reload.assignee } + expect { bulk_update(merge_request, assignee_ids: []) } + .not_to change { merge_request.reload.assignee_ids } end end end diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb index 8ccbba7fa58..15d1bb73ca3 100644 --- a/spec/services/issuable/destroy_service_spec.rb +++ b/spec/services/issuable/destroy_service_spec.rb @@ -34,7 +34,7 @@ describe Issuable::DestroyService do end context 'when issuable is a merge request' do - let!(:merge_request) { create(:merge_request, target_project: project, source_project: project, author: user, assignee: user) } + let!(:merge_request) { create(:merge_request, target_project: project, source_project: project, author: user, assignees: [user]) } it 'destroys the merge request' do expect { service.execute(merge_request) }.to change { project.merge_requests.count }.by(-1) diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index d37ca13ebd2..91bf4dccd77 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -43,9 +43,9 @@ describe Members::DestroyService do shared_examples 'a service destroying a member with access' do it_behaves_like 'a service destroying a member' - it 'invalidates cached counts for todos and assigned issues and merge requests', :aggregate_failures do + it 'invalidates cached counts for assigned issues and merge requests', :aggregate_failures do create(:issue, project: group_project, assignees: [member_user]) - create(:merge_request, source_project: group_project, assignee: member_user) + create(:merge_request, source_project: group_project, assignees: [member_user]) create(:todo, :pending, project: group_project, user: member_user) create(:todo, :done, project: group_project, user: member_user) diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 433ffbd97f0..706bcea8199 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -4,7 +4,7 @@ describe MergeRequests::CloseService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:merge_request) { create(:merge_request, assignee: user2, author: create(:user)) } + let(:merge_request) { create(:merge_request, assignees: [user2], author: create(:user)) } let(:project) { merge_request.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index 393299cce00..20bf1cbb8b6 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -118,7 +118,7 @@ describe MergeRequests::CreateFromIssueService do result = service.execute - expect(result[:merge_request].assignee).to eq(user) + expect(result[:merge_request].assignees).to eq([user]) end context 'when ref branch is set' do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index dc5d1cf2f04..30271e04c8e 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -32,7 +32,7 @@ describe MergeRequests::CreateService do expect(merge_request).to be_valid expect(merge_request.work_in_progress?).to be(false) expect(merge_request.title).to eq('Awesome merge_request') - expect(merge_request.assignee).to be_nil + expect(merge_request.assignees).to be_empty expect(merge_request.merge_params['force_remove_source_branch']).to eq('1') end @@ -73,7 +73,7 @@ describe MergeRequests::CreateService do description: "well this is not done yet\n/wip", source_branch: 'feature', target_branch: 'master', - assignee: assignee + assignees: [assignee] } end @@ -89,7 +89,7 @@ describe MergeRequests::CreateService do description: "well this is not done yet\n/wip", source_branch: 'feature', target_branch: 'master', - assignee: assignee + assignees: [assignee] } end @@ -106,11 +106,11 @@ describe MergeRequests::CreateService do description: 'please fix', source_branch: 'feature', target_branch: 'master', - assignee: assignee + assignees: [assignee] } end - it { expect(merge_request.assignee).to eq assignee } + it { expect(merge_request.assignees).to eq([assignee]) } it 'creates a todo for new assignee' do attributes = { @@ -301,7 +301,7 @@ describe MergeRequests::CreateService do let(:opts) do { - assignee_id: create(:user).id, + assignee_ids: create(:user).id, milestone_id: 1, title: 'Title', description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}"), @@ -317,7 +317,7 @@ describe MergeRequests::CreateService do it 'assigns and sets milestone to issuable from command' do expect(merge_request).to be_persisted - expect(merge_request.assignee).to eq(assignee) + expect(merge_request.assignees).to eq([assignee]) expect(merge_request.milestone).to eq(milestone) end end @@ -332,28 +332,28 @@ describe MergeRequests::CreateService do end it 'removes assignee_id when user id is invalid' do - opts = { title: 'Title', description: 'Description', assignee_id: -1 } + opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } merge_request = described_class.new(project, user, opts).execute - expect(merge_request.assignee_id).to be_nil + expect(merge_request.assignee_ids).to be_empty end it 'removes assignee_id when user id is 0' do - opts = { title: 'Title', description: 'Description', assignee_id: 0 } + opts = { title: 'Title', description: 'Description', assignee_ids: [0] } merge_request = described_class.new(project, user, opts).execute - expect(merge_request.assignee_id).to be_nil + expect(merge_request.assignee_ids).to be_empty end it 'saves assignee when user id is valid' do project.add_maintainer(assignee) - opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } merge_request = described_class.new(project, user, opts).execute - expect(merge_request.assignee).to eq(assignee) + expect(merge_request.assignees).to eq([assignee]) end context 'when assignee is set' do @@ -361,7 +361,7 @@ describe MergeRequests::CreateService do { title: 'Title', description: 'Description', - assignee_id: assignee.id, + assignee_ids: [assignee.id], source_branch: 'feature', target_branch: 'master' } @@ -387,7 +387,7 @@ describe MergeRequests::CreateService do levels.each do |level| it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do project.update(visibility_level: level) - opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } merge_request = described_class.new(project, user, opts).execute diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index 1430e12a07e..a87d8b8752c 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -7,7 +7,7 @@ describe MergeRequests::FfMergeService do create(:merge_request, source_branch: 'flatten-dir', target_branch: 'improve/awesome', - assignee: user2, + assignees: [user2], author: create(:user)) end let(:project) { merge_request.project } diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 887ec17171e..b0b3273e3dc 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::MergeService do set(:user) { create(:user) } set(:user2) { create(:user) } - let(:merge_request) { create(:merge_request, :simple, author: user2, assignee: user2) } + let(:merge_request) { create(:merge_request, :simple, author: user2, assignees: [user2]) } let(:project) { merge_request.project } before do @@ -111,7 +111,7 @@ describe MergeRequests::MergeService do end context 'closes related todos' do - let(:merge_request) { create(:merge_request, assignee: user, author: user) } + let(:merge_request) { create(:merge_request, assignees: [user], author: user) } let(:project) { merge_request.project } let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } let!(:todo) do diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index a3b48abae26..24d09c1fd00 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -149,7 +149,7 @@ describe MergeRequests::MergeToRefService do end context 'does not close related todos' do - let(:merge_request) { create(:merge_request, assignee: user, author: user) } + let(:merge_request) { create(:merge_request, assignees: [user], author: user) } let(:project) { merge_request.project } let!(:todo) do create(:todo, :assigned, diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 5ad6f5528f9..2cebefee5d6 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe MergeRequests::PostMergeService do let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, assignee: user) } + let(:merge_request) { create(:merge_request, assignees: [user]) } let(:project) { merge_request.project } before do diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 21e71509ed6..8b6db1ce33e 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -4,7 +4,7 @@ describe MergeRequests::ReopenService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:merge_request) { create(:merge_request, :closed, assignee: user2, author: create(:user)) } + let(:merge_request) { create(:merge_request, :closed, assignees: [user2], author: create(:user)) } let(:project) { merge_request.project } before do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 8e367db031c..0525899ebfa 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -13,7 +13,7 @@ describe MergeRequests::UpdateService, :mailer do let(:merge_request) do create(:merge_request, :simple, title: 'Old title', description: "FYI #{user2.to_reference}", - assignee_id: user3.id, + assignee_ids: [user3.id], source_project: project, author: create(:user)) end @@ -48,7 +48,7 @@ describe MergeRequests::UpdateService, :mailer do { title: 'New title', description: 'Also please fix', - assignee_id: user2.id, + assignee_ids: [user.id], state_event: 'close', label_ids: [label.id], target_branch: 'target', @@ -71,7 +71,7 @@ describe MergeRequests::UpdateService, :mailer do it 'matches base expectations' do expect(@merge_request).to be_valid expect(@merge_request.title).to eq('New title') - expect(@merge_request.assignee).to eq(user2) + expect(@merge_request.assignees).to match_array([user]) expect(@merge_request).to be_closed expect(@merge_request.labels.count).to eq(1) expect(@merge_request.labels.first.title).to eq(label.name) @@ -106,7 +106,7 @@ describe MergeRequests::UpdateService, :mailer do note = find_note('assigned to') expect(note).not_to be_nil - expect(note.note).to include "assigned to #{user2.to_reference}" + expect(note.note).to include "assigned to #{user.to_reference} and unassigned #{user3.to_reference}" end it 'creates a resource label event' do @@ -293,7 +293,7 @@ describe MergeRequests::UpdateService, :mailer do context 'when is reassigned' do before do - update_merge_request({ assignee: user2 }) + update_merge_request({ assignee_ids: [user2.id] }) end it 'marks previous assignee pending todos as done' do @@ -387,7 +387,7 @@ describe MergeRequests::UpdateService, :mailer do context 'when the assignee changes' do it 'updates open merge request counter for assignees when merge request is reassigned' do - update_merge_request(assignee_id: user2.id) + update_merge_request(assignee_ids: [user2.id]) expect(user3.assigned_open_merge_requests_count).to eq 0 expect(user2.assigned_open_merge_requests_count).to eq 1 @@ -541,36 +541,36 @@ describe MergeRequests::UpdateService, :mailer do end end - context 'updating asssignee_id' do + context 'updating asssignee_ids' do it 'does not update assignee when assignee_id is invalid' do - merge_request.update(assignee_id: user.id) + merge_request.update(assignee_ids: [user.id]) - update_merge_request(assignee_id: -1) + update_merge_request(assignee_ids: [-1]) - expect(merge_request.reload.assignee).to eq(user) + expect(merge_request.reload.assignees).to eq([user]) end it 'unassigns assignee when user id is 0' do - merge_request.update(assignee_id: user.id) + merge_request.update(assignee_ids: [user.id]) - update_merge_request(assignee_id: 0) + update_merge_request(assignee_ids: [0]) - expect(merge_request.assignee_id).to be_nil + expect(merge_request.assignee_ids).to be_empty end it 'saves assignee when user id is valid' do - update_merge_request(assignee_id: user.id) + update_merge_request(assignee_ids: [user.id]) - expect(merge_request.assignee_id).to eq(user.id) + expect(merge_request.assignee_ids).to eq([user.id]) end it 'does not update assignee_id when user cannot read issue' do - non_member = create(:user) - original_assignee = merge_request.assignee + non_member = create(:user) + original_assignees = merge_request.assignees - update_merge_request(assignee_id: non_member.id) + update_merge_request(assignee_ids: [non_member.id]) - expect(merge_request.assignee_id).to eq(original_assignee.id) + expect(merge_request.reload.assignees).to eq(original_assignees) end context "when issuable feature is private" do @@ -583,7 +583,7 @@ describe MergeRequests::UpdateService, :mailer do feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level" project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - expect { update_merge_request(assignee_id: assignee) }.not_to change { merge_request.assignee } + expect { update_merge_request(assignee_ids: [assignee]) }.not_to change { merge_request.reload.assignees } end end end @@ -619,7 +619,7 @@ describe MergeRequests::UpdateService, :mailer do end it 'is allowed by a user that can push to the source and can update the merge request' do - merge_request.update!(assignee: user) + merge_request.update!(assignees: [user]) source_project.add_developer(user) update_merge_request(allow_collaboration: true, title: 'Updated title') diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 9ba4a11104a..14c73852e65 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -125,11 +125,7 @@ describe NotificationService, :mailer do shared_examples 'participating by assignee notification' do it 'emails the participant' do - if issuable.is_a?(Issue) - issuable.assignees << participant - else - issuable.update_attribute(:assignee, participant) - end + issuable.assignees << participant notification_trigger @@ -620,13 +616,13 @@ describe NotificationService, :mailer do context "merge request diff note" do let(:project) { create(:project, :repository) } let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, source_project: project, assignee: user, author: create(:user)) } + let(:merge_request) { create(:merge_request, source_project: project, assignees: [user], author: create(:user)) } let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } before do build_team(note.project) project.add_maintainer(merge_request.author) - project.add_maintainer(merge_request.assignee) + merge_request.assignees.each { |assignee| project.add_maintainer(assignee) } end describe '#new_note' do @@ -637,7 +633,7 @@ describe NotificationService, :mailer do notification.new_note(note) expect(SentNotification.last(3).map(&:recipient).map(&:id)) - .to contain_exactly(merge_request.assignee.id, merge_request.author.id, @u_watcher.id) + .to contain_exactly(*merge_request.assignees.pluck(:id), merge_request.author.id, @u_watcher.id) expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.discussion_id) end end @@ -1223,11 +1219,12 @@ describe NotificationService, :mailer do let(:group) { create(:group) } let(:project) { create(:project, :public, :repository, namespace: group) } let(:another_project) { create(:project, :public, namespace: group) } - let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' } + let(:assignee) { create(:user) } + let(:merge_request) { create :merge_request, source_project: project, assignees: [assignee], description: 'cc @participant' } before do project.add_maintainer(merge_request.author) - project.add_maintainer(merge_request.assignee) + merge_request.assignees.each { |assignee| project.add_maintainer(assignee) } build_team(merge_request.target_project) add_users_with_subscription(merge_request.target_project, merge_request) update_custom_notification(:new_merge_request, @u_guest_custom, resource: project) @@ -1239,7 +1236,7 @@ describe NotificationService, :mailer do it do notification.new_merge_request(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_watcher) should_email(@watcher_and_subscriber) should_email(@u_participant_mentioned) @@ -1254,9 +1251,11 @@ describe NotificationService, :mailer do it 'adds "assigned" reason for assignee, if any' do notification.new_merge_request(merge_request, @u_disabled) - email = find_email_for(merge_request.assignee) + merge_request.assignees.each do |assignee| + email = find_email_for(assignee) - expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) + expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) + end end it "emails any mentioned users with the mention level" do @@ -1347,9 +1346,9 @@ describe NotificationService, :mailer do end it do - notification.reassigned_merge_request(merge_request, current_user, merge_request.author) + notification.reassigned_merge_request(merge_request, current_user, [assignee]) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(merge_request.author) should_email(@u_watcher) should_email(@u_participant_mentioned) @@ -1365,17 +1364,19 @@ describe NotificationService, :mailer do end it 'adds "assigned" reason for new assignee' do - notification.reassigned_merge_request(merge_request, current_user, merge_request.author) + notification.reassigned_merge_request(merge_request, current_user, [assignee]) - email = find_email_for(merge_request.assignee) + merge_request.assignees.each do |assignee| + email = find_email_for(assignee) - expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) + expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) + end end it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { merge_request } - let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, merge_request.author) } + let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) } end end @@ -1388,7 +1389,7 @@ describe NotificationService, :mailer do it do notification.push_to_merge_request(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_guest_custom) should_email(@u_custom_global) should_email(@u_participant_mentioned) @@ -1430,7 +1431,7 @@ describe NotificationService, :mailer do should_email(subscriber_1_to_group_label_2) should_email(subscriber_2_to_group_label_2) should_email(subscriber_to_label_2) - should_not_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_not_email(assignee) } should_not_email(merge_request.author) should_not_email(@u_watcher) should_not_email(@u_participant_mentioned) @@ -1499,7 +1500,7 @@ describe NotificationService, :mailer do it do notification.close_mr(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -1529,7 +1530,7 @@ describe NotificationService, :mailer do it do notification.merge_mr(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -1581,7 +1582,7 @@ describe NotificationService, :mailer do it do notification.reopen_mr(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_watcher) should_email(@u_participant_mentioned) should_email(@subscriber) @@ -1606,7 +1607,7 @@ describe NotificationService, :mailer do it do notification.resolve_all_discussions(merge_request, @u_disabled) - should_email(merge_request.assignee) + merge_request.assignees.each { |assignee| should_email(assignee) } should_email(@u_watcher) should_email(@u_participant_mentioned) should_email(@subscriber) @@ -1850,8 +1851,8 @@ describe NotificationService, :mailer do let(:guest) { create(:user) } let(:developer) { create(:user) } let(:assignee) { create(:user) } - let(:merge_request) { create(:merge_request, source_project: private_project, assignee: assignee) } - let(:merge_request1) { create(:merge_request, source_project: private_project, assignee: assignee, description: "cc @#{guest.username}") } + let(:merge_request) { create(:merge_request, source_project: private_project, assignees: [assignee]) } + let(:merge_request1) { create(:merge_request, source_project: private_project, assignees: [assignee], description: "cc @#{guest.username}") } let(:note) { create(:note, noteable: merge_request, project: private_project) } before do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index c7e5cca324f..c450f89c3cb 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -16,7 +16,9 @@ describe QuickActions::InterpretService do let(:service) { described_class.new(project, developer) } before do - stub_licensed_features(multiple_issue_assignees: false) + stub_licensed_features(multiple_issue_assignees: false, + multiple_merge_request_assignees: false) + project.add_developer(developer) end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 13d7d795703..51c5a803dbd 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -166,8 +166,8 @@ describe SystemNoteService do end end - describe '.change_issue_assignees' do - subject { described_class.change_issue_assignees(noteable, project, author, [assignee]) } + describe '.change_issuable_assignees' do + subject { described_class.change_issuable_assignees(noteable, project, author, [assignee]) } let(:assignee) { create(:user) } let(:assignee1) { create(:user) } @@ -180,7 +180,7 @@ describe SystemNoteService do def build_note(old_assignees, new_assignees) issue.assignees = new_assignees - described_class.change_issue_assignees(issue, project, author, old_assignees).note + described_class.change_issuable_assignees(issue, project, author, old_assignees).note end it_behaves_like 'a note with overridable created_at' diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 8631f3f9a33..89411b2e908 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -272,28 +272,6 @@ describe TodoService do end end - describe '#reassigned_issue' do - it 'creates a pending todo for new assignee' do - unassigned_issue.assignees << john_doe - service.reassigned_issue(unassigned_issue, author) - - should_create_todo(user: john_doe, target: unassigned_issue, action: Todo::ASSIGNED) - end - - it 'does not create a todo if unassigned' do - issue.assignees.destroy_all # rubocop: disable DestroyAll - - should_not_create_any_todo { service.reassigned_issue(issue, author) } - end - - it 'creates a todo if new assignee is the current user' do - unassigned_issue.assignees << john_doe - service.reassigned_issue(unassigned_issue, john_doe) - - should_create_todo(user: john_doe, target: unassigned_issue, author: john_doe, action: Todo::ASSIGNED) - end - end - describe '#mark_pending_todos_as_done' do it 'marks related pending todos to the target for the user as done' do first_todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) @@ -504,10 +482,60 @@ describe TodoService do end end + describe '#reassigned_issuable' do + shared_examples 'reassigned issuable' do + it 'creates a pending todo for new assignee' do + issuable_unassigned.assignees = [john_doe] + service.reassigned_issuable(issuable_unassigned, author) + + should_create_todo(user: john_doe, target: issuable_unassigned, action: Todo::ASSIGNED) + end + + it 'does not create a todo if unassigned' do + issuable_assigned.assignees = [] + + should_not_create_any_todo { service.reassigned_issuable(issuable_assigned, author) } + end + + it 'creates a todo if new assignee is the current user' do + issuable_assigned.assignees = [john_doe] + service.reassigned_issuable(issuable_assigned, john_doe) + + should_create_todo(user: john_doe, target: issuable_assigned, author: john_doe, action: Todo::ASSIGNED) + end + + it 'does not create a todo for guests' do + service.reassigned_issuable(issuable_assigned, author) + should_not_create_todo(user: guest, target: issuable_assigned, action: Todo::MENTIONED) + end + + it 'does not create a directly addressed todo for guests' do + service.reassigned_issuable(addressed_issuable_assigned, author) + should_not_create_todo(user: guest, target: addressed_issuable_assigned, action: Todo::DIRECTLY_ADDRESSED) + end + end + + context 'issuable is a merge request' do + it_behaves_like 'reassigned issuable' do + let(:issuable_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_issuable_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } + let(:issuable_unassigned) { create(:merge_request, source_project: project, author: author, assignees: []) } + end + end + + context 'issuable is an issue' do + it_behaves_like 'reassigned issuable' do + let(:issuable_assigned) { create(:issue, project: project, author: author, assignees: [john_doe], description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_issuable_assigned) { create(:issue, project: project, author: author, assignees: [john_doe], description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } + let(:issuable_unassigned) { create(:issue, project: project, author: author, assignees: []) } + end + end + end + describe 'Merge Requests' do - let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } - let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } - let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) } + let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } + let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignees: []) } describe '#new_merge_request' do it 'creates a pending todo if assigned' do @@ -659,38 +687,6 @@ describe TodoService do end end - describe '#reassigned_merge_request' do - it 'creates a pending todo for new assignee' do - mr_unassigned.update_attribute(:assignee, john_doe) - service.reassigned_merge_request(mr_unassigned, author) - - should_create_todo(user: john_doe, target: mr_unassigned, action: Todo::ASSIGNED) - end - - it 'does not create a todo if unassigned' do - mr_assigned.update_attribute(:assignee, nil) - - should_not_create_any_todo { service.reassigned_merge_request(mr_assigned, author) } - end - - it 'creates a todo if new assignee is the current user' do - mr_assigned.update_attribute(:assignee, john_doe) - service.reassigned_merge_request(mr_assigned, john_doe) - - should_create_todo(user: john_doe, target: mr_assigned, author: john_doe, action: Todo::ASSIGNED) - end - - it 'does not create a todo for guests' do - service.reassigned_merge_request(mr_assigned, author) - should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) - end - - it 'does not create a directly addressed todo for guests' do - service.reassigned_merge_request(addressed_mr_assigned, author) - should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) - end - end - describe '#merge_merge_request' do it 'marks related pending todos to the target for the user as done' do first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author) diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 83f1495a1c6..450e76d5f58 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -78,7 +78,7 @@ describe Users::DestroyService do end context "for an merge request the user was assigned to" do - let!(:merge_request) { create(:merge_request, source_project: project, assignee: user) } + let!(:merge_request) { create(:merge_request, source_project: project, assignees: [user]) } before do service.execute(user) @@ -91,7 +91,7 @@ describe Users::DestroyService do it 'migrates the merge request so that it is "Unassigned"' do migrated_merge_request = MergeRequest.find_by_id(merge_request.id) - expect(migrated_merge_request.assignee).to be_nil + expect(migrated_merge_request.assignees).to be_empty end end end |