diff options
Diffstat (limited to 'spec/features/merge_request')
68 files changed, 623 insertions, 119 deletions
diff --git a/spec/features/merge_request/batch_comments_spec.rb b/spec/features/merge_request/batch_comments_spec.rb new file mode 100644 index 00000000000..60671213d75 --- /dev/null +++ b/spec/features/merge_request/batch_comments_spec.rb @@ -0,0 +1,259 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Merge request > Batch comments', :js do + include MergeRequestDiffHelpers + include RepoHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + + before do + project.add_maintainer(user) + + sign_in(user) + end + + context 'Feature is enabled' do + before do + stub_feature_flags(diffs_batch_load: false) + + visit_diffs + end + + it 'has review bar' do + expect(page).to have_css('.review-bar-component', visible: false) + end + + it 'adds draft note' do + write_comment + + expect(find('.draft-note-component')).to have_content('Line is wrong') + + expect(page).to have_css('.review-bar-component') + + expect(find('.review-bar-content .btn-success')).to have_content('1') + end + + it 'publishes review' do + write_comment + + page.within('.review-bar-content') do + click_button 'Finish review' + click_button 'Submit review' + end + + wait_for_requests + + expect(page).not_to have_selector('.draft-note-component', text: 'Line is wrong') + + expect(page).to have_selector('.note:not(.draft-note)', text: 'Line is wrong') + end + + it 'publishes single comment' do + write_comment + + click_button 'Add comment now' + + wait_for_requests + + expect(page).not_to have_selector('.draft-note-component', text: 'Line is wrong') + + expect(page).to have_selector('.note:not(.draft-note)', text: 'Line is wrong') + end + + it 'discards review' do + write_comment + + click_button 'Discard review' + + click_button 'Delete all pending comments' + + wait_for_requests + + expect(page).not_to have_selector('.draft-note-component') + end + + it 'deletes draft note' do + write_comment + + accept_alert { find('.js-note-delete').click } + + wait_for_requests + + expect(page).not_to have_selector('.draft-note-component', text: 'Line is wrong') + end + + it 'edits draft note' do + write_comment + + find('.js-note-edit').click + + # make sure comment form is in view + execute_script("window.scrollBy(0, 200)") + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'Testing update') + click_button('Save comment') + end + + wait_for_requests + + expect(page).to have_selector('.draft-note-component', text: 'Testing update') + end + + context 'in parallel diff' do + before do + find('.js-show-diff-settings').click + click_button 'Side-by-side' + end + + it 'adds draft comments to both sides' do + write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9') + + # make sure line 9 is in the view + execute_script("window.scrollBy(0, -200)") + + write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9', button_text: 'Add to review', text: 'Another wrong line') + + expect(find('.new .draft-note-component')).to have_content('Line is wrong') + expect(find('.old .draft-note-component')).to have_content('Another wrong line') + + expect(find('.review-bar-content .btn-success')).to have_content('2') + end + end + + context 'thread is unresolved' do + let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + + before do + visit_diffs + end + + it 'publishes comment right away and resolves the thread' do + expect(active_discussion.resolved?).to eq(false) + + write_reply_to_discussion(button_text: 'Add comment now', resolve: true) + + page.within '.line-resolve-all-container' do + expect(page).to have_content('All threads resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'publishes review and resolves the thread' do + expect(active_discussion.resolved?).to eq(false) + + write_reply_to_discussion(resolve: true) + + page.within('.review-bar-content') do + click_button 'Finish review' + click_button 'Submit review' + end + + wait_for_requests + + page.within '.line-resolve-all-container' do + expect(page).to have_content('All threads resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + end + + context 'thread is resolved' do + let!(:active_discussion) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project).to_discussion } + + before do + active_discussion.resolve!(@current_user) + + visit_diffs + + page.find('.js-diff-comment-avatar').click + end + + it 'publishes comment right away and unresolves the thread' do + expect(active_discussion.resolved?).to eq(true) + + write_reply_to_discussion(button_text: 'Add comment now', unresolve: true) + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1 unresolved thread') + expect(page).not_to have_selector('.line-resolve-btn.is-active') + end + end + + it 'publishes review and unresolves the thread' do + expect(active_discussion.resolved?).to eq(true) + + wait_for_requests + + write_reply_to_discussion(button_text: 'Start a review', unresolve: true) + + page.within('.review-bar-content') do + click_button 'Finish review' + click_button 'Submit review' + end + + wait_for_requests + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1 unresolved thread') + expect(page).not_to have_selector('.line-resolve-btn.is-active') + end + end + end + end + + def visit_diffs + visit diffs_project_merge_request_path(merge_request.project, merge_request) + + wait_for_requests + end + + def write_comment(button_text: 'Start a review', text: 'Line is wrong') + click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: text) + click_button(button_text) + end + + wait_for_requests + end + + def write_parallel_comment(line, button_text: 'Start a review', text: 'Line is wrong') + find("td[id='#{line}']").hover + find(".is-over button").click + + page.within("form[data-line-code='#{line}']") do + fill_in('note_note', with: text) + click_button(button_text) + end + + wait_for_requests + end +end + +def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false) + page.within(first('.diff-files-holder .discussion-reply-holder')) do + click_button('Reply...') + + fill_in('note_note', with: text) + + if resolve + page.check('Resolve thread') + end + + if unresolve + page.check('Unresolve thread') + end + + click_button(button_text) + end + + wait_for_requests +end diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index 17ff494a6fa..4db1633abe6 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'a maintainer edits files on a source-branch of an MR from a fork', :js, :sidekiq_might_not_need_inline do +RSpec.describe 'a maintainer edits files on a source-branch of an MR from a fork', :js, :sidekiq_might_not_need_inline do include ProjectForksHelper let(:user) { create(:user, username: 'the-maintainer') } let(:target_project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_accepts_merge_request_spec.rb b/spec/features/merge_request/user_accepts_merge_request_spec.rb index 5e1ff232b80..d7c9c8bddb1 100644 --- a/spec/features/merge_request/user_accepts_merge_request_spec.rb +++ b/spec/features/merge_request/user_accepts_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User accepts a merge request', :js, :sidekiq_might_not_need_inline do +RSpec.describe 'User accepts a merge request', :js, :sidekiq_might_not_need_inline do let(:merge_request) { create(:merge_request, :with_diffs, :simple, source_project: project) } let(:project) { create(:project, :public, :repository) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb index 0ecd32565d0..fd13083c185 100644 --- a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb +++ b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do +RSpec.describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do include ProjectForksHelper let(:user) { create(:user) } let(:target_project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_assigns_themselves_spec.rb b/spec/features/merge_request/user_assigns_themselves_spec.rb index 549d6e50337..b6cd97dcc5a 100644 --- a/spec/features/merge_request/user_assigns_themselves_spec.rb +++ b/spec/features/merge_request/user_assigns_themselves_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User assigns themselves' do +RSpec.describe 'Merge request > User assigns themselves' do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:issue1) { create(:issue, project: project) } diff --git a/spec/features/merge_request/user_awards_emoji_spec.rb b/spec/features/merge_request/user_awards_emoji_spec.rb index 8aa90107251..62e4209f386 100644 --- a/spec/features/merge_request/user_awards_emoji_spec.rb +++ b/spec/features/merge_request/user_awards_emoji_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User awards emoji', :js do +RSpec.describe 'Merge request > User awards emoji', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, author: create(:user)) } diff --git a/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb b/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb new file mode 100644 index 00000000000..f3cbc1ea1f5 --- /dev/null +++ b/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User clicks on merge request tabs', :js do + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + it 'adds entry to page history' do + visit('/') + visit(merge_request_path(merge_request)) + click_link('Changes') + + expect(current_url).to match(/diffs$/) + + page.driver.go_back + + expect(current_url).to match(merge_request_path(merge_request)) + + page.driver.go_back + + expect(current_url).to match('/') + end +end diff --git a/spec/features/merge_request/user_closes_merge_request_spec.rb b/spec/features/merge_request/user_closes_merge_request_spec.rb index c5125c38ed7..669bd989f4f 100644 --- a/spec/features/merge_request/user_closes_merge_request_spec.rb +++ b/spec/features/merge_request/user_closes_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User closes a merge requests', :js do +RSpec.describe 'User closes a merge requests', :js do let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/user_comments_on_commit_spec.rb b/spec/features/merge_request/user_comments_on_commit_spec.rb index 6b869d93e4c..8fa1fe3812d 100644 --- a/spec/features/merge_request/user_comments_on_commit_spec.rb +++ b/spec/features/merge_request/user_comments_on_commit_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User comments on a commit', :js do +RSpec.describe 'User comments on a commit', :js do include MergeRequestDiffHelpers include RepoHelpers diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index 19b8a7f74b7..9cd3c7eaf76 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User comments on a diff', :js do +RSpec.describe 'User comments on a diff', :js do include MergeRequestDiffHelpers include RepoHelpers @@ -27,7 +27,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: 'Line is wrong') - click_button('Comment') + click_button('Add comment now') end page.within('.diff-files-holder > div:nth-child(3)') do @@ -46,7 +46,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: 'Line is correct') - click_button('Comment') + click_button('Add comment now') end wait_for_requests @@ -59,7 +59,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: 'Line is wrong') - click_button('Comment') + click_button('Add comment now') end wait_for_requests @@ -114,13 +114,47 @@ describe 'User comments on a diff', :js do include_examples 'comment on merge request file' end + context 'when adding multiline comments' do + it 'saves a multiline comment' do + click_diff_line(find("[id='#{sample_commit.line_code}']")) + + page.within('.discussion-form') do + find('#comment-line-start option', text: '-13').select_option + end + + page.within('.js-discussion-note-form') do + fill_in(:note_note, with: 'Line is wrong') + click_button('Add comment now') + end + + wait_for_requests + + page.within('.notes_holder') do + expect(page).to have_content('Line is wrong') + expect(page).to have_content('Comment on lines -13 to +14') + end + + visit(merge_request_path(merge_request)) + + page.within('.notes .discussion') do + expect(page).to have_content("#{user.name} #{user.to_reference} started a thread") + expect(page).to have_content(sample_commit.line_code_path) + expect(page).to have_content('Line is wrong') + end + + page.within('.notes-tab .badge') do + expect(page).to have_content('1') + end + end + end + context 'when editing comments' do it 'edits a comment' do click_diff_line(find("[id='#{sample_commit.line_code}']")) page.within('.js-discussion-note-form') do fill_in(:note_note, with: 'Line is wrong') - click_button('Comment') + click_button('Add comment now') end page.within('.diff-file:nth-of-type(5) .discussion .note') do @@ -146,7 +180,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in(:note_note, with: 'Line is wrong') - click_button('Comment') + click_button('Add comment now') end page.within('.notes-tab .badge') do diff --git a/spec/features/merge_request/user_comments_on_merge_request_spec.rb b/spec/features/merge_request/user_comments_on_merge_request_spec.rb index c7845b4cce4..73f2b1a25ce 100644 --- a/spec/features/merge_request/user_comments_on_merge_request_spec.rb +++ b/spec/features/merge_request/user_comments_on_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User comments on a merge request', :js do +RSpec.describe 'User comments on a merge request', :js do include RepoHelpers let(:project) { create(:project, :repository) } diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index cea9056cd93..34eaca24a01 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User creates image diff notes', :js do +RSpec.describe 'Merge request > User creates image diff notes', :js do include NoteInteractionHelpers let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_creates_merge_request_spec.rb b/spec/features/merge_request/user_creates_merge_request_spec.rb index 86ee9fa5aa5..37d329d4d5d 100644 --- a/spec/features/merge_request/user_creates_merge_request_spec.rb +++ b/spec/features/merge_request/user_creates_merge_request_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe "User creates a merge request", :js do +RSpec.describe "User creates a merge request", :js do include ProjectForksHelper let_it_be(:project) { create(:project, :repository) } diff --git a/spec/features/merge_request/user_creates_mr_spec.rb b/spec/features/merge_request/user_creates_mr_spec.rb index 665bc352c0f..9d97e57fe3a 100644 --- a/spec/features/merge_request/user_creates_mr_spec.rb +++ b/spec/features/merge_request/user_creates_mr_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User creates MR' do +RSpec.describe 'Merge request > User creates MR' do include ProjectForksHelper before do diff --git a/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb b/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb index 895cbb8f02b..23df7635aa1 100644 --- a/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb +++ b/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request < User customizes merge commit message', :js do +RSpec.describe 'Merge request < User customizes merge commit message', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:issue_1) { create(:issue, project: project)} diff --git a/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb b/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb index e6b77e28281..affd6f6b7b5 100644 --- a/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb +++ b/spec/features/merge_request/user_edits_assignees_sidebar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User edits assignees sidebar', :js do +RSpec.describe 'Merge request > User edits assignees sidebar', :js do let(:project) { create(:project, :public, :repository) } let(:protected_branch) { create(:protected_branch, :maintainers_can_push, name: 'master', project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project, target_branch: protected_branch.name) } @@ -20,49 +20,102 @@ describe 'Merge request > User edits assignees sidebar', :js do let(:sidebar_assignee_dropdown_item) { sidebar_assignee_block.find(".dropdown-menu li[data-user-id=\"#{assignee.id}\"]") } let(:sidebar_assignee_dropdown_tooltip) { sidebar_assignee_dropdown_item.find('a')['data-title'] || '' } - before do - stub_const('Autocomplete::UsersFinder::LIMIT', users_find_limit) + context 'when invite_members_version_a experiment is not enabled' do + before do + stub_const('Autocomplete::UsersFinder::LIMIT', users_find_limit) - sign_in(project.owner) + sign_in(project.owner) - merge_request.assignees << assignee + merge_request.assignees << assignee - visit project_merge_request_path(project, merge_request) + visit project_merge_request_path(project, merge_request) - wait_for_requests - end + wait_for_requests + end + + shared_examples 'when assigned' do |expected_tooltip: ''| + it 'shows assignee name' do + expect(sidebar_assignee_block).to have_text(assignee.name) + end + + it "shows assignee tooltip '#{expected_tooltip}'" do + expect(sidebar_assignee_tooltip).to eql(expected_tooltip) + end + + context 'when edit is clicked' do + before do + sidebar_assignee_block.click_link('Edit') + + wait_for_requests + end + + it "shows assignee tooltip '#{expected_tooltip}" do + expect(sidebar_assignee_dropdown_tooltip).to eql(expected_tooltip) + end + + it 'does not show invite link' do + page.within '.dropdown-menu-user' do + expect(page).not_to have_link('Invite Members') + end + end + end + end + + context 'when assigned to maintainer' do + let(:assignee) { project_maintainers.last } - shared_examples 'when assigned' do |expected_tooltip: ''| - it 'shows assignee name' do - expect(sidebar_assignee_block).to have_text(assignee.name) + it_behaves_like 'when assigned', expected_tooltip: '' end - it "shows assignee tooltip '#{expected_tooltip}'" do - expect(sidebar_assignee_tooltip).to eql(expected_tooltip) + context 'when assigned to developer' do + let(:assignee) { project_developers.last } + + it_behaves_like 'when assigned', expected_tooltip: 'Cannot merge' end + end - context 'when edit is clicked' do + context 'when invite_members_version_a experiment is enabled' do + let_it_be(:user) { create(:user) } + + before do + stub_experiment_for_user(invite_members_version_a: true) + sign_in(user) + end + + context 'when user can not see invite members' do before do - sidebar_assignee_block.click_link('Edit') + project.add_developer(user) + visit project_merge_request_path(project, merge_request) + + find('.block.assignee .edit-link').click wait_for_requests end - it "shows assignee tooltip '#{expected_tooltip}" do - expect(sidebar_assignee_dropdown_tooltip).to eql(expected_tooltip) + it 'does not see link to invite members' do + page.within '.dropdown-menu-user' do + expect(page).not_to have_link('Invite Members') + end end end - end - context 'when assigned to maintainer' do - let(:assignee) { project_maintainers.last } + context 'when user can see invite members' do + before do + project.add_maintainer(user) + visit project_merge_request_path(project, merge_request) - it_behaves_like 'when assigned', expected_tooltip: '' - end + find('.block.assignee .edit-link').click - context 'when assigned to developer' do - let(:assignee) { project_developers.last } + wait_for_requests + end - it_behaves_like 'when assigned', expected_tooltip: 'Cannot merge' + it 'sees link to invite members' do + page.within '.dropdown-menu-user' do + expect(page).to have_link('Invite Members', href: project_project_members_path(project)) + expect(page).to have_selector('[data-track-event="click_invite_members"]') + expect(page).to have_selector("[data-track-label='edit_assignee']") + end + end + end end end diff --git a/spec/features/merge_request/user_edits_merge_request_spec.rb b/spec/features/merge_request/user_edits_merge_request_spec.rb index 821db8a1d5b..84ecd6537dc 100644 --- a/spec/features/merge_request/user_edits_merge_request_spec.rb +++ b/spec/features/merge_request/user_edits_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User edits a merge request', :js do +RSpec.describe 'User edits a merge request', :js do include Select2Helper let(:project) { create(:project, :repository) } diff --git a/spec/features/merge_request/user_edits_mr_spec.rb b/spec/features/merge_request/user_edits_mr_spec.rb index e6b847c5e7f..2c949ed84f4 100644 --- a/spec/features/merge_request/user_edits_mr_spec.rb +++ b/spec/features/merge_request/user_edits_mr_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User edits MR' do +RSpec.describe 'Merge request > User edits MR' do include ProjectForksHelper before do diff --git a/spec/features/merge_request/user_expands_diff_spec.rb b/spec/features/merge_request/user_expands_diff_spec.rb index 9bce5264817..d3867a91846 100644 --- a/spec/features/merge_request/user_expands_diff_spec.rb +++ b/spec/features/merge_request/user_expands_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User expands diff', :js do +RSpec.describe 'User expands diff', :js do let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) } diff --git a/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb index 2a4192374bd..1a7baff2fb1 100644 --- a/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb +++ b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Batch diffs', :js do +RSpec.describe 'Batch diffs', :js do include MergeRequestDiffHelpers include RepoHelpers @@ -22,14 +22,14 @@ describe 'Batch diffs', :js do click_diff_line(find('.diff-file.file-holder:first-of-type tr.line_holder.new:first-of-type')) page.within('.js-discussion-note-form') do fill_in('note_note', with: 'First Line Comment') - click_button('Comment') + click_button('Add comment now') end # Add discussion to first line of last file click_diff_line(find('.diff-file.file-holder:last-of-type tr.line_holder.new:first-of-type')) page.within('.js-discussion-note-form') do fill_in('note_note', with: 'Last Line Comment') - click_button('Comment') + click_button('Add comment now') end wait_for_requests diff --git a/spec/features/merge_request/user_locks_discussion_spec.rb b/spec/features/merge_request/user_locks_discussion_spec.rb index 0eaaf32dc31..c8a6fdd4007 100644 --- a/spec/features/merge_request/user_locks_discussion_spec.rb +++ b/spec/features/merge_request/user_locks_discussion_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User locks discussion', :js do +RSpec.describe 'Merge request > User locks discussion', :js do let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } diff --git a/spec/features/merge_request/user_manages_subscription_spec.rb b/spec/features/merge_request/user_manages_subscription_spec.rb index 54d27a06bb1..9ed5b67fa0e 100644 --- a/spec/features/merge_request/user_manages_subscription_spec.rb +++ b/spec/features/merge_request/user_manages_subscription_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User manages subscription', :js do +RSpec.describe 'User manages subscription', :js do let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/user_merges_immediately_spec.rb b/spec/features/merge_request/user_merges_immediately_spec.rb index 1188d3b2ceb..47dc09ae79f 100644 --- a/spec/features/merge_request/user_merges_immediately_spec.rb +++ b/spec/features/merge_request/user_merges_immediately_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge requests > User merges immediately', :js do +RSpec.describe 'Merge requests > User merges immediately', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let!(:merge_request) do diff --git a/spec/features/merge_request/user_merges_merge_request_spec.rb b/spec/features/merge_request/user_merges_merge_request_spec.rb index 32e40740a61..7758fa8e666 100644 --- a/spec/features/merge_request/user_merges_merge_request_spec.rb +++ b/spec/features/merge_request/user_merges_merge_request_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe "User merges a merge request", :js do +RSpec.describe "User merges a merge request", :js do let(:user) { project.owner } before do diff --git a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb index 419f741d0ea..ea3e90a4508 100644 --- a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb +++ b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User merges only if pipeline succeeds', :js do +RSpec.describe 'Merge request > User merges only if pipeline succeeds', :js do let(:merge_request) { create(:merge_request_with_diffs) } let(:project) { merge_request.target_project } diff --git a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb index 5cc61333bb4..d5ff31de073 100644 --- a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb +++ b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User merges when pipeline succeeds', :js do +RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) do diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index ebfb5ce796f..dbad2f191a1 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User posts diff notes', :js do +RSpec.describe 'Merge request > User posts diff notes', :js do include MergeRequestDiffHelpers let(:merge_request) { create(:merge_request) } @@ -225,7 +225,7 @@ describe 'Merge request > User posts diff notes', :js do def should_allow_commenting(line_holder, diff_side = nil, asset_form_reset: true) write_comment_on_line(line_holder, diff_side) - click_button 'Comment' + click_button 'Add comment now' wait_for_requests diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index 0548d958322..3c70819319d 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User posts notes', :js do +RSpec.describe 'Merge request > User posts notes', :js do include NoteInteractionHelpers let_it_be(:project) { create(:project, :repository) } @@ -105,7 +105,7 @@ describe 'Merge request > User posts notes', :js do page.within('.discussion-reply-holder') do fill_in 'note[note]', with: 'A reply' - click_button 'Comment' + click_button 'Add comment now' wait_for_requests expect(page).to have_content('Your comment could not be submitted because discussion to reply to cannot be found') end diff --git a/spec/features/merge_request/user_rebases_merge_request_spec.rb b/spec/features/merge_request/user_rebases_merge_request_spec.rb index 34f009000dc..a3f72a6266b 100644 --- a/spec/features/merge_request/user_rebases_merge_request_spec.rb +++ b/spec/features/merge_request/user_rebases_merge_request_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe "User rebases a merge request", :js do +RSpec.describe "User rebases a merge request", :js do let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:user) { project.owner } diff --git a/spec/features/merge_request/user_reopens_merge_request_spec.rb b/spec/features/merge_request/user_reopens_merge_request_spec.rb index 6dee5770d0c..020929dc416 100644 --- a/spec/features/merge_request/user_reopens_merge_request_spec.rb +++ b/spec/features/merge_request/user_reopens_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User reopens a merge requests', :js do +RSpec.describe 'User reopens a merge requests', :js do let(:project) { create(:project, :public, :repository) } let!(:merge_request) { create(:closed_merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index 41a7456aed5..f96408fb10b 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User resolves conflicts', :js do +RSpec.describe 'Merge request > User resolves conflicts', :js do let(:project) { create(:project, :repository) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 0e30df518d7..aa3840b4376 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User resolves diff notes and threads', :js do +RSpec.describe 'Merge request > User resolves diff notes and threads', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:guest) { create(:user) } @@ -146,17 +146,16 @@ describe 'Merge request > User resolves diff notes and threads', :js do describe 'reply form' do before do click_button 'Toggle thread' - - page.within '.diff-content' do - click_button 'Reply...' - end end it 'allows user to comment' do page.within '.diff-content' do + click_button 'Reply...' + + find(".js-unresolve-checkbox").set false find('.js-note-text').set 'testing' - click_button 'Comment' + click_button 'Add comment now' wait_for_requests end @@ -181,9 +180,11 @@ describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & unresolve thread' do page.within '.diff-content' do + click_button 'Reply...' + find('.js-note-text').set 'testing' - click_button 'Comment & unresolve thread' + click_button 'Add comment now' wait_for_requests end @@ -197,8 +198,6 @@ describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to resolve from reply form without a comment' do page.within '.diff-content' do - click_button 'Reply...' - click_button 'Resolve thread' end @@ -214,7 +213,9 @@ describe 'Merge request > User resolves diff notes and threads', :js do find('.js-note-text').set 'testing' - click_button 'Comment & resolve thread' + find('.js-resolve-checkbox').set(true) + + click_button 'Add comment now' end page.within '.line-resolve-all-container' do @@ -445,7 +446,9 @@ describe 'Merge request > User resolves diff notes and threads', :js do find('.js-note-text').set 'testing' - click_button 'Comment & resolve thread' + find('.js-resolve-checkbox').set(true) + + click_button 'Add comment now' end page.within '.line-resolve-all-container' do @@ -462,7 +465,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do find('.js-note-text').set 'testing' - click_button 'Comment & unresolve thread' + click_button 'Add comment now' end page.within '.line-resolve-all-container' do diff --git a/spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb b/spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb index 9f7c97e510c..f8f3467f6fb 100644 --- a/spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb +++ b/spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User resolves outdated diff discussions', :js do +RSpec.describe 'Merge request > User resolves outdated diff discussions', :js do let(:project) { create(:project, :repository, :public) } let(:merge_request) do diff --git a/spec/features/merge_request/user_resolves_wip_mr_spec.rb b/spec/features/merge_request/user_resolves_wip_mr_spec.rb index 93ef0801791..34a3490a152 100644 --- a/spec/features/merge_request/user_resolves_wip_mr_spec.rb +++ b/spec/features/merge_request/user_resolves_wip_mr_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User resolves Work in Progress', :js do +RSpec.describe 'Merge request > User resolves Work in Progress', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) do diff --git a/spec/features/merge_request/user_reverts_merge_request_spec.rb b/spec/features/merge_request/user_reverts_merge_request_spec.rb index 906ff1d61b2..5e9611de460 100644 --- a/spec/features/merge_request/user_reverts_merge_request_spec.rb +++ b/spec/features/merge_request/user_reverts_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User reverts a merge request', :js do +RSpec.describe 'User reverts a merge request', :js do let(:merge_request) { create(:merge_request, :with_diffs, :simple, source_project: project) } let(:project) { create(:project, :public, :repository) } let(:user) { create(:user) } diff --git a/spec/features/merge_request/user_reviews_image_spec.rb b/spec/features/merge_request/user_reviews_image_spec.rb new file mode 100644 index 00000000000..533f3c9c91a --- /dev/null +++ b/spec/features/merge_request/user_reviews_image_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Merge request > image review', :js do + include MergeRequestDiffHelpers + include RepoHelpers + + let(:user) { project.owner } + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, author: user) } + + before do + sign_in(user) + + allow_any_instance_of(DiffHelper).to receive(:diff_file_blob_raw_url).and_return('/apple-touch-icon.png') + allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.png') + + visit diffs_project_merge_request_path(merge_request.project, merge_request) + + wait_for_requests + end + + it 'leaves review' do + find('.js-add-image-diff-note-button', match: :first).click + + find('.diff-content .note-textarea').native.send_keys('image diff test comment') + + click_button('Start a review') + + wait_for_requests + + page.within(find('.draft-note-component')) do + expect(page).to have_content('image diff test comment') + end + end +end diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb index 48c3ed7178d..d9950f5504b 100644 --- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb +++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User scrolls to note on load', :js do +RSpec.describe 'Merge request > User scrolls to note on load', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, author: user) } diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index 21599164ac3..415e6b29d5a 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees avatars on diff notes', :js do +RSpec.describe 'Merge request > User sees avatars on diff notes', :js do include NoteInteractionHelpers let(:project) { create(:project, :public, :repository) } @@ -42,7 +42,7 @@ describe 'Merge request > User sees avatars on diff notes', :js do page.within('.js-discussion-note-form') do find('.note-textarea').native.send_keys('Test comment') - click_button 'Comment' + click_button 'Add comment now' end expect(page).to have_content('Test comment') @@ -137,7 +137,7 @@ describe 'Merge request > User sees avatars on diff notes', :js do page.within '.js-discussion-note-form' do find('.js-note-text').native.send_keys('Test') - click_button 'Comment' + click_button 'Add comment now' wait_for_requests end @@ -155,7 +155,7 @@ describe 'Merge request > User sees avatars on diff notes', :js do page.within '.js-discussion-note-form' do find('.js-note-text').native.send_keys('Test') - find('.js-comment-button').click + click_button 'Add comment now' wait_for_requests end diff --git a/spec/features/merge_request/user_sees_breadcrumb_links_spec.rb b/spec/features/merge_request/user_sees_breadcrumb_links_spec.rb index 592ad3aae9b..95e435a333e 100644 --- a/spec/features/merge_request/user_sees_breadcrumb_links_spec.rb +++ b/spec/features/merge_request/user_sees_breadcrumb_links_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'New merge request breadcrumb' do +RSpec.describe 'New merge request breadcrumb' do let(:project) { create(:project, :repository) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_sees_check_out_branch_modal_spec.rb b/spec/features/merge_request/user_sees_check_out_branch_modal_spec.rb index f54161fbaec..e47f9ff2660 100644 --- a/spec/features/merge_request/user_sees_check_out_branch_modal_spec.rb +++ b/spec/features/merge_request/user_sees_check_out_branch_modal_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees check out branch modal', :js do +RSpec.describe 'Merge request > User sees check out branch modal', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project) } diff --git a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb index d7675cd06a8..ec2fb856be5 100644 --- a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb +++ b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User cherry-picks', :js do +RSpec.describe 'Merge request > User cherry-picks', :js do let(:group) { create(:group) } let(:project) { create(:project, :repository, namespace: group) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_sees_closing_issues_message_spec.rb b/spec/features/merge_request/user_sees_closing_issues_message_spec.rb index f77ea82649c..baef547a480 100644 --- a/spec/features/merge_request/user_sees_closing_issues_message_spec.rb +++ b/spec/features/merge_request/user_sees_closing_issues_message_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees closing issues message', :js do +RSpec.describe 'Merge request > User sees closing issues message', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:issue_1) { create(:issue, project: project)} diff --git a/spec/features/merge_request/user_sees_deleted_target_branch_spec.rb b/spec/features/merge_request/user_sees_deleted_target_branch_spec.rb index 9ef6847f7f5..7c93952ee99 100644 --- a/spec/features/merge_request/user_sees_deleted_target_branch_spec.rb +++ b/spec/features/merge_request/user_sees_deleted_target_branch_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees deleted target branch', :js do +RSpec.describe 'Merge request > User sees deleted target branch', :js do let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb index 9670bd798bf..1e547d504ef 100644 --- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees deployment widget', :js do +RSpec.describe 'Merge request > User sees deployment widget', :js do describe 'when merge request has associated environments' do let(:user) { create(:user) } let(:project) { create(:project, :repository) } diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index 868451883d8..2229b242d5b 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees diff', :js do +RSpec.describe 'Merge request > User sees diff', :js do include ProjectForksHelper include RepoHelpers diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index b4afd8c6332..ca8c4f84677 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees threads', :js do +RSpec.describe 'Merge request > User sees threads', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project) } diff --git a/spec/features/merge_request/user_sees_empty_state_spec.rb b/spec/features/merge_request/user_sees_empty_state_spec.rb index 88eba976d62..ac07b31731d 100644 --- a/spec/features/merge_request/user_sees_empty_state_spec.rb +++ b/spec/features/merge_request/user_sees_empty_state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees empty state' do +RSpec.describe 'Merge request > User sees empty state' do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb b/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb index 4cc129e5d5f..cae04dd1693 100644 --- a/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees merge button depending on unresolved threads', :js do +RSpec.describe 'Merge request > User sees merge button depending on unresolved threads', :js do let(:project) { create(:project, :repository) } let(:user) { project.creator } let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) } diff --git a/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb b/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb index 5b14450a289..e2aa10d80dd 100644 --- a/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb +++ b/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees pipelines triggered by merge request', :js do +RSpec.describe 'Merge request > User sees pipelines triggered by merge request', :js do include ProjectForksHelper include TestReportsHelper diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index eca011bc786..bd140a0643d 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees merge widget', :js do +RSpec.describe 'Merge request > User sees merge widget', :js do include ProjectForksHelper include TestReportsHelper include ReactiveCachingHelpers diff --git a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb index 29b8dc19860..04d8c52df61 100644 --- a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb +++ b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request < User sees mini pipeline graph', :js do +RSpec.describe 'Merge request < User sees mini pipeline graph', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, head_pipeline: pipeline) } diff --git a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb index b4fb844b943..cbd68025b50 100644 --- a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb +++ b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees MR from deleted forked project', :js do +RSpec.describe 'Merge request > User sees MR from deleted forked project', :js do include ProjectForksHelper let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb index 59e5f5c847d..e997fb3e853 100644 --- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' # This test serves as a regression test for a bug that caused an error # message to be shown by JavaScript when the source branch was deleted. # Please do not remove ":js". -describe 'Merge request > User sees MR with deleted source branch', :js do +RSpec.describe 'Merge request > User sees MR with deleted source branch', :js do let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb index 029f55c2cd6..20c45a1d652 100644 --- a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb +++ b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees notes from forked project', :js do +RSpec.describe 'Merge request > User sees notes from forked project', :js do include ProjectForksHelper let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb b/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb index d258b98f4a9..56092da5136 100644 --- a/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb +++ b/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees pipelines from forked project', :js do +RSpec.describe 'Merge request > User sees pipelines from forked project', :js do include ProjectForksHelper let(:target_project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_sees_pipelines_spec.rb b/spec/features/merge_request/user_sees_pipelines_spec.rb index f3d8f2b42f8..2d125087cb6 100644 --- a/spec/features/merge_request/user_sees_pipelines_spec.rb +++ b/spec/features/merge_request/user_sees_pipelines_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees pipelines', :js do +RSpec.describe 'Merge request > User sees pipelines', :js do describe 'pipeline tab' do let(:merge_request) { create(:merge_request) } let(:project) { merge_request.target_project } diff --git a/spec/features/merge_request/user_sees_system_notes_spec.rb b/spec/features/merge_request/user_sees_system_notes_spec.rb index 0482458d5ac..9f8d4c6d63f 100644 --- a/spec/features/merge_request/user_sees_system_notes_spec.rb +++ b/spec/features/merge_request/user_sees_system_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees system notes', :js do +RSpec.describe 'Merge request > User sees system notes', :js do let(:public_project) { create(:project, :public, :repository) } let(:private_project) { create(:project, :private, :repository) } let(:user) { private_project.creator } diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index 5b43fe407eb..75319c8a22d 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees versions', :js do +RSpec.describe 'Merge request > User sees versions', :js do let(:merge_request) do create(:merge_request).tap do |mr| mr.merge_request_diff.destroy @@ -34,7 +34,7 @@ describe 'Merge request > User sees versions', :js do page.within("form[data-line-code='#{line_code}']") do fill_in "note[note]", with: comment - find(".js-comment-button").click + click_button('Add comment now') end wait_for_requests diff --git a/spec/features/merge_request/user_sees_wip_help_message_spec.rb b/spec/features/merge_request/user_sees_wip_help_message_spec.rb index 1179303171c..42fe18cfc93 100644 --- a/spec/features/merge_request/user_sees_wip_help_message_spec.rb +++ b/spec/features/merge_request/user_sees_wip_help_message_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User sees WIP help message' do +RSpec.describe 'Merge request > User sees WIP help message' do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb index 22b2ea81b32..bf445de44ba 100644 --- a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb +++ b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User selects branches for new MR', :js do +RSpec.describe 'Merge request > User selects branches for new MR', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index 62e0e4d76ed..b81c0e49538 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User comments on a diff', :js do +RSpec.describe 'User comments on a diff', :js do include MergeRequestDiffHelpers include RepoHelpers @@ -49,7 +49,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion\n# change to a comment\n```") - click_button('Comment') + click_button('Add comment now') end wait_for_requests @@ -77,7 +77,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion\n# change to a comment\n```") - click_button('Comment') + click_button('Add comment now') end wait_for_requests @@ -93,6 +93,100 @@ describe 'User comments on a diff', :js do end end + context 'applying suggestions in batches' do + def hash(path) + diff_file = merge_request.diffs(paths: [path]).diff_files.first + Digest::SHA1.hexdigest(diff_file.file_path) + end + + file1 = 'files/ruby/popen.rb' + file2 = 'files/ruby/regex.rb' + + let(:files) do + [ + { + hash: hash(file1), + line_code: "#{hash(file1)}_12_12" + }, + { + hash: hash(file2), + line_code: "#{hash(file2)}_21_21" + } + ] + end + + it 'can add and remove suggestions from a batch' do + files.each_with_index do |file, index| + page.within("[id='#{file[:hash]}']") do + find("button[title='Show full file']").click + wait_for_requests + + click_diff_line(find("[id='#{file[:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: "```suggestion\n# change to a comment\n```") + click_button('Add comment now') + wait_for_requests + end + end + + page.within("[id='#{file[:hash]}']") do + expect(page).not_to have_content('Applied') + + click_button('Add suggestion to batch') + wait_for_requests + + expect(page).to have_content('Remove from batch') + expect(page).to have_content("Apply suggestions #{index + 1}") + end + end + + page.within("[id='#{files[0][:hash]}']") do + click_button('Remove from batch') + wait_for_requests + + expect(page).to have_content('Apply suggestion') + expect(page).to have_content('Add suggestion to batch') + end + + page.within("[id='#{files[1][:hash]}']") do + expect(page).to have_content('Remove from batch') + expect(page).to have_content('Apply suggestions 1') + end + end + + it 'can apply multiple suggestions as a batch' do + files.each_with_index do |file, index| + page.within("[id='#{file[:hash]}']") do + find("button[title='Show full file']").click + wait_for_requests + + click_diff_line(find("[id='#{file[:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: "```suggestion\n# change to a comment\n```") + click_button('Add comment now') + wait_for_requests + end + end + + page.within("[id='#{file[:hash]}']") do + click_button('Add suggestion to batch') + wait_for_requests + end + end + + expect(page).not_to have_content('Applied') + + page.within("[id='#{files[0][:hash]}']") do + click_button('Apply suggestions 2') + wait_for_requests + end + + expect(page).to have_content('Applied').twice + end + end + context 'multiple suggestions in expanded lines' do # https://gitlab.com/gitlab-org/gitlab/issues/38277 it 'suggestions are appliable', :quarantine do @@ -119,7 +213,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion\n# change to a comment\n```") - click_button('Comment') + click_button('Add comment now') wait_for_requests end @@ -127,7 +221,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion\n# 2nd change to a comment\n```") - click_button('Comment') + click_button('Add comment now') wait_for_requests end @@ -158,7 +252,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion:-2\n# or that\n# heh\n```") - click_button('Comment') + click_button('Add comment now') end wait_for_requests @@ -201,7 +295,7 @@ describe 'User comments on a diff', :js do page.within('.js-discussion-note-form') do fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```") - click_button('Comment') + click_button('Add comment now') end wait_for_requests diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb index 4db067a4e41..fab500f47bf 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge request > User toggles whitespace changes', :js do +RSpec.describe 'Merge request > User toggles whitespace changes', :js do let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } let(:user) { project.creator } diff --git a/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb index 1ebe9e2e409..b864cb55785 100644 --- a/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb +++ b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Merge Request > User tries to access private project information through the new mr page' do +RSpec.describe 'Merge Request > User tries to access private project information through the new mr page' do let(:current_user) { create(:user) } let(:private_project) do create(:project, :public, :repository, diff --git a/spec/features/merge_request/user_uses_quick_actions_spec.rb b/spec/features/merge_request/user_uses_quick_actions_spec.rb index 318f8812263..04a2e046f42 100644 --- a/spec/features/merge_request/user_uses_quick_actions_spec.rb +++ b/spec/features/merge_request/user_uses_quick_actions_spec.rb @@ -7,7 +7,7 @@ require 'spec_helper' # for example, adding quick actions when creating the issue and checking DateTime formats on UI. # Because this kind of spec takes more time to run there is no need to add new ones # for each existing quick action unless they test something not tested by existing tests. -describe 'Merge request > User uses quick actions', :js do +RSpec.describe 'Merge request > User uses quick actions', :js do include Spec::Support::Helpers::Features::NotesHelpers let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index cd0cf1cc78a..14d10fc1c9f 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User views diffs', :js do +RSpec.describe 'User views diffs', :js do let(:merge_request) do create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') end diff --git a/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb b/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb index a38bc4f702b..370341a43f9 100644 --- a/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb +++ b/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb @@ -6,7 +6,7 @@ require 'spec_helper' # updated. # This can occur when the fork a merge request is created from is in the process # of being destroyed. -describe 'User views merged merge request from deleted fork' do +RSpec.describe 'User views merged merge request from deleted fork' do include ProjectForksHelper let(:project) { create(:project, :repository) } diff --git a/spec/features/merge_request/user_views_open_merge_request_spec.rb b/spec/features/merge_request/user_views_open_merge_request_spec.rb index a788fc71286..448844ae57d 100644 --- a/spec/features/merge_request/user_views_open_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_open_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'User views an open merge request' do +RSpec.describe 'User views an open merge request' do let(:merge_request) do create(:merge_request, source_project: project, target_project: project, description: '# Description header') end diff --git a/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb b/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb index 78d9c6c6db1..a6de443e96f 100644 --- a/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'Project > Merge request > View user status' do +RSpec.describe 'Project > Merge request > View user status' do let(:project) { create(:project, :public, :repository) } let(:merge_request) do create(:merge_request, source_project: project, target_project: project, author: create(:user)) |