diff options
| author | Valery Sizov <valery@gitlab.com> | 2017-05-04 15:11:15 +0300 |
|---|---|---|
| committer | Valery Sizov <valery@gitlab.com> | 2017-05-04 17:11:53 +0300 |
| commit | 387c4b2c21a44360386a9b8ce6849e7f1b8a3de9 (patch) | |
| tree | 446b8338efe8ad22ca03b00b2dc72b22c4174e02 /spec | |
| parent | 68c12e15cc236548918f91393ebef3c06c124814 (diff) | |
| download | gitlab-ce-387c4b2c21a44360386a9b8ce6849e7f1b8a3de9.tar.gz | |
Backport of multiple_assignees_feature [ci skip]
Diffstat (limited to 'spec')
71 files changed, 1351 insertions, 699 deletions
diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index 762e90f4a16..085f3fd8543 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -14,7 +14,7 @@ describe Dashboard::TodosController do describe 'GET #index' do context 'when using pagination' do let(:last_page) { user.todos.page.total_pages } - let!(:issues) { create_list(:issue, 2, project: project, assignee: user) } + let!(:issues) { create_list(:issue, 2, project: project, assignees: [user]) } before do issues.each { |issue| todo_service.new_issue(issue, user) } diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index 15667e8d4b1..dc3b72c6de4 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -34,7 +34,7 @@ describe Projects::Boards::IssuesController do issue = create(:labeled_issue, project: project, labels: [planning]) create(:labeled_issue, project: project, labels: [planning]) create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow) - create(:labeled_issue, project: project, labels: [development], assignee: johndoe) + create(:labeled_issue, project: project, labels: [development], assignees: [johndoe]) issue.subscribe(johndoe, project) list_issues user: user, board: board, list: list2 diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 5f1f892821a..1f79e72495a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -173,12 +173,12 @@ describe Projects::IssuesController do namespace_id: project.namespace.to_param, project_id: project, id: issue.iid, - issue: { assignee_id: assignee.id }, + issue: { assignee_ids: [assignee.id] }, format: :json body = JSON.parse(response.body) - expect(body['assignee'].keys) - .to match_array(%w(name username avatar_url)) + expect(body['assignees'].first.keys) + .to match_array(%w(id name username avatar_url)) end end @@ -348,7 +348,7 @@ describe Projects::IssuesController do let(:admin) { create(:admin) } let!(:issue) { create(:issue, project: project) } let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) } - let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignee: assignee) } + let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignees: [assignee]) } describe 'GET #index' do it 'does not list confidential issues for guests' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a793da4162a..0483c6b7879 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1067,7 +1067,7 @@ describe Projects::MergeRequestsController do end it 'correctly pluralizes flash message on success' do - issue2.update!(assignee: user) + issue2.assignees = [user] post_assign_issues diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb index 58b14e09740..9ea325ab41b 100644 --- a/spec/features/atom/dashboard_issues_spec.rb +++ b/spec/features/atom/dashboard_issues_spec.rb @@ -32,7 +32,7 @@ describe "Dashboard Issues Feed", feature: true do end context "issue with basic fields" do - let!(:issue2) { create(:issue, author: user, assignee: assignee, project: project2, description: 'test desc') } + let!(:issue2) { create(:issue, author: user, assignees: [assignee], project: project2, description: 'test desc') } it "renders issue fields" do visit issues_dashboard_path(:atom, private_token: user.private_token) @@ -41,7 +41,7 @@ describe "Dashboard Issues Feed", feature: true do expect(entry).to be_present expect(entry).to have_selector('author email', text: issue2.author_public_email) - expect(entry).to have_selector('assignee email', text: issue2.assignee_public_email) + expect(entry).to have_selector('assignees email', text: assignee.public_email) expect(entry).not_to have_selector('labels') expect(entry).not_to have_selector('milestone') expect(entry).to have_selector('description', text: issue2.description) @@ -51,7 +51,7 @@ describe "Dashboard Issues Feed", feature: true do context "issue with label and milestone" do let!(:milestone1) { create(:milestone, project: project1, title: 'v1') } let!(:label1) { create(:label, project: project1, title: 'label1') } - let!(:issue1) { create(:issue, author: user, assignee: assignee, project: project1, milestone: milestone1) } + let!(:issue1) { create(:issue, author: user, assignees: [assignee], project: project1, milestone: milestone1) } before do issue1.labels << label1 @@ -64,7 +64,7 @@ describe "Dashboard Issues Feed", feature: true do expect(entry).to be_present expect(entry).to have_selector('author email', text: issue1.author_public_email) - expect(entry).to have_selector('assignee email', text: issue1.assignee_public_email) + expect(entry).to have_selector('assignees email', text: assignee.public_email) expect(entry).to have_selector('labels label', text: label1.title) expect(entry).to have_selector('milestone', text: milestone1.title) expect(entry).not_to have_selector('description') diff --git a/spec/features/atom/issues_spec.rb b/spec/features/atom/issues_spec.rb index b3903ec2faf..78f8f46a04e 100644 --- a/spec/features/atom/issues_spec.rb +++ b/spec/features/atom/issues_spec.rb @@ -6,7 +6,7 @@ describe 'Issues Feed', feature: true do let!(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } let!(:group) { create(:group) } let!(:project) { create(:project) } - let!(:issue) { create(:issue, author: user, assignee: assignee, project: project) } + let!(:issue) { create(:issue, author: user, assignees: [assignee], project: project) } before do project.team << [user, :developer] @@ -22,7 +22,7 @@ describe 'Issues Feed', feature: true do to have_content('application/atom+xml') expect(body).to have_selector('title', text: "#{project.name} issues") expect(body).to have_selector('author email', text: issue.author_public_email) - expect(body).to have_selector('assignee email', text: issue.author_public_email) + expect(body).to have_selector('assignees email', text: issue.author_public_email) expect(body).to have_selector('entry summary', text: issue.title) end end @@ -36,7 +36,7 @@ describe 'Issues Feed', feature: true do to have_content('application/atom+xml') expect(body).to have_selector('title', text: "#{project.name} issues") expect(body).to have_selector('author email', text: issue.author_public_email) - expect(body).to have_selector('assignee email', text: issue.author_public_email) + expect(body).to have_selector('assignees email', text: issue.author_public_email) expect(body).to have_selector('entry summary', text: issue.title) end end diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index a172ce1e8c0..18585488e26 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -71,7 +71,7 @@ describe 'Issue Boards', feature: true, js: true do let!(:list2) { create(:list, board: board, label: development, position: 1) } let!(:confidential_issue) { create(:labeled_issue, :confidential, project: project, author: user, labels: [planning], relative_position: 9) } - let!(:issue1) { create(:labeled_issue, project: project, assignee: user, labels: [planning], relative_position: 8) } + let!(:issue1) { create(:labeled_issue, project: project, assignees: [user], labels: [planning], relative_position: 8) } let!(:issue2) { create(:labeled_issue, project: project, author: user2, labels: [planning], relative_position: 7) } let!(:issue3) { create(:labeled_issue, project: project, labels: [planning], relative_position: 6) } let!(:issue4) { create(:labeled_issue, project: project, labels: [planning], relative_position: 5) } diff --git a/spec/features/boards/modal_filter_spec.rb b/spec/features/boards/modal_filter_spec.rb index 4a4c13e79c8..e1367c675e5 100644 --- a/spec/features/boards/modal_filter_spec.rb +++ b/spec/features/boards/modal_filter_spec.rb @@ -98,7 +98,7 @@ describe 'Issue Boards add issue modal filtering', :feature, :js do end context 'assignee' do - let!(:issue) { create(:issue, project: project, assignee: user2) } + let!(:issue) { create(:issue, project: project, assignees: [user2]) } before do project.team << [user2, :developer] diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index bafa4f05937..02b6b5dc888 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -4,13 +4,14 @@ describe 'Issue Boards', feature: true, js: true do include WaitForVueResource let(:user) { create(:user) } + let(:user2) { create(:user) } let(:project) { create(:empty_project, :public) } let!(:milestone) { create(:milestone, project: project) } let!(:development) { create(:label, project: project, name: 'Development') } let!(:bug) { create(:label, project: project, name: 'Bug') } let!(:regression) { create(:label, project: project, name: 'Regression') } let!(:stretch) { create(:label, project: project, name: 'Stretch') } - let!(:issue1) { create(:labeled_issue, project: project, assignee: user, milestone: milestone, labels: [development], relative_position: 2) } + let!(:issue1) { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], relative_position: 2) } let!(:issue2) { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) } let(:board) { create(:board, project: project) } let!(:list) { create(:list, board: board, label: development, position: 0) } @@ -20,6 +21,7 @@ describe 'Issue Boards', feature: true, js: true do Timecop.freeze project.team << [user, :master] + project.team.add_developer(user2) login_as(user) @@ -101,6 +103,26 @@ describe 'Issue Boards', feature: true, js: true do expect(card).to have_selector('.avatar') end + it 'adds multiple assignees' do + click_card(card) + + page.within('.assignee') do + click_link 'Edit' + + wait_for_ajax + + page.within('.dropdown-menu-user') do + click_link user.name + click_link user2.name + end + + expect(page).to have_content(user.name) + expect(page).to have_content(user2.name) + end + + expect(card.all('.avatar').length).to eq(2) + end + it 'removes the assignee' do card_two = first('.board').find('.card:nth-child(2)') click_card(card_two) @@ -112,10 +134,11 @@ describe 'Issue Boards', feature: true, js: true do page.within('.dropdown-menu-user') do click_link 'Unassigned' - - wait_for_vue_resource end + find('.dropdown-menu-toggle').click + wait_for_vue_resource + expect(page).to have_content('No assignee') end @@ -128,7 +151,7 @@ describe 'Issue Boards', feature: true, js: true do page.within(find('.assignee')) do expect(page).to have_content('No assignee') - click_link 'assign yourself' + click_button 'assign yourself' wait_for_vue_resource @@ -138,7 +161,7 @@ describe 'Issue Boards', feature: true, js: true do expect(card).to have_selector('.avatar') end - it 'resets assignee dropdown' do + it 'updates assignee dropdown' do click_card(card) page.within('.assignee') do @@ -162,7 +185,7 @@ describe 'Issue Boards', feature: true, js: true do page.within('.assignee') do click_link 'Edit' - expect(page).not_to have_selector('.is-active') + expect(page).to have_selector('.is-active') end end end diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index 4fca7577e74..3d536c5ba40 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -7,7 +7,7 @@ describe 'Navigation bar counter', feature: true, caching: true do let(:merge_request) { create(:merge_request, source_project: project) } before do - issue.update(assignee: user) + issue.assignees = [user] merge_request.update(assignee: user) login_as(user) end @@ -17,7 +17,7 @@ describe 'Navigation bar counter', feature: true, caching: true do expect_counters('issues', '1') - issue.update(assignee: nil) + issue.assignees = [] Timecop.travel(3.minutes.from_now) do visit issues_path diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index f4420814c3a..86c7954e60c 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -11,7 +11,7 @@ RSpec.describe 'Dashboard Issues', feature: true do let!(:authored_issue) { create :issue, author: current_user, project: project } let!(:authored_issue_on_public_project) { create :issue, author: current_user, project: public_project } - let!(:assigned_issue) { create :issue, assignee: current_user, project: project } + let!(:assigned_issue) { create :issue, assignees: [current_user], project: project } let!(:other_issue) { create :issue, project: project } before do @@ -30,6 +30,11 @@ RSpec.describe 'Dashboard Issues', feature: true do find('#assignee_id', visible: false).set('') find('.js-author-search', match: :first).click find('.dropdown-menu-author li a', match: :first, text: current_user.to_reference).click + find('.js-author-search', match: :first).click + + page.within '.dropdown-menu-user' do + expect(find('.dropdown-menu-author li a.is-active', match: :first, text: current_user.to_reference)).to be_visible + end expect(page).to have_content(authored_issue.title) expect(page).to have_content(authored_issue_on_public_project.title) diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index b6b87905231..ad60fb2c74f 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/spec/features/dashboard_issues_spec.rb @@ -10,8 +10,8 @@ describe "Dashboard Issues filtering", feature: true, js: true do project.team << [user, :master] login_as(user) - create(:issue, project: project, author: user, assignee: user) - create(:issue, project: project, author: user, assignee: user, milestone: milestone) + create(:issue, project: project, author: user, assignees: [user]) + create(:issue, project: project, author: user, assignees: [user], milestone: milestone) visit_issues end diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index f5b54463df8..005a029a393 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -54,11 +54,11 @@ describe "GitLab Flavored Markdown", feature: true do before do @other_issue = create(:issue, author: @user, - assignee: @user, + assignees: [@user], project: project) @issue = create(:issue, author: @user, - assignee: @user, + assignees: [@user], project: project, title: "fix #{@other_issue.to_reference}", description: "ask #{fred.to_reference} for details") diff --git a/spec/features/issues/award_emoji_spec.rb b/spec/features/issues/award_emoji_spec.rb index 71df3c949db..853632614c4 100644 --- a/spec/features/issues/award_emoji_spec.rb +++ b/spec/features/issues/award_emoji_spec.rb @@ -7,7 +7,7 @@ describe 'Awards Emoji', feature: true do let!(:user) { create(:user) } let(:issue) do create(:issue, - assignee: @user, + assignees: [user], project: project) end diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index c824aa6a414..ece62c8da41 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -51,15 +51,16 @@ describe 'Filter issues', js: true, feature: true do create(:issue, project: project, title: "issue with 'single quotes'") create(:issue, project: project, title: "issue with \"double quotes\"") create(:issue, project: project, title: "issue with !@\#{$%^&*()-+") - create(:issue, project: project, title: "issue by assignee", milestone: milestone, author: user, assignee: user) - create(:issue, project: project, title: "issue by assignee with searchTerm", milestone: milestone, author: user, assignee: user) + create(:issue, project: project, title: "issue by assignee", milestone: milestone, author: user, assignees: [user]) + create(:issue, project: project, title: "issue by assignee with searchTerm", milestone: milestone, author: user, assignees: [user]) + issue = create(:issue, title: "Bug 2", project: project, milestone: milestone, author: user, - assignee: user) + assignees: [user]) issue.labels << bug_label issue_with_caps_label = create(:issue, @@ -67,7 +68,7 @@ describe 'Filter issues', js: true, feature: true do project: project, milestone: milestone, author: user, - assignee: user) + assignees: [user]) issue_with_caps_label.labels << caps_sensitive_label issue_with_everything = create(:issue, @@ -75,7 +76,7 @@ describe 'Filter issues', js: true, feature: true do project: project, milestone: milestone, author: user, - assignee: user) + assignees: [user]) issue_with_everything.labels << bug_label issue_with_everything.labels << caps_sensitive_label diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 21b8cf3add5..5798292033b 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -10,7 +10,7 @@ describe 'New/edit issue', feature: true, js: true do let!(:milestone) { create(:milestone, project: project) } let!(:label) { create(:label, project: project) } let!(:label2) { create(:label, project: project) } - let!(:issue) { create(:issue, project: project, assignee: user, milestone: milestone) } + let!(:issue) { create(:issue, project: project, assignees: [user], milestone: milestone) } before do project.team << [user, :master] @@ -23,25 +23,65 @@ describe 'New/edit issue', feature: true, js: true do visit new_namespace_project_issue_path(project.namespace, project) end + describe 'multiple assignees' do + before do + click_button 'Unassigned' + end + + it 'unselects other assignees when unassigned is selected' do + page.within '.dropdown-menu-user' do + click_link user2.name + end + + page.within '.dropdown-menu-user' do + click_link 'Unassigned' + end + + page.within '.js-assignee-search' do + expect(page).to have_content 'Unassigned' + end + + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match('0') + end + + it 'toggles assign to me when current user is selected and unselected' do + page.within '.dropdown-menu-user' do + click_link user.name + end + + expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible + + page.within '.dropdown-menu-user' do + click_link user.name + end + + expect(find('a', text: 'Assign to me')).to be_visible + end + end + it 'allows user to create new issue' do fill_in 'issue_title', with: 'title' fill_in 'issue_description', with: 'title' expect(find('a', text: 'Assign to me')).to be_visible - click_button 'Assignee' + click_button 'Unassigned' page.within '.dropdown-menu-user' do click_link user2.name end - expect(find('input[name="issue[assignee_id]"]', visible: false).value).to match(user2.id.to_s) + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s) page.within '.js-assignee-search' do expect(page).to have_content user2.name end expect(find('a', text: 'Assign to me')).to be_visible click_link 'Assign to me' - expect(find('input[name="issue[assignee_id]"]', visible: false).value).to match(user.id.to_s) + assignee_ids = page.all('input[name="issue[assignee_ids][]"]', visible: false) + + expect(assignee_ids[0].value).to match(user2.id.to_s) + expect(assignee_ids[1].value).to match(user.id.to_s) + page.within '.js-assignee-search' do - expect(page).to have_content user.name + expect(page).to have_content "#{user2.name} + 1 more" end expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible @@ -69,7 +109,7 @@ describe 'New/edit issue', feature: true, js: true do page.within '.issuable-sidebar' do page.within '.assignee' do - expect(page).to have_content user.name + expect(page).to have_content "2 Assignees" end page.within '.milestone' do @@ -141,7 +181,7 @@ describe 'New/edit issue', feature: true, js: true do end it 'allows user to update issue' do - expect(find('input[name="issue[assignee_id]"]', visible: false).value).to match(user.id.to_s) + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s) expect(find('input[name="issue[milestone_id]"]', visible: false).value).to match(milestone.id.to_s) expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible diff --git a/spec/features/issues/issue_sidebar_spec.rb b/spec/features/issues/issue_sidebar_spec.rb index 82b80a69bed..e9a05f56543 100644 --- a/spec/features/issues/issue_sidebar_spec.rb +++ b/spec/features/issues/issue_sidebar_spec.rb @@ -42,6 +42,21 @@ feature 'Issue Sidebar', feature: true do expect(page).to have_content(user2.name) end end + + it 'assigns yourself' do + find('.block.assignee .dropdown-menu-toggle').click + + click_button 'assign yourself' + + wait_for_ajax + + find('.block.assignee .edit-link').click + + page.within '.dropdown-menu-user' do + expect(page.find('.dropdown-header')).to be_visible + expect(page.find('.dropdown-menu-user-link.is-active')).to have_content(user.name) + end + end end context 'as a allowed user' do diff --git a/spec/features/issues/update_issues_spec.rb b/spec/features/issues/update_issues_spec.rb index 7fa83c1fcf7..b250fa2ed3c 100644 --- a/spec/features/issues/update_issues_spec.rb +++ b/spec/features/issues/update_issues_spec.rb @@ -99,7 +99,7 @@ feature 'Multiple issue updating from issues#index', feature: true do end def create_assigned - create(:issue, project: project, assignee: user) + create(:issue, project: project, assignees: [user]) end def create_with_milestone diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 81cc8513454..cc81303f032 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -18,7 +18,7 @@ describe 'Issues', feature: true do let!(:issue) do create(:issue, author: @user, - assignee: @user, + assignees: [@user], project: project) end @@ -43,7 +43,7 @@ describe 'Issues', feature: true do let!(:issue) do create(:issue, author: @user, - assignee: @user, + assignees: [@user], project: project) end @@ -61,7 +61,7 @@ describe 'Issues', feature: true do expect(page).to have_content 'No assignee - assign yourself' end - expect(issue.reload.assignee).to be_nil + expect(issue.reload.assignees).to be_empty end end @@ -138,7 +138,7 @@ describe 'Issues', feature: true do describe 'Issue info' do it 'excludes award_emoji from comment count' do - issue = create(:issue, author: @user, assignee: @user, project: project, title: 'foobar') + issue = create(:issue, author: @user, assignees: [@user], project: project, title: 'foobar') create(:award_emoji, awardable: issue) visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id) @@ -153,14 +153,14 @@ describe 'Issues', feature: true do %w(foobar barbaz gitlab).each do |title| create(:issue, author: @user, - assignee: @user, + assignees: [@user], project: project, title: title) end @issue = Issue.find_by(title: 'foobar') @issue.milestone = create(:milestone, project: project) - @issue.assignee = nil + @issue.assignees = [] @issue.save end @@ -351,9 +351,9 @@ describe 'Issues', feature: true do let(:user2) { create(:user) } before do - foo.assignee = user2 + foo.assignees << user2 foo.save - bar.assignee = user2 + bar.assignees << user2 bar.save end @@ -396,7 +396,7 @@ describe 'Issues', feature: true do end describe 'update labels from issue#show', js: true do - let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } + let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) } let!(:label) { create(:label, project: project) } before do @@ -415,7 +415,7 @@ describe 'Issues', feature: true do end describe 'update assignee from issue#show' do - let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } + let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) } context 'by authorized user' do it 'allows user to select unassigned', js: true do @@ -426,10 +426,14 @@ describe 'Issues', feature: true do click_link 'Edit' click_link 'Unassigned' + first('.title').click expect(page).to have_content 'No assignee' end - expect(issue.reload.assignee).to be_nil + # wait_for_ajax does not work with vue-resource at the moment + sleep 1 + + expect(issue.reload.assignees).to be_empty end it 'allows user to select an assignee', js: true do @@ -461,14 +465,18 @@ describe 'Issues', feature: true do click_link 'Edit' click_link @user.name - page.within '.value' do + find('.dropdown-menu-toggle').click + + page.within '.value .author' do expect(page).to have_content @user.name end click_link 'Edit' click_link @user.name - page.within '.value' do + find('.dropdown-menu-toggle').click + + page.within '.value .assign-yourself' do expect(page).to have_content "No assignee" end end @@ -487,7 +495,7 @@ describe 'Issues', feature: true do login_with guest visit namespace_project_issue_path(project.namespace, project, issue) - expect(page).to have_content issue.assignee.name + expect(page).to have_content issue.assignees.first.name end end end @@ -558,7 +566,7 @@ describe 'Issues', feature: true do let(:user2) { create(:user) } before do - issue.assignee = user2 + issue.assignees << user2 issue.save end end @@ -655,7 +663,7 @@ describe 'Issues', feature: true do describe 'due date' do context 'update due on issue#show', js: true do - let(:issue) { create(:issue, project: project, author: @user, assignee: @user) } + let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) } before do visit namespace_project_issue_path(project.namespace, project, issue) diff --git a/spec/features/merge_requests/assign_issues_spec.rb b/spec/features/merge_requests/assign_issues_spec.rb index 43cc6f2a2a7..ec49003772b 100644 --- a/spec/features/merge_requests/assign_issues_spec.rb +++ b/spec/features/merge_requests/assign_issues_spec.rb @@ -33,7 +33,7 @@ feature 'Merge request issue assignment', js: true, feature: true do end it "doesn't display if related issues are already assigned" do - [issue1, issue2].each { |issue| issue.update!(assignee: user) } + [issue1, issue2].each { |issue| issue.update!(assignees: [user]) } visit_merge_request diff --git a/spec/features/milestones/show_spec.rb b/spec/features/milestones/show_spec.rb index 40b4dc63697..227eb04ba72 100644 --- a/spec/features/milestones/show_spec.rb +++ b/spec/features/milestones/show_spec.rb @@ -5,7 +5,7 @@ describe 'Milestone show', feature: true do let(:project) { create(:empty_project) } let(:milestone) { create(:milestone, project: project) } let(:labels) { create_list(:label, 2, project: project) } - let(:issue_params) { { project: project, assignee: user, author: user, milestone: milestone, labels: labels } } + let(:issue_params) { { project: project, assignees: [user], author: user, milestone: milestone, labels: labels } } before do project.add_user(user, :developer) diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index d28a853bbc2..fa5e30075e3 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -12,7 +12,7 @@ feature 'issuable templates', feature: true, js: true do context 'user creates an issue using templates' do let(:template_content) { 'this is a test "bug" template' } let(:longtemplate_content) { %Q(this\n\n\n\n\nis\n\n\n\n\na\n\n\n\n\nbug\n\n\n\n\ntemplate) } - let(:issue) { create(:issue, author: user, assignee: user, project: project) } + let(:issue) { create(:issue, author: user, assignees: [user], project: project) } let(:description_addition) { ' appending to description' } background do @@ -72,7 +72,7 @@ feature 'issuable templates', feature: true, js: true do context 'user creates an issue using templates, with a prior description' do let(:prior_description) { 'test issue description' } let(:template_content) { 'this is a test "bug" template' } - let(:issue) { create(:issue, author: user, assignee: user, project: project) } + let(:issue) { create(:issue, author: user, assignees: [user], project: project) } background do project.repository.create_file( diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index da6388dcdf2..498a4a5cba0 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -5,7 +5,7 @@ describe "Search", feature: true do let(:user) { create(:user) } let(:project) { create(:empty_project, namespace: user.namespace) } - let!(:issue) { create(:issue, project: project, assignee: user) } + let!(:issue) { create(:issue, project: project, assignees: [user]) } let!(:issue2) { create(:issue, project: project, author: user) } before do diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index e2d9cfdd0b0..a23c4ca2b92 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -6,7 +6,7 @@ describe 'Unsubscribe links', feature: true do let(:recipient) { create(:user) } let(:author) { create(:user) } let(:project) { create(:empty_project, :public) } - let(:params) { { title: 'A bug!', description: 'Fix it!', assignee: recipient } } + let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } } let(:issue) { Issues::CreateService.new(project, author, params).execute } let(:mail) { ActionMailer::Base.deliveries.last } diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index a5f717e6233..e6bf77aee6a 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -1,19 +1,19 @@ require 'spec_helper' describe IssuesFinder do - set(:user) { create(:user) } - set(:user2) { create(:user) } - set(:project1) { create(:empty_project) } - set(:project2) { create(:empty_project) } - set(:milestone) { create(:milestone, project: project1) } - set(:label) { create(:label, project: project2) } - set(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') } - set(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') } - set(:issue3) { create(:issue, author: user2, assignee: user2, project: project2, title: 'tanuki', description: 'tanuki') } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:project1) { create(:empty_project) } + let(:project2) { create(:empty_project) } + let(:milestone) { create(:milestone, project: project1) } + let(:label) { create(:label, project: project2) } + let(:issue1) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab') } + let(:issue2) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab') } + let(:issue3) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki') } describe '#execute' do - set(:closed_issue) { create(:issue, author: user2, assignee: user2, project: project2, state: 'closed') } - set(:label_link) { create(:label_link, label: label, target: issue2) } + let(:closed_issue) { create(:issue, author: user2, assignees: [user2], project: project2, state: 'closed') } + let!(:label_link) { create(:label_link, label: label, target: issue2) } let(:search_user) { user } let(:params) { {} } let(:issues) { described_class.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute } @@ -91,7 +91,7 @@ describe IssuesFinder do before do milestones.each do |milestone| - create(:issue, project: milestone.project, milestone: milestone, author: user, assignee: user) + create(:issue, project: milestone.project, milestone: milestone, author: user, assignees: [user]) end end @@ -126,7 +126,7 @@ describe IssuesFinder do before do milestones.each do |milestone| - create(:issue, project: milestone.project, milestone: milestone, author: user, assignee: user) + create(:issue, project: milestone.project, milestone: milestone, author: user, assignees: [user]) end end diff --git a/spec/fixtures/api/schemas/issue.json b/spec/fixtures/api/schemas/issue.json index 21c078e0f44..983beb838b7 100644 --- a/spec/fixtures/api/schemas/issue.json +++ b/spec/fixtures/api/schemas/issue.json @@ -40,11 +40,31 @@ "additionalProperties": false } }, +<<<<<<< HEAD "assignee": { "id": { "type": "integet" }, "name": { "type": "string" }, "username": { "type": "string" }, "avatar_url": { "type": "uri" } +======= + "assignees": { + "type": "array", + "items": { + "type": ["object", "null"], + "required": [ + "id", + "name", + "username", + "avatar_url" + ], + "properties": { + "id": { "type": "integer" }, + "name": { "type": "string" }, + "username": { "type": "string" }, + "avatar_url": { "type": "uri" } + } + } +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master }, "subscribed": { "type": ["boolean", "null"] } }, diff --git a/spec/fixtures/api/schemas/public_api/v4/issues.json b/spec/fixtures/api/schemas/public_api/v4/issues.json index 52199e75734..2d1c84ee93d 100644 --- a/spec/fixtures/api/schemas/public_api/v4/issues.json +++ b/spec/fixtures/api/schemas/public_api/v4/issues.json @@ -33,6 +33,21 @@ }, "additionalProperties": false }, + "assignees": { + "type": "array", + "items": { + "type": ["object", "null"], + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" } + }, + "additionalProperties": false + } + }, "assignee": { "type": ["object", "null"], "properties": { @@ -67,7 +82,7 @@ "required": [ "id", "iid", "project_id", "title", "description", "state", "created_at", "updated_at", "labels", - "milestone", "assignee", "author", "user_notes_count", + "milestone", "assignees", "author", "user_notes_count", "upvotes", "downvotes", "due_date", "confidential", "web_url" ], diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 93bb711f29a..c1ecb46aece 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -4,6 +4,23 @@ describe IssuablesHelper do let(:label) { build_stubbed(:label) } let(:label2) { build_stubbed(:label) } + describe '#users_dropdown_label' do + let(:user) { build_stubbed(:user) } + let(:user2) { build_stubbed(:user) } + + it 'returns unassigned' do + expect(users_dropdown_label([])).to eq('Unassigned') + end + + it 'returns selected user\'s name' do + expect(users_dropdown_label([user])).to eq(user.name) + end + + it 'returns selected user\'s name and counter' do + expect(users_dropdown_label([user, user2])).to eq("#{user.name} + 1 more") + end + end + describe '#issuable_labels_tooltip' do it 'returns label text' do expect(issuable_labels_tooltip([label])).to eq(label.title) diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index b55ff2f473a..5ea160b7790 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -8,14 +8,14 @@ import Vue from 'vue'; import Cookies from 'js-cookie'; -require('~/lib/utils/url_utility'); -require('~/boards/models/issue'); -require('~/boards/models/label'); -require('~/boards/models/list'); -require('~/boards/models/user'); -require('~/boards/services/board_service'); -require('~/boards/stores/boards_store'); -require('./mock_data'); +import '~/lib/utils/url_utility'; +import '~/boards/models/issue'; +import '~/boards/models/label'; +import '~/boards/models/list'; +import '~/boards/models/assignee'; +import '~/boards/services/board_service'; +import '~/boards/stores/boards_store'; +import './mock_data'; describe('Store', () => { beforeEach(() => { @@ -212,7 +212,8 @@ describe('Store', () => { title: 'Testing', iid: 2, confidential: false, - labels: [] + labels: [], + assignees: [], }); const list = gl.issueBoards.BoardsStore.addList(listObj); diff --git a/spec/javascripts/boards/issue_card_spec.js b/spec/javascripts/boards/issue_card_spec.js index 1a5e9e9fd07..b1907ac3070 100644 --- a/spec/javascripts/boards/issue_card_spec.js +++ b/spec/javascripts/boards/issue_card_spec.js @@ -1,20 +1,20 @@ -/* global ListUser */ +/* global ListAssignee */ /* global ListLabel */ /* global listObj */ /* global ListIssue */ import Vue from 'vue'; -require('~/boards/models/issue'); -require('~/boards/models/label'); -require('~/boards/models/list'); -require('~/boards/models/user'); -require('~/boards/stores/boards_store'); -require('~/boards/components/issue_card_inner'); -require('./mock_data'); +import '~/boards/models/issue'; +import '~/boards/models/label'; +import '~/boards/models/list'; +import '~/boards/models/assignee'; +import '~/boards/stores/boards_store'; +import '~/boards/components/issue_card_inner'; +import './mock_data'; describe('Issue card component', () => { - const user = new ListUser({ + const user = new ListAssignee({ id: 1, name: 'testing 123', username: 'test', @@ -40,6 +40,7 @@ describe('Issue card component', () => { iid: 1, confidential: false, labels: [list.label], + assignees: [], }); component = new Vue({ @@ -92,12 +93,12 @@ describe('Issue card component', () => { it('renders confidential icon', (done) => { component.issue.confidential = true; - setTimeout(() => { + Vue.nextTick(() => { expect( component.$el.querySelector('.confidential-icon'), ).not.toBeNull(); done(); - }, 0); + }); }); it('renders issue ID with #', () => { @@ -109,34 +110,32 @@ describe('Issue card component', () => { describe('assignee', () => { it('does not render assignee', () => { expect( - component.$el.querySelector('.card-assignee'), + component.$el.querySelector('.card-assignee .avatar'), ).toBeNull(); }); describe('exists', () => { beforeEach((done) => { - component.issue.assignee = user; + component.issue.assignees = [user]; - setTimeout(() => { - done(); - }, 0); + Vue.nextTick(() => done()); }); it('renders assignee', () => { expect( - component.$el.querySelector('.card-assignee'), + component.$el.querySelector('.card-assignee .avatar'), ).not.toBeNull(); }); it('sets title', () => { expect( - component.$el.querySelector('.card-assignee').getAttribute('title'), + component.$el.querySelector('.card-assignee a').getAttribute('title'), ).toContain(`Assigned to ${user.name}`); }); it('sets users path', () => { expect( - component.$el.querySelector('.card-assignee').getAttribute('href'), + component.$el.querySelector('.card-assignee a').getAttribute('href'), ).toBe('/test'); }); @@ -148,6 +147,75 @@ describe('Issue card component', () => { }); }); + describe('multiple assignees', () => { + beforeEach((done) => { + component.issue.assignees = [ + user, + new ListAssignee({ + id: 2, + name: 'user2', + username: 'user2', + avatar: 'test_image', + }), + new ListAssignee({ + id: 3, + name: 'user3', + username: 'user3', + avatar: 'test_image', + }), + new ListAssignee({ + id: 4, + name: 'user4', + username: 'user4', + avatar: 'test_image', + })]; + + Vue.nextTick(() => done()); + }); + + it('renders all four assignees', () => { + expect(component.$el.querySelectorAll('.card-assignee .avatar').length).toEqual(4); + }); + + describe('more than four assignees', () => { + beforeEach((done) => { + component.issue.assignees.push(new ListAssignee({ + id: 5, + name: 'user5', + username: 'user5', + avatar: 'test_image', + })); + + Vue.nextTick(() => done()); + }); + + it('renders more avatar counter', () => { + expect(component.$el.querySelector('.card-assignee .avatar-counter').innerText).toEqual('+2'); + }); + + it('renders three assignees', () => { + expect(component.$el.querySelectorAll('.card-assignee .avatar').length).toEqual(3); + }); + + it('renders 99+ avatar counter', (done) => { + for (let i = 5; i < 104; i += 1) { + const u = new ListAssignee({ + id: i, + name: 'name', + username: 'username', + avatar: 'test_image', + }); + component.issue.assignees.push(u); + } + + Vue.nextTick(() => { + expect(component.$el.querySelector('.card-assignee .avatar-counter').innerText).toEqual('99+'); + done(); + }); + }); + }); + }); + describe('labels', () => { it('does not render any', () => { expect( @@ -159,9 +227,7 @@ describe('Issue card component', () => { beforeEach((done) => { component.issue.addLabel(label1); - setTimeout(() => { - done(); - }, 0); + Vue.nextTick(() => done()); }); it('does not render list label', () => { diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index c96dfe94a4a..cd1497bc5e6 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -2,14 +2,15 @@ /* global BoardService */ /* global ListIssue */ -require('~/lib/utils/url_utility'); -require('~/boards/models/issue'); -require('~/boards/models/label'); -require('~/boards/models/list'); -require('~/boards/models/user'); -require('~/boards/services/board_service'); -require('~/boards/stores/boards_store'); -require('./mock_data'); +import Vue from 'vue'; +import '~/lib/utils/url_utility'; +import '~/boards/models/issue'; +import '~/boards/models/label'; +import '~/boards/models/list'; +import '~/boards/models/assignee'; +import '~/boards/services/board_service'; +import '~/boards/stores/boards_store'; +import './mock_data'; describe('Issue model', () => { let issue; @@ -27,7 +28,13 @@ describe('Issue model', () => { title: 'test', color: 'red', description: 'testing' - }] + }], + assignees: [{ + id: 1, + name: 'name', + username: 'username', + avatar_url: 'http://avatar_url', + }], }); }); @@ -80,6 +87,33 @@ describe('Issue model', () => { expect(issue.labels.length).toBe(0); }); + it('adds assignee', () => { + issue.addAssignee({ + id: 2, + name: 'Bruce Wayne', + username: 'batman', + avatar_url: 'http://batman', + }); + + expect(issue.assignees.length).toBe(2); + }); + + it('finds assignee', () => { + const assignee = issue.findAssignee(issue.assignees[0]); + expect(assignee).toBeDefined(); + }); + + it('removes assignee', () => { + const assignee = issue.findAssignee(issue.assignees[0]); + issue.removeAssignee(assignee); + expect(issue.assignees.length).toBe(0); + }); + + it('removes all assignees', () => { + issue.removeAllAssignees(); + expect(issue.assignees.length).toBe(0); + }); + it('sets position to infinity if no position is stored', () => { expect(issue.position).toBe(Infinity); }); @@ -90,9 +124,31 @@ describe('Issue model', () => { iid: 1, confidential: false, relative_position: 1, - labels: [] + labels: [], + assignees: [], }); expect(relativePositionIssue.position).toBe(1); }); + + describe('update', () => { + it('passes assignee ids when there are assignees', (done) => { + spyOn(Vue.http, 'patch').and.callFake((url, data) => { + expect(data.issue.assignee_ids).toEqual([1]); + done(); + }); + + issue.update('url'); + }); + + it('passes assignee ids of [0] when there are no assignees', (done) => { + spyOn(Vue.http, 'patch').and.callFake((url, data) => { + expect(data.issue.assignee_ids).toEqual([0]); + done(); + }); + + issue.removeAllAssignees(); + issue.update('url'); + }); + }); }); diff --git a/spec/javascripts/boards/list_spec.js b/spec/javascripts/boards/list_spec.js index 24a2da9f6b6..88a0e6855f4 100644 --- a/spec/javascripts/boards/list_spec.js +++ b/spec/javascripts/boards/list_spec.js @@ -8,14 +8,14 @@ import Vue from 'vue'; -require('~/lib/utils/url_utility'); -require('~/boards/models/issue'); -require('~/boards/models/label'); -require('~/boards/models/list'); -require('~/boards/models/user'); -require('~/boards/services/board_service'); -require('~/boards/stores/boards_store'); -require('./mock_data'); +import '~/lib/utils/url_utility'; +import '~/boards/models/issue'; +import '~/boards/models/label'; +import '~/boards/models/list'; +import '~/boards/models/assignee'; +import '~/boards/services/board_service'; +import '~/boards/stores/boards_store'; +import './mock_data'; describe('List model', () => { let list; @@ -94,7 +94,8 @@ describe('List model', () => { title: 'Testing', iid: _.random(10000), confidential: false, - labels: [list.label, listDup.label] + labels: [list.label, listDup.label], + assignees: [], }); list.issues.push(issue); diff --git a/spec/javascripts/boards/mock_data.js b/spec/javascripts/boards/mock_data.js index a4fa694eebe..a64c3964ee3 100644 --- a/spec/javascripts/boards/mock_data.js +++ b/spec/javascripts/boards/mock_data.js @@ -33,7 +33,8 @@ const BoardsMockData = { title: 'Testing', iid: 1, confidential: false, - labels: [] + labels: [], + assignees: [], }], size: 1 } diff --git a/spec/javascripts/boards/modal_store_spec.js b/spec/javascripts/boards/modal_store_spec.js index 80db816aff8..32e6d04df9f 100644 --- a/spec/javascripts/boards/modal_store_spec.js +++ b/spec/javascripts/boards/modal_store_spec.js @@ -1,10 +1,10 @@ /* global ListIssue */ -require('~/boards/models/issue'); -require('~/boards/models/label'); -require('~/boards/models/list'); -require('~/boards/models/user'); -require('~/boards/stores/modal_store'); +import '~/boards/models/issue'; +import '~/boards/models/label'; +import '~/boards/models/list'; +import '~/boards/models/assignee'; +import '~/boards/stores/modal_store'; describe('Modal store', () => { let issue; @@ -21,12 +21,14 @@ describe('Modal store', () => { iid: 1, confidential: false, labels: [], + assignees: [], }); issue2 = new ListIssue({ title: 'Testing', iid: 2, confidential: false, labels: [], + assignees: [], }); Store.store.issues.push(issue); Store.store.issues.push(issue2); diff --git a/spec/javascripts/issuable_time_tracker_spec.js b/spec/javascripts/issuable_time_tracker_spec.js index 0a830f25e29..8ff93c4f918 100644 --- a/spec/javascripts/issuable_time_tracker_spec.js +++ b/spec/javascripts/issuable_time_tracker_spec.js @@ -2,7 +2,7 @@ import Vue from 'vue'; -require('~/issuable/time_tracking/components/time_tracker'); +import timeTracker from '~/sidebar/components/time_tracking/time_tracker'; function initTimeTrackingComponent(opts) { setFixtures(` @@ -16,187 +16,185 @@ function initTimeTrackingComponent(opts) { time_spent: opts.timeSpent, human_time_estimate: opts.timeEstimateHumanReadable, human_time_spent: opts.timeSpentHumanReadable, - docsUrl: '/help/workflow/time_tracking.md', + rootPath: '/', }; - const TimeTrackingComponent = Vue.component('issuable-time-tracker'); + const TimeTrackingComponent = Vue.extend(timeTracker); this.timeTracker = new TimeTrackingComponent({ el: '#mock-container', propsData: this.initialData, }); } -((gl) => { - describe('Issuable Time Tracker', function() { - describe('Initialization', function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 5000, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '1h 23m' }); - }); +describe('Issuable Time Tracker', function() { + describe('Initialization', function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 5000, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '1h 23m' }); + }); - it('should return something defined', function() { - expect(this.timeTracker).toBeDefined(); - }); + it('should return something defined', function() { + expect(this.timeTracker).toBeDefined(); + }); - it ('should correctly set timeEstimate', function(done) { - Vue.nextTick(() => { - expect(this.timeTracker.timeEstimate).toBe(this.initialData.time_estimate); - done(); - }); + it ('should correctly set timeEstimate', function(done) { + Vue.nextTick(() => { + expect(this.timeTracker.timeEstimate).toBe(this.initialData.time_estimate); + done(); }); - it ('should correctly set time_spent', function(done) { - Vue.nextTick(() => { - expect(this.timeTracker.timeSpent).toBe(this.initialData.time_spent); - done(); - }); + }); + it ('should correctly set time_spent', function(done) { + Vue.nextTick(() => { + expect(this.timeTracker.timeSpent).toBe(this.initialData.time_spent); + done(); }); }); + }); - describe('Content Display', function() { - describe('Panes', function() { - describe('Comparison pane', function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 5000, timeEstimateHumanReadable: '', timeSpentHumanReadable: '' }); + describe('Content Display', function() { + describe('Panes', function() { + describe('Comparison pane', function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 5000, timeEstimateHumanReadable: '', timeSpentHumanReadable: '' }); + }); + + it('should show the "Comparison" pane when timeEstimate and time_spent are truthy', function(done) { + Vue.nextTick(() => { + const $comparisonPane = this.timeTracker.$el.querySelector('.time-tracking-comparison-pane'); + expect(this.timeTracker.showComparisonState).toBe(true); + done(); }); + }); - it('should show the "Comparison" pane when timeEstimate and time_spent are truthy', function(done) { + describe('Remaining meter', function() { + it('should display the remaining meter with the correct width', function(done) { Vue.nextTick(() => { - const $comparisonPane = this.timeTracker.$el.querySelector('.time-tracking-comparison-pane'); - expect(this.timeTracker.showComparisonState).toBe(true); + const meterWidth = this.timeTracker.$el.querySelector('.time-tracking-comparison-pane .meter-fill').style.width; + const correctWidth = '5%'; + + expect(meterWidth).toBe(correctWidth); done(); - }); + }) }); - describe('Remaining meter', function() { - it('should display the remaining meter with the correct width', function(done) { - Vue.nextTick(() => { - const meterWidth = this.timeTracker.$el.querySelector('.time-tracking-comparison-pane .meter-fill').style.width; - const correctWidth = '5%'; - - expect(meterWidth).toBe(correctWidth); - done(); - }) - }); - - it('should display the remaining meter with the correct background color when within estimate', function(done) { - Vue.nextTick(() => { - const styledMeter = $(this.timeTracker.$el).find('.time-tracking-comparison-pane .within_estimate .meter-fill'); - expect(styledMeter.length).toBe(1); - done() - }); + it('should display the remaining meter with the correct background color when within estimate', function(done) { + Vue.nextTick(() => { + const styledMeter = $(this.timeTracker.$el).find('.time-tracking-comparison-pane .within_estimate .meter-fill'); + expect(styledMeter.length).toBe(1); + done() }); + }); - it('should display the remaining meter with the correct background color when over estimate', function(done) { - this.timeTracker.time_estimate = 100000; - this.timeTracker.time_spent = 20000000; - Vue.nextTick(() => { - const styledMeter = $(this.timeTracker.$el).find('.time-tracking-comparison-pane .over_estimate .meter-fill'); - expect(styledMeter.length).toBe(1); - done(); - }); + it('should display the remaining meter with the correct background color when over estimate', function(done) { + this.timeTracker.time_estimate = 100000; + this.timeTracker.time_spent = 20000000; + Vue.nextTick(() => { + const styledMeter = $(this.timeTracker.$el).find('.time-tracking-comparison-pane .over_estimate .meter-fill'); + expect(styledMeter.length).toBe(1); + done(); }); }); }); + }); - describe("Estimate only pane", function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 0, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '' }); - }); + describe("Estimate only pane", function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 100000, timeSpent: 0, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '' }); + }); - it('should display the human readable version of time estimated', function(done) { - Vue.nextTick(() => { - const estimateText = this.timeTracker.$el.querySelector('.time-tracking-estimate-only-pane').innerText; - const correctText = 'Estimated: 2h 46m'; + it('should display the human readable version of time estimated', function(done) { + Vue.nextTick(() => { + const estimateText = this.timeTracker.$el.querySelector('.time-tracking-estimate-only-pane').innerText; + const correctText = 'Estimated: 2h 46m'; - expect(estimateText).toBe(correctText); - done(); - }); + expect(estimateText).toBe(correctText); + done(); }); }); + }); - describe('Spent only pane', function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 5000, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '1h 23m' }); - }); + describe('Spent only pane', function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 5000, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '1h 23m' }); + }); - it('should display the human readable version of time spent', function(done) { - Vue.nextTick(() => { - const spentText = this.timeTracker.$el.querySelector('.time-tracking-spend-only-pane').innerText; - const correctText = 'Spent: 1h 23m'; + it('should display the human readable version of time spent', function(done) { + Vue.nextTick(() => { + const spentText = this.timeTracker.$el.querySelector('.time-tracking-spend-only-pane').innerText; + const correctText = 'Spent: 1h 23m'; - expect(spentText).toBe(correctText); - done(); - }); + expect(spentText).toBe(correctText); + done(); }); }); + }); - describe('No time tracking pane', function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 0, timeEstimateHumanReadable: 0, timeSpentHumanReadable: 0 }); - }); + describe('No time tracking pane', function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 0, timeEstimateHumanReadable: '', timeSpentHumanReadable: '' }); + }); - it('should only show the "No time tracking" pane when both timeEstimate and time_spent are falsey', function(done) { - Vue.nextTick(() => { - const $noTrackingPane = this.timeTracker.$el.querySelector('.time-tracking-no-tracking-pane'); - const noTrackingText =$noTrackingPane.innerText; - const correctText = 'No estimate or time spent'; + it('should only show the "No time tracking" pane when both timeEstimate and time_spent are falsey', function(done) { + Vue.nextTick(() => { + const $noTrackingPane = this.timeTracker.$el.querySelector('.time-tracking-no-tracking-pane'); + const noTrackingText =$noTrackingPane.innerText; + const correctText = 'No estimate or time spent'; - expect(this.timeTracker.showNoTimeTrackingState).toBe(true); - expect($noTrackingPane).toBeVisible(); - expect(noTrackingText).toBe(correctText); - done(); - }); + expect(this.timeTracker.showNoTimeTrackingState).toBe(true); + expect($noTrackingPane).toBeVisible(); + expect(noTrackingText).toBe(correctText); + done(); }); }); + }); - describe("Help pane", function() { - beforeEach(function() { - initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 0 }); - }); + describe("Help pane", function() { + beforeEach(function() { + initTimeTrackingComponent.call(this, { timeEstimate: 0, timeSpent: 0 }); + }); - it('should not show the "Help" pane by default', function(done) { - Vue.nextTick(() => { - const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); + it('should not show the "Help" pane by default', function(done) { + Vue.nextTick(() => { + const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); - expect(this.timeTracker.showHelpState).toBe(false); - expect($helpPane).toBeNull(); - done(); - }); + expect(this.timeTracker.showHelpState).toBe(false); + expect($helpPane).toBeNull(); + done(); }); + }); - it('should show the "Help" pane when help button is clicked', function(done) { - Vue.nextTick(() => { - $(this.timeTracker.$el).find('.help-button').click(); + it('should show the "Help" pane when help button is clicked', function(done) { + Vue.nextTick(() => { + $(this.timeTracker.$el).find('.help-button').click(); - setTimeout(() => { - const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); - expect(this.timeTracker.showHelpState).toBe(true); - expect($helpPane).toBeVisible(); - done(); - }, 10); - }); + setTimeout(() => { + const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); + expect(this.timeTracker.showHelpState).toBe(true); + expect($helpPane).toBeVisible(); + done(); + }, 10); }); + }); - it('should not show the "Help" pane when help button is clicked and then closed', function(done) { - Vue.nextTick(() => { - $(this.timeTracker.$el).find('.help-button').click(); + it('should not show the "Help" pane when help button is clicked and then closed', function(done) { + Vue.nextTick(() => { + $(this.timeTracker.$el).find('.help-button').click(); - setTimeout(() => { + setTimeout(() => { - $(this.timeTracker.$el).find('.close-help-button').click(); + $(this.timeTracker.$el).find('.close-help-button').click(); - setTimeout(() => { - const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); + setTimeout(() => { + const $helpPane = this.timeTracker.$el.querySelector('.time-tracking-help-state'); - expect(this.timeTracker.showHelpState).toBe(false); - expect($helpPane).toBeNull(); + expect(this.timeTracker.showHelpState).toBe(false); + expect($helpPane).toBeNull(); - done(); - }, 1000); + done(); }, 1000); - }); + }, 1000); }); }); }); }); }); -})(window.gl || (window.gl = {})); +}); diff --git a/spec/javascripts/subbable_resource_spec.js b/spec/javascripts/subbable_resource_spec.js deleted file mode 100644 index 454386697f5..00000000000 --- a/spec/javascripts/subbable_resource_spec.js +++ /dev/null @@ -1,63 +0,0 @@ -/* eslint-disable max-len, arrow-parens, comma-dangle */ - -require('~/subbable_resource'); - -/* -* Test that each rest verb calls the publish and subscribe function and passes the correct value back -* -* -* */ -((global) => { - describe('Subbable Resource', function () { - describe('PubSub', function () { - beforeEach(function () { - this.MockResource = new global.SubbableResource('https://example.com'); - }); - it('should successfully add a single subscriber', function () { - const callback = () => {}; - this.MockResource.subscribe(callback); - - expect(this.MockResource.subscribers.length).toBe(1); - expect(this.MockResource.subscribers[0]).toBe(callback); - }); - - it('should successfully add multiple subscribers', function () { - const callbackOne = () => {}; - const callbackTwo = () => {}; - const callbackThree = () => {}; - - this.MockResource.subscribe(callbackOne); - this.MockResource.subscribe(callbackTwo); - this.MockResource.subscribe(callbackThree); - - expect(this.MockResource.subscribers.length).toBe(3); - }); - - it('should successfully publish an update to a single subscriber', function () { - const state = { myprop: 1 }; - - const callbacks = { - one: (data) => expect(data.myprop).toBe(2), - two: (data) => expect(data.myprop).toBe(2), - three: (data) => expect(data.myprop).toBe(2) - }; - - const spyOne = spyOn(callbacks, 'one'); - const spyTwo = spyOn(callbacks, 'two'); - const spyThree = spyOn(callbacks, 'three'); - - this.MockResource.subscribe(callbacks.one); - this.MockResource.subscribe(callbacks.two); - this.MockResource.subscribe(callbacks.three); - - state.myprop += 1; - - this.MockResource.publish(state); - - expect(spyOne).toHaveBeenCalled(); - expect(spyTwo).toHaveBeenCalled(); - expect(spyThree).toHaveBeenCalled(); - }); - }); - }); -})(window.gl || (window.gl = {})); diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index 8a6fe1ad6a3..7c4a0f32c7b 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -113,7 +113,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do it 'allows references for assignee' do assignee = create(:user) project = create(:empty_project, :public) - issue = create(:issue, :confidential, project: project, assignee: assignee) + issue = create(:issue, :confidential, project: project, assignees: [assignee]) link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: assignee) diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/github_import/issue_formatter_spec.rb index f34d09f2c1d..a4089592cf2 100644 --- a/spec/lib/gitlab/github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/issue_formatter_spec.rb @@ -43,7 +43,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do description: "*Created by: octocat*\n\nI'm having a problem with this.", state: 'opened', author_id: project.creator_id, - assignee_id: nil, + assignee_ids: [], created_at: created_at, updated_at: updated_at } @@ -64,7 +64,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do description: "*Created by: octocat*\n\nI'm having a problem with this.", state: 'closed', author_id: project.creator_id, - assignee_id: nil, + assignee_ids: [], created_at: created_at, updated_at: updated_at } @@ -77,19 +77,19 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do let(:raw_data) { double(base_data.merge(assignee: octocat)) } it 'returns nil as assignee_id when is not a GitLab user' do - expect(issue.attributes.fetch(:assignee_id)).to be_nil + expect(issue.attributes.fetch(:assignee_ids)).to be_empty end it 'returns GitLab user id associated with GitHub id as assignee_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') - expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id + expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id] end it 'returns GitLab user id associated with GitHub email as assignee_id' do gl_user = create(:user, email: octocat.email) - expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id + expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id] end end diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb index ccaa88a5c79..622a0f513f4 100644 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ b/spec/lib/gitlab/google_code_import/importer_spec.rb @@ -49,7 +49,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do expect(issue).not_to be_nil expect(issue.iid).to eq(169) expect(issue.author).to eq(project.creator) - expect(issue.assignee).to eq(mapped_user) + expect(issue.assignees).to eq([mapped_user]) expect(issue.state).to eq("closed") expect(issue.label_names).to include("Priority: Medium") expect(issue.label_names).to include("Status: Fixed") diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 0abf89d060c..cd2fa27bb9f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -3,7 +3,7 @@ issues: - subscriptions - award_emoji - author -- assignee +- assignees - updated_by - milestone - notes @@ -16,6 +16,7 @@ issues: - merge_requests_closing_issues - metrics - timelogs +- issue_assignees events: - author - project diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 1035428b2e7..5aeb29b7fec 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -203,7 +203,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do end def setup_project - issue = create(:issue, assignee: user) + issue = create(:issue, assignees: [user]) snippet = create(:project_snippet) release = create(:release) group = create(:group) diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index e0ebea63eb4..a7c8e7f1f57 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -89,7 +89,7 @@ describe Gitlab::ProjectSearchResults, lib: true do let(:project) { create(:empty_project, :internal) } let!(:issue) { create(:issue, project: project, title: 'Issue 1') } let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } - let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignees: [assignee]) } it 'does not list project confidential issues for non project members' do results = described_class.new(non_member, project, query) diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 847fb977400..31c3cd4d53c 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -72,9 +72,9 @@ describe Gitlab::SearchResults do let(:admin) { create(:admin) } let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') } let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) } - let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignees: [assignee]) } let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) } - let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee) } + let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignees: [assignee]) } let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') } it 'does not list confidential issues for non project members' do diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 9f12e40d808..1e6260270fe 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -36,11 +36,11 @@ describe Notify do end context 'for issues' do - let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project) } - let(:issue_with_description) { create(:issue, author: current_user, assignee: assignee, project: project, description: 'My awesome description') } + let(:issue) { create(:issue, author: current_user, assignees: [assignee], project: project) } + let(:issue_with_description) { create(:issue, author: current_user, assignees: [assignee], project: project, description: 'My awesome description') } describe 'that are new' do - subject { described_class.new_issue_email(issue.assignee_id, issue.id) } + subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } it_behaves_like 'an assignee email' it_behaves_like 'an email starting a new thread with reply-by-email enabled' do @@ -69,7 +69,7 @@ describe Notify do end describe 'that are new with a description' do - subject { described_class.new_issue_email(issue_with_description.assignee_id, issue_with_description.id) } + subject { described_class.new_issue_email(issue_with_description.assignees.first.id, issue_with_description.id) } it_behaves_like 'it should show Gmail Actions View Issue link' @@ -79,7 +79,7 @@ describe Notify do end describe 'that have been reassigned' do - subject { described_class.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user.id) } + subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } it_behaves_like 'a multiple recipients email' it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 3ecba2e9687..27890e33b49 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -10,7 +10,6 @@ describe Issuable do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author) } - it { is_expected.to belong_to(:assignee) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } @@ -66,60 +65,6 @@ describe Issuable do end end - describe 'assignee_name' do - it 'is delegated to assignee' do - issue.update!(assignee: create(:user)) - - expect(issue.assignee_name).to eq issue.assignee.name - end - - it 'returns nil when assignee is nil' do - issue.assignee_id = nil - issue.save(validate: false) - - expect(issue.assignee_name).to eq nil - end - end - - describe "before_save" do - describe "#update_cache_counts" do - context "when previous assignee exists" do - before do - assignee = create(:user) - issue.project.team << [assignee, :developer] - issue.update(assignee: assignee) - end - - it "updates cache counts for new assignee" do - user = create(:user) - - expect(user).to receive(:update_cache_counts) - - issue.update(assignee: user) - end - - it "updates cache counts for previous assignee" do - old_assignee = issue.assignee - allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee) - - expect(old_assignee).to receive(:update_cache_counts) - - issue.update(assignee: nil) - end - end - - context "when previous assignee does not exist" do - before{ issue.update(assignee: nil) } - - it "updates cache count for the new assignee" do - expect_any_instance_of(User).to receive(:update_cache_counts) - - issue.update(assignee: user) - end - end - end - end - describe ".search" do let!(:searchable_issue) { create(:issue, title: "Searchable issue") } @@ -307,7 +252,20 @@ describe Issuable do end context "issue is assigned" do - before { issue.update_attribute(:assignee, user) } + before { issue.assignees << user } + + it "returns correct hook data" do + expect(data[:assignees].first).to eq(user.hook_attrs) + end + end + + context "merge_request is assigned" do + let(:merge_request) { create(:merge_request) } + let(:data) { merge_request.to_hook_data(user) } + + before do + merge_request.update_attribute(:assignee, user) + end it "returns correct hook data" do expect(data[:object_attributes]['assignee_id']).to eq(user.id) @@ -329,24 +287,6 @@ describe Issuable do include_examples 'deprecated repository hook data' end - describe '#card_attributes' do - it 'includes the author name' do - allow(issue).to receive(:author).and_return(double(name: 'Robert')) - allow(issue).to receive(:assignee).and_return(nil) - - expect(issue.card_attributes). - to eq({ 'Author' => 'Robert', 'Assignee' => nil }) - end - - it 'includes the assignee name' do - allow(issue).to receive(:author).and_return(double(name: 'Robert')) - allow(issue).to receive(:assignee).and_return(double(name: 'Douwe')) - - expect(issue.card_attributes). - to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) - end - end - describe '#labels_array' do let(:project) { create(:empty_project) } let(:bug) { create(:label, project: project, title: 'bug') } @@ -475,27 +415,6 @@ describe Issuable do end end - describe '#assignee_or_author?' do - let(:user) { build(:user, id: 1) } - let(:issue) { build(:issue) } - - it 'returns true for a user that is assigned to an issue' do - issue.assignee = user - - expect(issue.assignee_or_author?(user)).to eq(true) - end - - it 'returns true for a user that is the author of an issue' do - issue.author = user - - expect(issue.assignee_or_author?(user)).to eq(true) - end - - it 'returns false for a user that is not the assignee or author' do - expect(issue.assignee_or_author?(user)).to eq(false) - end - end - describe '#spend_time' do let(:user) { create(:user) } let(:issue) { create(:issue) } diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 68e4c0a522b..675b730c557 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -11,13 +11,13 @@ describe Milestone, 'Milestoneish' do let(:milestone) { create(:milestone, project: project) } let!(:issue) { create(:issue, project: project, milestone: milestone) } let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) } - let!(:security_issue_2) { create(:issue, :confidential, project: project, assignee: assignee, milestone: milestone) } + let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) } let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) } let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) } let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } before do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8c90a538f57..e5954d80f00 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -92,8 +92,8 @@ describe Event, models: true do let(:author) { create(:author) } let(:assignee) { create(:user) } let(:admin) { create(:admin) } - let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note_on_commit) { create(:note_on_commit, project: project) } let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) } diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb index d8aed25c041..93c2c538e10 100644 --- a/spec/models/issue_collection_spec.rb +++ b/spec/models/issue_collection_spec.rb @@ -28,7 +28,7 @@ describe IssueCollection do end it 'returns the issues the user is assigned to' do - issue1.assignee = user + issue1.assignees << user expect(collection.updatable_by_user(user)).to eq([issue1]) end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 8748b98a4e3..725f5c2311f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe Issue, models: true do describe "Associations" do it { is_expected.to belong_to(:milestone) } + it { is_expected.to have_many(:assignees) } end describe 'modules' do @@ -37,6 +38,64 @@ describe Issue, models: true do end end + describe "before_save" do + describe "#update_cache_counts when an issue is reassigned" do + let(:issue) { create(:issue) } + let(:assignee) { create(:user) } + + context "when previous assignee exists" do + before do + issue.project.team << [assignee, :developer] + issue.assignees << assignee + end + + it "updates cache counts for new assignee" do + user = create(:user) + + expect(user).to receive(:update_cache_counts) + + issue.assignees << user + end + + it "updates cache counts for previous assignee" do + issue.assignees.first + + expect_any_instance_of(User).to receive(:update_cache_counts) + + issue.assignees.destroy_all + end + end + + context "when previous assignee does not exist" do + it "updates cache count for the new assignee" do + issue.assignees = [] + + expect_any_instance_of(User).to receive(:update_cache_counts) + + issue.assignees << assignee + end + end + end + end + + describe '#card_attributes' do + it 'includes the author name' do + allow(subject).to receive(:author).and_return(double(name: 'Robert')) + allow(subject).to receive(:assignees).and_return([]) + + expect(subject.card_attributes). + to eq({ 'Author' => 'Robert', 'Assignee' => '' }) + end + + it 'includes the assignee name' do + allow(subject).to receive(:author).and_return(double(name: 'Robert')) + allow(subject).to receive(:assignees).and_return([double(name: 'Douwe')]) + + expect(subject.card_attributes). + to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) + end + end + describe '#closed_at' do after do Timecop.return @@ -124,13 +183,24 @@ describe Issue, models: true do end end - describe '#is_being_reassigned?' do - it 'returns true if the issue assignee has changed' do - subject.assignee = create(:user) - expect(subject.is_being_reassigned?).to be_truthy + describe '#assignee_or_author?' do + let(:user) { create(:user) } + let(:issue) { create(:issue) } + + it 'returns true for a user that is assigned to an issue' do + issue.assignees << user + + expect(issue.assignee_or_author?(user)).to be_truthy end - it 'returns false if the issue assignee has not changed' do - expect(subject.is_being_reassigned?).to be_falsey + + it 'returns true for a user that is the author of an issue' do + issue.update(author: user) + + expect(issue.assignee_or_author?(user)).to be_truthy + end + + it 'returns false for a user that is not the assignee or author' do + expect(issue.assignee_or_author?(user)).to be_falsey end end @@ -383,14 +453,14 @@ describe Issue, models: true do user1 = create(:user) user2 = create(:user) project = create(:empty_project) - issue = create(:issue, assignee: user1, project: project) + issue = create(:issue, assignees: [user1], project: project) project.add_developer(user1) project.add_developer(user2) expect(user1.assigned_open_issues_count).to eq(1) expect(user2.assigned_open_issues_count).to eq(0) - issue.assignee = user2 + issue.assignees = [user2] issue.save expect(user1.assigned_open_issues_count).to eq(0) @@ -676,6 +746,11 @@ describe Issue, models: true do expect(attrs_hash).to include(:human_total_time_spent) expect(attrs_hash).to include('time_estimate') end + + it 'includes assignee_ids and deprecated assignee_id' do + expect(attrs_hash).to include(:assignee_id) + expect(attrs_hash).to include(:assignee_ids) + end end describe '#check_for_spam' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8b72125dd5d..6cf3dd30ead 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -9,6 +9,7 @@ describe MergeRequest, models: true do it { is_expected.to belong_to(:target_project).class_name('Project') } it { is_expected.to belong_to(:source_project).class_name('Project') } it { is_expected.to belong_to(:merge_user).class_name("User") } + it { is_expected.to belong_to(:assignee) } it { is_expected.to have_many(:merge_request_diffs).dependent(:destroy) } end @@ -86,6 +87,86 @@ describe MergeRequest, models: true do end end + describe "before_save" do + describe "#update_cache_counts when a merge request is reassigned" do + let(:project) { create :project } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:assignee) { create :user } + + context "when previous assignee exists" do + before do + project.team << [assignee, :developer] + merge_request.update(assignee: assignee) + end + + it "updates cache counts for new assignee" do + user = create(:user) + + expect(user).to receive(:update_cache_counts) + + merge_request.update(assignee: user) + end + + it "updates cache counts for previous assignee" do + old_assignee = merge_request.assignee + allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee) + + expect(old_assignee).to receive(:update_cache_counts) + + merge_request.update(assignee: nil) + end + end + + context "when previous assignee does not exist" do + it "updates cache count for the new assignee" do + merge_request.update(assignee: nil) + + expect_any_instance_of(User).to receive(:update_cache_counts) + + merge_request.update(assignee: assignee) + end + end + end + end + + describe '#card_attributes' do + it 'includes the author name' do + allow(subject).to receive(:author).and_return(double(name: 'Robert')) + allow(subject).to receive(:assignee).and_return(nil) + + expect(subject.card_attributes). + to eq({ 'Author' => 'Robert', 'Assignee' => nil }) + end + + it 'includes the assignee name' do + allow(subject).to receive(:author).and_return(double(name: 'Robert')) + allow(subject).to receive(:assignee).and_return(double(name: 'Douwe')) + + expect(subject.card_attributes). + to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) + end + end + + describe '#assignee_or_author?' do + let(:user) { create(:user) } + + it 'returns true for a user that is assigned to a merge request' do + subject.assignee = user + + expect(subject.assignee_or_author?(user)).to eq(true) + end + + it 'returns true for a user that is the author of a merge request' do + subject.author = user + + expect(subject.assignee_or_author?(user)).to eq(true) + end + + it 'returns false for a user that is not the assignee or author' do + expect(subject.assignee_or_author?(user)).to eq(false) + end + end + describe '#cache_merge_request_closes_issues!' do before do subject.project.team << [subject.author, :developer] @@ -295,16 +376,6 @@ describe MergeRequest, models: true do end end - describe '#is_being_reassigned?' do - it 'returns true if the merge_request assignee has changed' do - subject.assignee = create(:user) - expect(subject.is_being_reassigned?).to be_truthy - end - it 'returns false if the merge request assignee has not changed' do - expect(subject.is_being_reassigned?).to be_falsey - end - end - describe '#for_fork?' do it 'returns true if the merge request is for a fork' do subject.source_project = build_stubbed(:empty_project, namespace: create(:group)) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 3ca13111acb..f7c317021dc 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -19,7 +19,7 @@ describe API::Issues do let!(:closed_issue) do create :closed_issue, author: user, - assignee: user, + assignees: [user], project: project, state: :closed, milestone: milestone, @@ -31,14 +31,14 @@ describe API::Issues do :confidential, project: project, author: author, - assignee: assignee, + assignees: [assignee], created_at: generate(:past_time), updated_at: 2.hours.ago end let!(:issue) do create :issue, author: user, - assignee: user, + assignees: [user], project: project, milestone: milestone, created_at: generate(:past_time), @@ -265,7 +265,7 @@ describe API::Issues do let!(:group_closed_issue) do create :closed_issue, author: user, - assignee: user, + assignees: [user], project: group_project, state: :closed, milestone: group_milestone, @@ -276,13 +276,13 @@ describe API::Issues do :confidential, project: group_project, author: author, - assignee: assignee, + assignees: [assignee], updated_at: 2.hours.ago end let!(:group_issue) do create :issue, author: user, - assignee: user, + assignees: [user], project: group_project, milestone: group_milestone, updated_at: 1.hour.ago, @@ -687,6 +687,7 @@ describe API::Issues do expect(json_response['updated_at']).to be_present expect(json_response['labels']).to eq(issue.label_names) expect(json_response['milestone']).to be_a Hash + expect(json_response['assignees']).to be_a Array expect(json_response['assignee']).to be_a Hash expect(json_response['author']).to be_a Hash expect(json_response['confidential']).to be_falsy @@ -759,15 +760,38 @@ describe API::Issues do end describe "POST /projects/:id/issues" do + context 'support for deprecated assignee_id' do + it 'creates a new project issue' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', assignee_id: user2.id + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['assignee']['name']).to eq(user2.name) + expect(json_response['assignees'].first['name']).to eq(user2.name) + end + end + it 'creates a new project issue' do post api("/projects/#{project.id}/issues", user), +<<<<<<< HEAD title: 'new issue', labels: 'label, label2' +======= + title: 'new issue', labels: 'label, label2', weight: 3, + assignee_ids: [user2.id] +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master expect(response).to have_http_status(201) expect(json_response['title']).to eq('new issue') expect(json_response['description']).to be_nil expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['confidential']).to be_falsy +<<<<<<< HEAD +======= + expect(json_response['weight']).to eq(3) + expect(json_response['assignee']['name']).to eq(user2.name) + expect(json_response['assignees'].first['name']).to eq(user2.name) +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master end it 'creates a new confidential project issue' do @@ -1057,6 +1081,46 @@ describe API::Issues do end end + describe 'PUT /projects/:id/issues/:issue_iid to update assignee' do + context 'support for deprecated assignee_id' do + it 'removes assignee' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + assignee_id: 0 + + expect(response).to have_http_status(200) + + expect(json_response['assignee']).to be_nil + end + + it 'updates an issue with new assignee' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + assignee_id: user2.id + + expect(response).to have_http_status(200) + + expect(json_response['assignee']['name']).to eq(user2.name) + end + end + + it 'removes assignee' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + assignee_ids: [0] + + expect(response).to have_http_status(200) + + expect(json_response['assignees']).to be_empty + end + + it 'updates an issue with new assignee' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + assignee_ids: [user2.id] + + expect(response).to have_http_status(200) + + expect(json_response['assignees'].first['name']).to eq(user2.name) + end + end + describe 'PUT /projects/:id/issues/:issue_iid to update labels' do let!(:label) { create(:label, title: 'dummy', project: project) } let!(:label_link) { create(:label_link, label: label, target: issue) } diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb index ef5b10a1615..a1124aebcec 100644 --- a/spec/requests/api/v3/issues_spec.rb +++ b/spec/requests/api/v3/issues_spec.rb @@ -14,7 +14,7 @@ describe API::V3::Issues do let!(:closed_issue) do create :closed_issue, author: user, - assignee: user, + assignees: [user], project: project, state: :closed, milestone: milestone, @@ -26,14 +26,14 @@ describe API::V3::Issues do :confidential, project: project, author: author, - assignee: assignee, + assignees: [assignee], created_at: generate(:past_time), updated_at: 2.hours.ago end let!(:issue) do create :issue, author: user, - assignee: user, + assignees: [user], project: project, milestone: milestone, created_at: generate(:past_time), @@ -247,7 +247,7 @@ describe API::V3::Issues do let!(:group_closed_issue) do create :closed_issue, author: user, - assignee: user, + assignees: [user], project: group_project, state: :closed, milestone: group_milestone, @@ -258,13 +258,13 @@ describe API::V3::Issues do :confidential, project: group_project, author: author, - assignee: assignee, + assignees: [assignee], updated_at: 2.hours.ago end let!(:group_issue) do create :issue, author: user, - assignee: user, + assignees: [user], project: group_project, milestone: group_milestone, updated_at: 1.hour.ago @@ -737,13 +737,22 @@ describe API::V3::Issues do describe "POST /projects/:id/issues" do it 'creates a new project issue' do post v3_api("/projects/#{project.id}/issues", user), +<<<<<<< HEAD title: 'new issue', labels: 'label, label2' +======= + title: 'new issue', labels: 'label, label2', weight: 3, assignee_id: assignee.id +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master expect(response).to have_http_status(201) expect(json_response['title']).to eq('new issue') expect(json_response['description']).to be_nil expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['confidential']).to be_falsy +<<<<<<< HEAD +======= + expect(json_response['weight']).to eq(3) + expect(json_response['assignee']['name']).to eq(assignee.name) +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master end it 'creates a new confidential project issue' do @@ -1140,6 +1149,57 @@ describe API::V3::Issues do end end +<<<<<<< HEAD +======= + describe 'PUT /projects/:id/issues/:issue_id to update assignee' do + it 'updates an issue with no assignee' do + put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), assignee_id: 0 + + expect(response).to have_http_status(200) + expect(json_response['assignee']).to eq(nil) + end + + it 'updates an issue with assignee' do + put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), assignee_id: user2.id + + expect(response).to have_http_status(200) + expect(json_response['assignee']['name']).to eq(user2.name) + end + end + + describe 'PUT /projects/:id/issues/:issue_id to update weight' do + it 'updates an issue with no weight' do + put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), weight: 5 + + expect(response).to have_http_status(200) + expect(json_response['weight']).to eq(5) + end + + it 'removes a weight from an issue' do + weighted_issue = create(:issue, project: project, weight: 2) + + put v3_api("/projects/#{project.id}/issues/#{weighted_issue.id}", user), weight: nil + + expect(response).to have_http_status(200) + expect(json_response['weight']).to be_nil + end + + it 'returns 400 if weight is less than minimum weight' do + put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), weight: -1 + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('weight does not have a valid value') + end + + it 'returns 400 if weight is more than maximum weight' do + put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), weight: 10 + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('weight does not have a valid value') + end + end + +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master describe "DELETE /projects/:id/issues/:issue_id" do it "rejects a non member from deleting an issue" do delete v3_api("/projects/#{project.id}/issues/#{issue.id}", non_member) diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 7a1ac027310..5b1639ca0d6 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -4,11 +4,12 @@ describe Issuable::BulkUpdateService, services: true do let(:user) { create(:user) } let(:project) { create(:empty_project, namespace: user.namespace) } - def bulk_update(issues, extra_params = {}) + def bulk_update(issuables, extra_params = {}) bulk_update_params = extra_params - .reverse_merge(issuable_ids: Array(issues).map(&:id).join(',')) + .reverse_merge(issuable_ids: Array(issuables).map(&:id).join(',')) - Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute('issue') + type = Array(issuables).first.model_name.param_key + Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute(type) end describe 'close issues' do @@ -47,15 +48,15 @@ describe Issuable::BulkUpdateService, services: true do end end - describe 'updating assignee' do - let(:issue) { create(:issue, project: project, assignee: user) } + describe 'updating merge request assignee' do + let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignee: user) } context 'when the new assignee ID is a valid user' do it 'succeeds' do new_assignee = create(:user) project.team << [new_assignee, :developer] - result = bulk_update(issue, assignee_id: new_assignee.id) + result = bulk_update(merge_request, assignee_id: new_assignee.id) expect(result[:success]).to be_truthy expect(result[:count]).to eq(1) @@ -65,22 +66,59 @@ describe Issuable::BulkUpdateService, services: true do assignee = create(:user) project.team << [assignee, :developer] - expect { bulk_update(issue, assignee_id: assignee.id) } - .to change { issue.reload.assignee }.from(user).to(assignee) + expect { bulk_update(merge_request, assignee_id: assignee.id) } + .to change { merge_request.reload.assignee }.from(user).to(assignee) end end context "when the new assignee ID is #{IssuableFinder::NONE}" do it "unassigns the issues" do - expect { bulk_update(issue, assignee_id: IssuableFinder::NONE) } - .to change { issue.reload.assignee }.to(nil) + expect { bulk_update(merge_request, assignee_id: IssuableFinder::NONE) } + .to change { merge_request.reload.assignee }.to(nil) end end context 'when the new assignee ID is not present' do it 'does not unassign' do - expect { bulk_update(issue, assignee_id: nil) } - .not_to change { issue.reload.assignee } + expect { bulk_update(merge_request, assignee_id: nil) } + .not_to change { merge_request.reload.assignee } + end + end + end + + describe 'updating issue assignee' do + let(:issue) { create(:issue, project: project, assignees: [user]) } + + context 'when the new assignee ID is a valid user' do + it 'succeeds' do + new_assignee = create(:user) + project.team << [new_assignee, :developer] + + result = bulk_update(issue, assignee_ids: [new_assignee.id]) + + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(1) + end + + it 'updates the assignee to the use ID passed' do + assignee = create(:user) + project.team << [assignee, :developer] + expect { bulk_update(issue, assignee_ids: [assignee.id]) } + .to change { issue.reload.assignees.first }.from(user).to(assignee) + end + end + + context "when the new assignee ID is #{IssuableFinder::NONE}" do + it "unassigns the issues" do + expect { bulk_update(issue, assignee_ids: [IssuableFinder::NONE.to_s]) } + .to change { issue.reload.assignees.count }.from(1).to(0) + end + end + + context 'when the new assignee ID is not present' do + it 'does not unassign' do + expect { bulk_update(issue, assignee_ids: []) } + .not_to change{ issue.reload.assignees } end end end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 7a54373963e..51840531711 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -4,7 +4,7 @@ describe Issues::CloseService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:issue) { create(:issue, assignee: user2) } + let(:issue) { create(:issue, assignees: [user2]) } let(:project) { issue.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 80bfb731550..01edc46496d 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -6,10 +6,10 @@ describe Issues::CreateService, services: true do describe '#execute' do let(:issue) { described_class.new(project, user, opts).execute } + let(:assignee) { create(:user) } + let(:milestone) { create(:milestone, project: project) } context 'when params are valid' do - let(:assignee) { create(:user) } - let(:milestone) { create(:milestone, project: project) } let(:labels) { create_pair(:label, project: project) } before do @@ -20,7 +20,7 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee_id: assignee.id, + assignee_ids: [assignee.id], label_ids: labels.map(&:id), milestone_id: milestone.id, due_date: Date.tomorrow } @@ -29,7 +29,7 @@ describe Issues::CreateService, services: true do it 'creates the issue with the given params' do expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') - expect(issue.assignee).to eq assignee + expect(issue.assignees).to eq [assignee] expect(issue.labels).to match_array labels expect(issue.milestone).to eq milestone expect(issue.due_date).to eq Date.tomorrow @@ -37,6 +37,7 @@ describe Issues::CreateService, services: true do context 'when current user cannot admin issues in the project' do let(:guest) { create(:user) } + before do project.team << [guest, :guest] end @@ -47,7 +48,7 @@ describe Issues::CreateService, services: true do expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') expect(issue.description).to eq('please fix') - expect(issue.assignee).to be_nil + expect(issue.assignees).to be_empty expect(issue.labels).to be_empty expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil @@ -136,10 +137,83 @@ describe Issues::CreateService, services: true do end end - it_behaves_like 'issuable create service' + context 'issue create service' do + context 'assignees' do + before { project.team << [user, :master] } + + it 'removes assignee when user id is invalid' do + opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + + it 'removes assignee when user id is 0' do + opts = { title: 'Title', description: 'Description', assignee_ids: [0] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + + it 'saves assignee when user id is valid' do + project.team << [assignee, :master] + opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to eq([assignee]) + end + + context "when issuable feature is private" do + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE) + end + + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + 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_ids: [assignee.id] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + end + end + end + end it_behaves_like 'new issuable record that supports slash commands' + context 'Slash commands' do + context 'with assignee and milestone in params and command' do + let(:opts) do + { + assignee_ids: [create(:user).id], + milestone_id: 1, + title: 'Title', + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :master] + end + + it 'assigns and sets milestone to issuable from command' do + expect(issue).to be_persisted + expect(issue.assignees).to eq([assignee]) + expect(issue.milestone).to eq(milestone) + end + end + end + context 'resolving discussions' do let(:discussion) { create(:diff_note_on_merge_request).to_discussion } let(:merge_request) { discussion.noteable } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 5b324f3c706..1797a23ee8a 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -14,7 +14,7 @@ describe Issues::UpdateService, services: true do let(:issue) do create(:issue, title: 'Old title', description: "for #{user2.to_reference}", - assignee_id: user3.id, + assignee_ids: [user3.id], project: project) end @@ -40,7 +40,7 @@ describe Issues::UpdateService, services: true do { title: 'New title', description: 'Also please fix', - assignee_id: user2.id, + assignee_ids: [user2.id, user3.id], state_event: 'close', label_ids: [label.id], due_date: Date.tomorrow @@ -53,15 +53,15 @@ describe Issues::UpdateService, services: true do expect(issue).to be_valid expect(issue.title).to eq 'New title' expect(issue.description).to eq 'Also please fix' - expect(issue.assignee).to eq user2 + expect(issue.assignees).to match_array([user2, user3]) expect(issue).to be_closed expect(issue.labels).to match_array [label] expect(issue.due_date).to eq Date.tomorrow end it 'sorts issues as specified by parameters' do - issue1 = create(:issue, project: project, assignee_id: user3.id) - issue2 = create(:issue, project: project, assignee_id: user3.id) + issue1 = create(:issue, project: project, assignees: [user3]) + issue2 = create(:issue, project: project, assignees: [user3]) [issue, issue1, issue2].each do |issue| issue.move_to_end @@ -87,7 +87,7 @@ describe Issues::UpdateService, services: true do expect(issue).to be_valid expect(issue.title).to eq 'New title' expect(issue.description).to eq 'Also please fix' - expect(issue.assignee).to eq user3 + expect(issue.assignees).to match_array [user3] expect(issue.labels).to be_empty expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil @@ -137,7 +137,7 @@ describe Issues::UpdateService, services: true do { title: 'New title', description: 'Also please fix', - assignee_id: user2.id, + assignee_ids: [user2], state_event: 'close', label_ids: [label.id], confidential: true @@ -163,12 +163,12 @@ describe Issues::UpdateService, services: true do it 'does not update assignee_id with unauthorized users' do project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) update_issue(confidential: true) - non_member = create(:user) - original_assignee = issue.assignee + non_member = create(:user) + original_assignees = issue.assignees - update_issue(assignee_id: non_member.id) + update_issue(assignee_ids: [non_member.id]) - expect(issue.reload.assignee_id).to eq(original_assignee.id) + expect(issue.reload.assignees).to eq(original_assignees) end end @@ -205,7 +205,7 @@ describe Issues::UpdateService, services: true do context 'when is reassigned' do before do - update_issue(assignee: user2) + update_issue(assignees: [user2]) end it 'marks previous assignee todos as done' do @@ -408,6 +408,41 @@ describe Issues::UpdateService, services: true do end end + context 'updating asssignee_id' do + it 'does not update assignee when assignee_id is invalid' do + update_issue(assignee_ids: [-1]) + + expect(issue.reload.assignees).to eq([user3]) + end + + it 'unassigns assignee when user id is 0' do + update_issue(assignee_ids: [0]) + + expect(issue.reload.assignees).to be_empty + end + + it 'does not update assignee_id when user cannot read issue' do + update_issue(assignee_ids: [create(:user).id]) + + expect(issue.reload.assignees).to eq([user3]) + end + + context "when issuable feature is private" do + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + assignee = create(:user) + project.update(visibility_level: level) + feature_visibility_attr = :"#{issue.model_name.plural}_access_level" + project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) + + expect{ update_issue(assignee_ids: [assignee.id]) }.not_to change{ issue.assignees } + end + end + end + end + context 'updating mentions' do let(:mentionable) { issue } include_examples 'updating mentions', Issues::UpdateService diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index fe75757dd29..d3556020d4d 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -15,14 +15,14 @@ describe MergeRequests::AssignIssuesService, services: true do expect(service.assignable_issues.map(&:id)).to include(issue.id) end - it 'ignores issues already assigned to any user' do - issue.update!(assignee: create(:user)) + it 'ignores issues the user cannot update assignee on' do + project.team.truncate expect(service.assignable_issues).to be_empty end - it 'ignores issues the user cannot update assignee on' do - project.team.truncate + it 'ignores issues already assigned to any user' do + issue.assignees = [create(:user)] expect(service.assignable_issues).to be_empty end @@ -44,7 +44,7 @@ describe MergeRequests::AssignIssuesService, services: true do end it 'assigns these to the merge request owner' do - expect { service.execute }.to change { issue.reload.assignee }.to(user) + expect { service.execute }.to change { issue.assignees.first }.to(user) end it 'ignores external issues' do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 0e16c7cc94b..ace82380cc9 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -84,7 +84,87 @@ describe MergeRequests::CreateService, services: true do end end - it_behaves_like 'issuable create service' + context 'Slash commands' do + context 'with assignee and milestone in params and command' do + let(:merge_request) { described_class.new(project, user, opts).execute } + let(:milestone) { create(:milestone, project: project) } + + let(:opts) do + { + assignee_id: create(:user).id, + milestone_id: 1, + title: 'Title', + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}"), + source_branch: 'feature', + target_branch: 'master' + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :master] + end + + 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.milestone).to eq(milestone) + end + end + end + + context 'merge request create service' do + context 'asssignee_id' do + let(:assignee) { create(:user) } + + before { project.team << [user, :master] } + + it 'removes assignee_id when user id is invalid' do + opts = { title: 'Title', description: 'Description', assignee_id: -1 } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + + it 'removes assignee_id when user id is 0' do + opts = { title: 'Title', description: 'Description', assignee_id: 0 } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + project.team << [assignee, :master] + opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee).to eq(assignee) + end + + context "when issuable feature is private" do + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE) + end + + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + 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 } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + end + end + end + end context 'while saving references to issues that the created merge request closes' do let(:first_issue) { create(:issue, project: project) } diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f2ca1e6fcbd..694ec3a579f 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -423,6 +423,54 @@ describe MergeRequests::UpdateService, services: true do end end + context 'updating asssignee_id' do + it 'does not update assignee when assignee_id is invalid' do + merge_request.update(assignee_id: user.id) + + update_merge_request(assignee_id: -1) + + expect(merge_request.reload.assignee).to eq(user) + end + + it 'unassigns assignee when user id is 0' do + merge_request.update(assignee_id: user.id) + + update_merge_request(assignee_id: 0) + + expect(merge_request.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + update_merge_request(assignee_id: user.id) + + expect(merge_request.assignee_id).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 + + update_merge_request(assignee_id: non_member.id) + + expect(merge_request.assignee_id).to eq(original_assignee.id) + end + + context "when issuable feature is private" do + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + assignee = create(:user) + project.update(visibility_level: level) + 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 } + end + end + end + end + include_examples 'issuable update service' do let(:open_issuable) { merge_request } let(:closed_issuable) { create(:closed_merge_request, source_project: project) } diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index 1a64c8bbf00..0edcc50ed7b 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -66,7 +66,7 @@ describe Notes::SlashCommandsService, services: true do expect(content).to eq '' expect(note.noteable).to be_closed expect(note.noteable.labels).to match_array(labels) - expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.assignees).to eq([assignee]) expect(note.noteable.milestone).to eq(milestone) end end @@ -113,7 +113,7 @@ describe Notes::SlashCommandsService, services: true do expect(content).to eq "HELLO\nWORLD" expect(note.noteable).to be_closed expect(note.noteable.labels).to match_array(labels) - expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.assignees).to eq([assignee]) expect(note.noteable.milestone).to eq(milestone) end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 989fd90cda9..74f96b97909 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -4,6 +4,7 @@ describe NotificationService, services: true do include EmailHelpers let(:notification) { NotificationService.new } + let(:assignee) { create(:user) } around(:each) do |example| perform_enqueued_jobs do @@ -52,7 +53,11 @@ describe NotificationService, services: true do shared_examples 'participating by assignee notification' do it 'emails the participant' do - issuable.update_attribute(:assignee, participant) + if issuable.is_a?(Issue) + issuable.assignees << participant + else + issuable.update_attribute(:assignee, participant) + end notification_trigger @@ -103,14 +108,14 @@ describe NotificationService, services: true do describe 'Notes' do context 'issue note' do let(:project) { create(:empty_project, :private) } - let(:issue) { create(:issue, project: project, assignee: create(:user)) } - let(:mentioned_issue) { create(:issue, assignee: issue.assignee) } + let(:issue) { create(:issue, project: project, assignees: [assignee]) } + let(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @outsider also') } before do build_team(note.project) project.add_master(issue.author) - project.add_master(issue.assignee) + project.add_master(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, resource: project) @@ -130,7 +135,7 @@ describe NotificationService, services: true do should_email(@u_watcher) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_email(@u_custom_global) should_email(@u_mentioned) should_email(@subscriber) @@ -196,7 +201,7 @@ describe NotificationService, services: true do notification.new_note(note) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_email(@u_mentioned) should_email(@u_custom_global) should_not_email(@u_guest_custom) @@ -218,7 +223,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") } let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") } @@ -244,8 +249,8 @@ describe NotificationService, services: true do context 'issue note mention' do let(:project) { create(:empty_project, :public) } - let(:issue) { create(:issue, project: project, assignee: create(:user)) } - let(:mentioned_issue) { create(:issue, assignee: issue.assignee) } + let(:issue) { create(:issue, project: project, assignees: [assignee]) } + let(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@all mentioned') } before do @@ -269,7 +274,7 @@ describe NotificationService, services: true do should_email(@u_guest_watcher) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_not_email(note.author) should_email(@u_mentioned) should_not_email(@u_disabled) @@ -449,7 +454,7 @@ describe NotificationService, services: true do let(:group) { create(:group) } let(:project) { create(:empty_project, :public, namespace: group) } let(:another_project) { create(:empty_project, :public, namespace: group) } - let(:issue) { create :issue, project: project, assignee: create(:user), description: 'cc @participant' } + let(:issue) { create :issue, project: project, assignees: [assignee], description: 'cc @participant' } before do build_team(issue.project) @@ -465,7 +470,7 @@ describe NotificationService, services: true do it do notification.new_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(assignee) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -480,10 +485,10 @@ describe NotificationService, services: true do end it do - create_global_setting_for(issue.assignee, :mention) + create_global_setting_for(issue.assignees.first, :mention) notification.new_issue(issue, @u_disabled) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) end it "emails the author if they've opted into notifications about their activity" do @@ -528,7 +533,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - 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, assignees: [assignee]) } it "emails subscribers of the issue's labels that can read the issue" do project.add_developer(member) @@ -572,9 +577,9 @@ describe NotificationService, services: true do end it 'emails new assignee' do - notification.reassigned_issue(issue, @u_disabled) + notification.reassigned_issue(issue, @u_disabled, [assignee]) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -588,9 +593,8 @@ describe NotificationService, services: true do end it 'emails previous assignee even if he has the "on mention" notif level' do - issue.update_attribute(:assignee, @u_mentioned) - issue.update_attributes(assignee: @u_watcher) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_watcher]) should_email(@u_mentioned) should_email(@u_watcher) @@ -606,11 +610,11 @@ describe NotificationService, services: true do end it 'emails new assignee even if he has the "on mention" notif level' do - issue.update_attributes(assignee: @u_mentioned) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned - should_email(issue.assignee) + expect(issue.assignees.first).to be @u_mentioned + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -624,11 +628,11 @@ describe NotificationService, services: true do end it 'emails new assignee' do - issue.update_attribute(:assignee, @u_mentioned) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned - should_email(issue.assignee) + expect(issue.assignees.first).to be @u_mentioned + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -642,17 +646,17 @@ describe NotificationService, services: true do end it 'does not email new assignee if they are the current user' do - issue.update_attribute(:assignee, @u_mentioned) - notification.reassigned_issue(issue, @u_mentioned) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_mentioned, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned + expect(issue.assignees.first).to be @u_mentioned should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) should_email(@u_participant_mentioned) should_email(@subscriber) should_email(@u_custom_global) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -662,7 +666,7 @@ describe NotificationService, services: true do it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { issue } - let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled) } + let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } end end @@ -705,7 +709,7 @@ describe NotificationService, services: true do it "doesn't send email to anyone but subscribers of the given labels" do notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) should_not_email(issue.author) should_not_email(@u_watcher) should_not_email(@u_guest_watcher) @@ -729,7 +733,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - 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, assignees: [assignee]) } let!(:label_1) { create(:label, project: project, issues: [confidential_issue]) } let!(:label_2) { create(:label, project: project) } @@ -767,7 +771,7 @@ describe NotificationService, services: true do it 'sends email to issue assignee and issue author' do notification.close_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) @@ -798,7 +802,7 @@ describe NotificationService, services: true do it 'sends email to issue notification recipients' do notification.reopen_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) @@ -826,7 +830,7 @@ describe NotificationService, services: true do it 'sends email to issue notification recipients' do notification.issue_moved(issue, new_issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index 7916c2d957c..c198c3eedfc 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -11,7 +11,7 @@ describe Projects::AutocompleteService, services: true do let(:project) { create(:empty_project, :public) } let!(:issue) { create(:issue, project: project, title: 'Issue 1') } let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } - let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignees: [assignee]) } it 'does not list project confidential issues for guests' do autocomplete = described_class.new(project, nil) diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index 29e65fe7ce6..865a7c69875 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do let(:project) { create(:empty_project, :public) } let(:developer) { create(:user) } + let(:developer2) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:inprogress) { create(:label, project: project, title: 'In Progress') } @@ -42,6 +43,7 @@ describe SlashCommands::InterpretService, services: true do end end +<<<<<<< HEAD shared_examples 'assign command' do it 'fetches assignee and populates assignee_id if content contains /assign' do _, updates = service.execute(content, issuable) @@ -59,6 +61,8 @@ describe SlashCommands::InterpretService, services: true do end end +======= +>>>>>>> b0a2435... Merge branch 'multiple_assignees_review_upstream' into ee_master shared_examples 'milestone command' do it 'fetches milestone and populates milestone_id if content contains /milestone' do milestone # populate the milestone @@ -371,14 +375,46 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end - it_behaves_like 'assign command' do + context 'assign command' do let(:content) { "/assign @#{developer.username}" } - let(:issuable) { issue } + + context 'Issue' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, issue) + + expect(updates).to eq(assignee_ids: [developer.id]) + end + end + + context 'Merge Request' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: developer.id) + end + end end - it_behaves_like 'assign command' do - let(:content) { "/assign @#{developer.username}" } - let(:issuable) { merge_request } + context 'assign command with multiple assignees' do + let(:content) { "/assign @#{developer.username} @#{developer2.username}" } + + before{ project.team << [developer2, :developer] } + + context 'Issue' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, issue) + + expect(updates[:assignee_ids]).to match_array([developer.id, developer2.id]) + end + end + + context 'Merge Request' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: developer.id) + end + end end it_behaves_like 'empty command' do @@ -391,14 +427,26 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end - it_behaves_like 'unassign command' do + context 'unassign command' do let(:content) { '/unassign' } - let(:issuable) { issue } - end - it_behaves_like 'unassign command' do - let(:content) { '/unassign' } - let(:issuable) { merge_request } + context 'Issue' do + it 'populates assignee_ids: [] if content contains /unassign' do + issue.update(assignee_ids: [developer.id]) + _, updates = service.execute(content, issue) + + expect(updates).to eq(assignee_ids: []) + end + end + + context 'Merge Request' do + it 'populates assignee_id: nil if content contains /unassign' do + merge_request.update(assignee_id: developer.id) + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: nil) + end + end end it_behaves_like 'milestone command' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 75d7caf2508..5f1b82e8355 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -6,6 +6,7 @@ describe SystemNoteService, services: true do let(:project) { create(:empty_project) } let(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } + let(:issue) { noteable } shared_examples_for 'a system note' do let(:expected_noteable) { noteable } @@ -155,6 +156,50 @@ describe SystemNoteService, services: true do end end + describe '.change_issue_assignees' do + subject { described_class.change_issue_assignees(noteable, project, author, [assignee]) } + + let(:assignee) { create(:user) } + let(:assignee1) { create(:user) } + let(:assignee2) { create(:user) } + let(:assignee3) { create(:user) } + + it_behaves_like 'a system note' + + def build_note(old_assignees, new_assignees) + issue.assignees = new_assignees + described_class.change_issue_assignees(issue, project, author, old_assignees).note + end + + it 'builds a correct phrase when an assignee is added to a non-assigned issue' do + expect(build_note([], [assignee1])).to eq "assigned to @#{assignee1.username}" + end + + it 'builds a correct phrase when assignee removed' do + expect(build_note([assignee1], [])).to eq 'removed all assignees' + end + + it 'builds a correct phrase when assignees changed' do + expect(build_note([assignee1], [assignee2])).to eq \ + "assigned to @#{assignee2.username} and unassigned @#{assignee1.username}" + end + + it 'builds a correct phrase when three assignees removed and one added' do + expect(build_note([assignee, assignee1, assignee2], [assignee3])).to eq \ + "assigned to @#{assignee3.username} and unassigned @#{assignee.username}, @#{assignee1.username}, and @#{assignee2.username}" + end + + it 'builds a correct phrase when one assignee changed from a set' do + expect(build_note([assignee, assignee1], [assignee, assignee2])).to eq \ + "assigned to @#{assignee2.username} and unassigned @#{assignee1.username}" + end + + it 'builds a correct phrase when one assignee removed from a set' do + expect(build_note([assignee, assignee1, assignee2], [assignee, assignee1])).to eq \ + "unassigned @#{assignee2.username}" + end + end + describe '.change_label' do subject { described_class.change_label(noteable, project, author, added, removed) } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 89b3b6aad10..175a42a32d9 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -25,11 +25,11 @@ describe TodoService, services: true do end describe 'Issues' do - let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } - let(:addressed_issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } - let(:unassigned_issue) { create(:issue, project: project, assignee: nil) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) } - let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: directly_addressed) } + let(:issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } + let(:unassigned_issue) { create(:issue, project: project, assignees: []) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: mentions) } + let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: directly_addressed) } describe '#new_issue' do it 'creates a todo if assigned' do @@ -43,7 +43,7 @@ describe TodoService, services: true do end it 'creates a todo if assignee is the current user' do - unassigned_issue.update_attribute(:assignee, john_doe) + unassigned_issue.assignees = [john_doe] service.new_issue(unassigned_issue, john_doe) should_create_todo(user: john_doe, target: unassigned_issue, author: john_doe, action: Todo::ASSIGNED) @@ -258,20 +258,20 @@ describe TodoService, services: true do describe '#reassigned_issue' do it 'creates a pending todo for new assignee' do - unassigned_issue.update_attribute(:assignee, john_doe) + 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.update_attribute(:assignee, nil) + issue.assignees.destroy_all 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.update_attribute(:assignee, john_doe) + 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) @@ -361,7 +361,7 @@ describe TodoService, services: true do describe '#new_note' do let!(:first_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) } let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) } let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } @@ -854,7 +854,7 @@ describe TodoService, services: true do end it 'updates cached counts when a todo is created' do - issue = create(:issue, project: project, assignee: john_doe, author: author, description: mentions) + issue = create(:issue, project: project, assignees: [john_doe], author: author, description: mentions) expect(john_doe.todos_pending_count).to eq(0) expect(john_doe).to receive(:update_todos_count_cache).and_call_original @@ -866,8 +866,8 @@ describe TodoService, services: true do end describe '#mark_todos_as_done' do - let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) } - let(:another_issue) { create(:issue, project: project, author: author, assignee: john_doe) } + let(:issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } + let(:another_issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } it 'marks a relation of todos as done' do create(:todo, :mentioned, user: john_doe, target: issue, project: project) diff --git a/spec/support/features/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb index 5bbe36d9b7f..f7499ede09c 100644 --- a/spec/support/features/issuable_slash_commands_shared_examples.rb +++ b/spec/support/features/issuable_slash_commands_shared_examples.rb @@ -62,7 +62,7 @@ shared_examples 'issuable record that supports slash commands in its description note = issuable.notes.user.first expect(note.note).to eq "Awesome!" - expect(issuable.assignee).to eq assignee + expect(issuable.assignees).to eq [assignee] expect(issuable.labels).to eq [label_bug] expect(issuable.milestone).to eq milestone end @@ -80,7 +80,7 @@ shared_examples 'issuable record that supports slash commands in its description issuable.reload expect(issuable.notes.user).to be_empty - expect(issuable.assignee).to eq assignee + expect(issuable.assignees).to eq [assignee] expect(issuable.labels).to eq [label_bug] expect(issuable.milestone).to eq milestone end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index 944ea30656f..57b6abe12b7 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -10,7 +10,7 @@ module ExportFileHelper create(:release, project: project) - issue = create(:issue, assignee: user, project: project) + issue = create(:issue, assignees: [user], project: project) snippet = create(:project_snippet, project: project) label = create(:label, project: project) milestone = create(:milestone, project: project) diff --git a/spec/support/services/issuable_create_service_shared_examples.rb b/spec/support/services/issuable_create_service_shared_examples.rb deleted file mode 100644 index 4f0c745b7ee..00000000000 --- a/spec/support/services/issuable_create_service_shared_examples.rb +++ /dev/null @@ -1,52 +0,0 @@ -shared_examples 'issuable create service' do - context 'asssignee_id' do - let(:assignee) { create(:user) } - - before { project.team << [user, :master] } - - it 'removes assignee_id when user id is invalid' do - opts = { title: 'Title', description: 'Description', assignee_id: -1 } - - issuable = described_class.new(project, user, opts).execute - - expect(issuable.assignee_id).to be_nil - end - - it 'removes assignee_id when user id is 0' do - opts = { title: 'Title', description: 'Description', assignee_id: 0 } - - issuable = described_class.new(project, user, opts).execute - - expect(issuable.assignee_id).to be_nil - end - - it 'saves assignee when user id is valid' do - project.team << [assignee, :master] - opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } - - issuable = described_class.new(project, user, opts).execute - - expect(issuable.assignee_id).to eq(assignee.id) - end - - context "when issuable feature is private" do - before do - project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, - merge_requests_access_level: ProjectFeature::PRIVATE) - end - - levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] - - 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 } - - issuable = described_class.new(project, user, opts).execute - - expect(issuable.assignee_id).to be_nil - end - end - end - end -end diff --git a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb index 9e9cdf3e48b..1dd3663b944 100644 --- a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb +++ b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb @@ -49,23 +49,7 @@ shared_examples 'new issuable record that supports slash commands' do it 'assigns and sets milestone to issuable' do expect(issuable).to be_persisted - expect(issuable.assignee).to eq(assignee) - expect(issuable.milestone).to eq(milestone) - end - end - - context 'with assignee and milestone in params and command' do - let(:example_params) do - { - assignee: create(:user), - milestone_id: 1, - description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") - } - end - - it 'assigns and sets milestone to issuable from command' do - expect(issuable).to be_persisted - expect(issuable.assignee).to eq(assignee) + expect(issuable.assignees).to eq([assignee]) expect(issuable.milestone).to eq(milestone) end end diff --git a/spec/support/services/issuable_update_service_shared_examples.rb b/spec/support/services/issuable_update_service_shared_examples.rb index 49cea1e608c..8947f20562f 100644 --- a/spec/support/services/issuable_update_service_shared_examples.rb +++ b/spec/support/services/issuable_update_service_shared_examples.rb @@ -18,52 +18,4 @@ shared_examples 'issuable update service' do end end end - - context 'asssignee_id' do - it 'does not update assignee when assignee_id is invalid' do - open_issuable.update(assignee_id: user.id) - - update_issuable(assignee_id: -1) - - expect(open_issuable.reload.assignee).to eq(user) - end - - it 'unassigns assignee when user id is 0' do - open_issuable.update(assignee_id: user.id) - - update_issuable(assignee_id: 0) - - expect(open_issuable.assignee_id).to be_nil - end - - it 'saves assignee when user id is valid' do - update_issuable(assignee_id: user.id) - - expect(open_issuable.assignee_id).to eq(user.id) - end - - it 'does not update assignee_id when user cannot read issue' do - non_member = create(:user) - original_assignee = open_issuable.assignee - - update_issuable(assignee_id: non_member.id) - - expect(open_issuable.assignee_id).to eq(original_assignee.id) - end - - context "when issuable feature is private" do - levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] - - levels.each do |level| - it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do - assignee = create(:user) - project.update(visibility_level: level) - feature_visibility_attr = :"#{open_issuable.model_name.plural}_access_level" - project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - - expect{ update_issuable(assignee_id: assignee) }.not_to change{ open_issuable.assignee } - end - end - end - end end diff --git a/spec/support/time_tracking_shared_examples.rb b/spec/support/time_tracking_shared_examples.rb index 01bc80f957e..bd5d71f964a 100644 --- a/spec/support/time_tracking_shared_examples.rb +++ b/spec/support/time_tracking_shared_examples.rb @@ -34,7 +34,7 @@ shared_examples 'issuable time tracker' do submit_time('/estimate 3w 1d 1h') submit_time('/remove_estimate') - page.within '#issuable-time-tracker' do + page.within '.time-tracking-component-wrap' do expect(page).to have_content 'No estimate or time spent' end end @@ -43,13 +43,13 @@ shared_examples 'issuable time tracker' do submit_time('/spend 3w 1d 1h') submit_time('/remove_time_spent') - page.within '#issuable-time-tracker' do + page.within '.time-tracking-component-wrap' do expect(page).to have_content 'No estimate or time spent' end end it 'shows the help state when icon is clicked' do - page.within '#issuable-time-tracker' do + page.within '.time-tracking-component-wrap' do find('.help-button').click expect(page).to have_content 'Track time with slash commands' expect(page).to have_content 'Learn more' @@ -57,7 +57,7 @@ shared_examples 'issuable time tracker' do end it 'hides the help state when close icon is clicked' do - page.within '#issuable-time-tracker' do + page.within '.time-tracking-component-wrap' do find('.help-button').click find('.close-help-button').click @@ -67,7 +67,7 @@ shared_examples 'issuable time tracker' do end it 'displays the correct help url' do - page.within '#issuable-time-tracker' do + page.within '.time-tracking-component-wrap' do find('.help-button').click expect(find_link('Learn more')[:href]).to have_content('/help/workflow/time_tracking.md') |