diff options
author | Rémy Coutable <remy@rymai.me> | 2017-08-01 21:03:07 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-01-18 17:07:57 +0300 |
commit | 799cd471099e032a60fbdb1ba6841e617f6c122f (patch) | |
tree | e18e25e7ec82f9fb35bad04890e57c21a69097af /spec/features/merge_request | |
parent | 50a649530a53a04d2044f9e4c5502586a7ed0c59 (diff) |
Improve MR feature specs and reduce duplication
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec/features/merge_request')
38 files changed, 4133 insertions, 0 deletions
diff --git a/spec/features/merge_request/user_assigns_themselves_spec.rb b/spec/features/merge_request/user_assigns_themselves_spec.rb new file mode 100644 index 00000000000..b6b38186a22 --- /dev/null +++ b/spec/features/merge_request/user_assigns_themselves_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +describe 'Merge request > User assigns themselves' do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:issue1) { create(:issue, project: project) } + let(:issue2) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue1.to_reference} and #{issue2.to_reference}") } + + context 'logged in as a member of the project' do + before do + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'updates related issues', :js do + click_link 'Assign yourself to these issues' + + expect(page).to have_content '2 issues have been assigned to you' + end + + it 'returns user to the merge request', :js do + click_link 'Assign yourself to these issues' + + expect(page).to have_content merge_request.description + end + + context 'when related issues are already assigned' do + before do + [issue1, issue2].each { |issue| issue.update!(assignees: [user]) } + end + + it 'does not display if related issues are already assigned' do + expect(page).not_to have_content 'Assign yourself' + end + end + end + + context 'logged in as a non-member of the project' do + before do + sign_in(create(:user)) + visit project_merge_request_path(project, merge_request) + end + + it 'does not not show assignment link' do + expect(page).not_to have_content 'Assign yourself' + end + end +end diff --git a/spec/features/merge_request/user_awards_emoji_spec.rb b/spec/features/merge_request/user_awards_emoji_spec.rb new file mode 100644 index 00000000000..15a0878fb16 --- /dev/null +++ b/spec/features/merge_request/user_awards_emoji_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +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) } + + describe 'logged in' do + before do + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'adds award to merge request' do + first('.js-emoji-btn').click + expect(page).to have_selector('.js-emoji-btn.active') + expect(first('.js-emoji-btn')).to have_content '1' + + visit project_merge_request_path(project, merge_request) + expect(first('.js-emoji-btn')).to have_content '1' + end + + it 'removes award from merge request' do + first('.js-emoji-btn').click + find('.js-emoji-btn.active').click + expect(first('.js-emoji-btn')).to have_content '0' + + visit project_merge_request_path(project, merge_request) + expect(first('.js-emoji-btn')).to have_content '0' + end + + it 'has only one menu on the page' do + first('.js-add-award').click + expect(page).to have_selector('.emoji-menu') + + expect(page).to have_selector('.emoji-menu', count: 1) + end + end + + describe 'logged out' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'does not see award menu button' do + expect(page).not_to have_selector('.js-award-holder') + end + end +end diff --git a/spec/features/merge_request/user_cherry_picks_spec.rb b/spec/features/merge_request/user_cherry_picks_spec.rb new file mode 100644 index 00000000000..494096b21c0 --- /dev/null +++ b/spec/features/merge_request/user_cherry_picks_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +describe 'Merge request > User cherry-picks', :js do + let(:group) { create(:group) } + let(:project) { create(:project, :repository, namespace: group) } + let(:user) { project.creator } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'Viewing a merged merge request' do + before do + service = MergeRequests::MergeService.new(project, user) + + perform_enqueued_jobs do + service.execute(merge_request) + end + end + + # Fast-forward merge, or merged before GitLab 8.5. + context 'Without a merge commit' do + before do + merge_request.merge_commit_sha = nil + merge_request.save + end + + it 'does not show a Cherry-pick button' do + visit project_merge_request_path(project, merge_request) + + expect(page).not_to have_link 'Cherry-pick' + end + end + + context 'With a merge commit' do + it 'shows a Cherry-pick button' do + visit project_merge_request_path(project, merge_request) + + expect(page).to have_link 'Cherry-pick' + end + end + end +end 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 new file mode 100644 index 00000000000..7c4fd25bb39 --- /dev/null +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -0,0 +1,206 @@ +require 'spec_helper' + +feature 'Merge request > User creates image diff notes', :js do + include NoteInteractionHelpers + + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + + before do + sign_in(user) + + # Stub helper to return any blob file as image from public app folder. + # This is necessary to run this specs since we don't display repo images in capybara. + 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.ico') + end + + context 'create commit diff notes' do + commit_id = '2f63565e7aac07bcdadb654e253078b727143ec4' + + describe 'create a new diff note' do + before do + visit project_commit_path(project, commit_id) + create_image_diff_note + end + + it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do + indicator = find('.js-image-badge') + badge = find('.image-diff-avatar-link .badge') + + expect(indicator).to have_content('1') + expect(badge).to have_content('1') + + find('.js-diff-notes-toggle').click + + expect(page).not_to have_content('image diff test comment') + + find('.js-diff-notes-toggle').click + + expect(page).to have_content('image diff test comment') + end + end + + describe 'render commit diff notes' do + let(:path) { "files/images/6049019_460s.jpg" } + let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } + + let(:note1_position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + width: 100, + height: 100, + x: 10, + y: 10, + position_type: "image", + diff_refs: commit.diff_refs + ) + end + + let(:note2_position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + width: 100, + height: 100, + x: 20, + y: 20, + position_type: "image", + diff_refs: commit.diff_refs + ) + end + + let!(:note1) { create(:diff_note_on_commit, commit_id: commit.id, project: project, position: note1_position, note: 'my note 1') } + let!(:note2) { create(:diff_note_on_commit, commit_id: commit.id, project: project, position: note2_position, note: 'my note 2') } + + before do + visit project_commit_path(project, commit.id) + wait_for_requests + end + + it 'render diff indicators within the image diff frame, diff notes, and avatar badge numbers' do + expect(page).to have_css('.js-image-badge', count: 2) + expect(page).to have_css('.diff-content .note', count: 2) + expect(page).to have_css('.image-diff-avatar-link', text: 1) + expect(page).to have_css('.image-diff-avatar-link', text: 2) + end + end + end + + %w(inline parallel).each do |view| + context "#{view} view" do + let(:merge_request) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, author: user) } + let(:path) { "files/images/ee_repo_logo.png" } + + let(:position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + width: 100, + height: 100, + x: 1, + y: 1, + position_type: "image", + diff_refs: merge_request.diff_refs + ) + end + + let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } + + describe 'creating a new diff note' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: view) + create_image_diff_note + end + + it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do + indicator = find('.js-image-badge', match: :first) + badge = find('.image-diff-avatar-link .badge', match: :first) + + expect(indicator).to have_content('1') + expect(badge).to have_content('1') + + page.all('.js-diff-notes-toggle')[0].click + page.all('.js-diff-notes-toggle')[1].click + + expect(page).not_to have_content('image diff test comment') + + page.all('.js-diff-notes-toggle')[0].click + page.all('.js-diff-notes-toggle')[1].click + + expect(page).to have_content('image diff test comment') + end + end + end + end + + describe 'discussion tab polling' do + let(:merge_request) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, author: user) } + let(:path) { "files/images/ee_repo_logo.png" } + + let(:position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + width: 100, + height: 100, + x: 50, + y: 50, + position_type: "image", + diff_refs: merge_request.diff_refs + ) + end + + before do + visit project_merge_request_path(project, merge_request) + end + + it 'render diff indicators within the image frame' do + diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + + wait_for_requests + + expect(page).to have_selector('.image-comment-badge') + expect(page).to have_content(diff_note.note) + end + end + + describe 'image view modes' do + before do + visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4') + end + + it 'resizes image in onion skin view mode' do + find('.view-modes-menu .onion-skin').click + + expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;') + end + + it 'resets onion skin view mode opacity when toggling between view modes' do + find('.view-modes-menu .onion-skin').click + + # Simulate dragging onion-skin slider + drag_and_drop_by(find('.dragger'), -30, 0) + + expect(find('.onion-skin-frame .frame.added', visible: false)['style']).not_to match('opacity: 1;') + + find('.view-modes-menu .swipe').click + find('.view-modes-menu .onion-skin').click + + expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;') + end + end + + def drag_and_drop_by(element, right_by, down_by) + page.driver.browser.action.drag_and_drop_by(element.native, right_by, down_by).perform + end + + def create_image_diff_note + find('.js-add-image-diff-note-button', match: :first).click + page.all('.js-add-image-diff-note-button')[0].click + find('.diff-content .note-textarea').native.send_keys('image diff test comment') + click_button 'Comment' + wait_for_requests + end +end diff --git a/spec/features/merge_request/user_creates_mr_spec.rb b/spec/features/merge_request/user_creates_mr_spec.rb new file mode 100644 index 00000000000..1ac31de62cb --- /dev/null +++ b/spec/features/merge_request/user_creates_mr_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe 'Merge request > User creates MR' do + it_behaves_like 'a creatable merge request' + + context 'from a forked project' do + include ProjectForksHelper + + let(:canonical_project) { create(:project, :public, :repository) } + + let(:source_project) do + fork_project(canonical_project, user, + repository: true, + namespace: user.namespace) + end + + context 'to canonical project' do + it_behaves_like 'a creatable merge request' + end + + context 'to another forked project' do + let(:target_project) do + fork_project(canonical_project, user, + repository: true, + namespace: user.namespace) + end + + it_behaves_like 'a creatable merge request' + end + end +end 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 new file mode 100644 index 00000000000..e1e70b6d260 --- /dev/null +++ b/spec/features/merge_request/user_customizes_merge_commit_message_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +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)} + let(:issue_2) { create(:issue, project: project)} + let(:merge_request) do + create( + :merge_request, + :simple, + source_project: project, + description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" + ) + end + let(:textbox) { page.find(:css, '.js-commit-message', visible: false) } + let(:default_message) do + [ + "Merge branch 'feature' into 'master'", + merge_request.title, + "Closes #{issue_1.to_reference} and #{issue_2.to_reference}", + "See merge request #{merge_request.to_reference(full: true)}" + ].join("\n\n") + end + let(:message_with_description) do + [ + "Merge branch 'feature' into 'master'", + merge_request.title, + merge_request.description, + "See merge request #{merge_request.to_reference(full: true)}" + ].join("\n\n") + end + + before do + project.add_master(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'toggles commit message between message with description and without description' do + expect(page).not_to have_selector('.js-commit-message') + click_button "Modify commit message" + expect(textbox).to be_visible + expect(textbox.value).to eq(default_message) + + click_link "Include description in commit message" + + expect(textbox.value).to eq(message_with_description) + + click_link "Don't include description in commit message" + + expect(textbox.value).to eq(default_message) + end +end diff --git a/spec/features/merge_request/user_edits_mr_spec.rb b/spec/features/merge_request/user_edits_mr_spec.rb new file mode 100644 index 00000000000..8c9e782aa76 --- /dev/null +++ b/spec/features/merge_request/user_edits_mr_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +describe 'Merge request > User edits MR' do + it_behaves_like 'an editable merge request' + + context 'for a forked project' do + it_behaves_like 'an editable merge request' do + let(:source_project) { create(:project, :repository, forked_from_project: target_project) } + end + end +end diff --git a/spec/features/merge_request/user_locks_discussion_spec.rb b/spec/features/merge_request/user_locks_discussion_spec.rb new file mode 100644 index 00000000000..a68df872334 --- /dev/null +++ b/spec/features/merge_request/user_locks_discussion_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +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) } + + before do + sign_in(user) + end + + context 'when the discussion is locked' do + before do + merge_request.update_attribute(:discussion_locked, true) + end + + context 'when a user is a team member' do + before do + project.add_developer(user) + visit project_merge_request_path(project, merge_request) + end + + it 'the user can create a comment' do + page.within('.issuable-discussion #notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('.issuable-discussion #notes')).to have_content('Some new comment') + end + end + + context 'when a user is not a team member' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'the user can not create a comment' do + page.within('.issuable-discussion #notes') do + expect(page).not_to have_selector('js-main-target-form') + expect(page.find('.disabled-comment')) + .to have_content('This merge request is locked. Only project members can comment.') + end + end + end + end +end diff --git a/spec/features/merge_request/user_merges_immediately_spec.rb b/spec/features/merge_request/user_merges_immediately_spec.rb new file mode 100644 index 00000000000..b16fc9bfc89 --- /dev/null +++ b/spec/features/merge_request/user_merges_immediately_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +describe 'Merge requests > User merges immediately', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let!(:merge_request) do + create(:merge_request_with_diffs, source_project: project, + author: user, + title: 'Bug NS-04', + head_pipeline: pipeline, + source_branch: pipeline.ref) + end + let(:pipeline) do + create(:ci_pipeline, project: project, + ref: 'master', + sha: project.repository.commit('master').id) + end + + context 'when there is active pipeline for merge request' do + before do + create(:ci_build, pipeline: pipeline) + project.add_master(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'enables merge immediately' do + page.within '.mr-widget-body' do + find('.dropdown-toggle').click + + Sidekiq::Testing.fake! do + click_link 'Merge immediately' + + expect(find('.accept-merge-request.btn-info')).to have_content('Merge in progress') + + wait_for_requests + end + end + end + end +end 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 new file mode 100644 index 00000000000..a045791f6b4 --- /dev/null +++ b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb @@ -0,0 +1,145 @@ +require 'rails_helper' + +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 } + + before do + project.add_master(merge_request.author) + sign_in(merge_request.author) + end + + context 'project does not have CI enabled' do + it 'allows MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge' + end + end + + context 'when project has CI enabled' do + let!(:pipeline) do + create(:ci_empty_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: status, head_pipeline_of: merge_request) + end + + context 'when merge requests can only be merged if the pipeline succeeds' do + before do + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) + end + + context 'when CI is running' do + let(:status) { :running } + + it 'does not allow to merge immediately' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge when pipeline succeeds' + expect(page).not_to have_button '.js-merge-moment' + end + end + + context 'when CI failed' do + let(:status) { :failed } + + it 'does not allow MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_css('button[disabled="disabled"]', text: 'Merge') + expect(page).to have_content('Please retry the job or push a new commit to fix the failure') + end + end + + context 'when CI canceled' do + let(:status) { :canceled } + + it 'does not allow MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).not_to have_button 'Merge' + expect(page).to have_content('Please retry the job or push a new commit to fix the failure') + end + end + + context 'when CI succeeded' do + let(:status) { :success } + + it 'allows MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge' + end + end + + context 'when CI skipped' do + let(:status) { :skipped } + + it 'allows MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge' + end + end + end + + context 'when merge requests can be merged when the build failed' do + before do + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, false) + end + + context 'when CI is running' do + let(:status) { :running } + + it 'allows MR to be merged immediately' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge when pipeline succeeds' + + page.find('.js-merge-moment').click + expect(page).to have_content 'Merge immediately' + end + end + + context 'when CI failed' do + let(:status) { :failed } + + it 'allows MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge' + end + end + + context 'when CI succeeded' do + let(:status) { :success } + + it 'allows MR to be merged' do + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).to have_button 'Merge' + end + end + end + end +end 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 new file mode 100644 index 00000000000..890774922aa --- /dev/null +++ b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb @@ -0,0 +1,186 @@ +require 'rails_helper' + +describe 'Merge request > User merges when pipeline succeeds', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, + author: user, + title: 'Bug NS-04', + merge_params: { force_remove_source_branch: '1' }) + end + let(:pipeline) do + create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + head_pipeline_of: merge_request) + end + + before do + project.add_master(user) + end + + context 'when there is active pipeline for merge request' do + before do + create(:ci_build, pipeline: pipeline) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + describe 'enabling Merge when pipeline succeeds' do + shared_examples 'Merge when pipeline succeeds activator' do + it 'activates the Merge when pipeline succeeds feature' do + click_button "Merge when pipeline succeeds" + + expect(page).to have_content "Set by #{user.name} to be merged automatically when the pipeline succeeds" + expect(page).to have_content "The source branch will not be removed" + expect(page).to have_selector ".js-cancel-auto-merge" + visit project_merge_request_path(project, merge_request) # Needed to refresh the page + expect(page).to have_content /enabled an automatic merge when the pipeline for \h{8} succeeds/i + end + end + + context "when enabled immediately" do + it_behaves_like 'Merge when pipeline succeeds activator' + end + + context 'when enabled after pipeline status changed' do + before do + pipeline.run! + + # We depend on merge request widget being reloaded + # so we have to wait for asynchronous call to reload it + # and have_content expectation handles that. + # + expect(page).to have_content "Pipeline ##{pipeline.id} running" + end + + it_behaves_like 'Merge when pipeline succeeds activator' + end + + context 'when enabled after it was previously canceled' do + before do + click_button "Merge when pipeline succeeds" + click_link "Cancel automatic merge" + end + + it_behaves_like 'Merge when pipeline succeeds activator' + end + + context 'when it was enabled and then canceled' do + let(:merge_request) do + create(:merge_request_with_diffs, + :merge_when_pipeline_succeeds, + source_project: project, + title: 'Bug NS-04', + author: user, + merge_user: user, + merge_params: { force_remove_source_branch: '1' }) + end + + before do + click_link "Cancel automatic merge" + end + + it_behaves_like 'Merge when pipeline succeeds activator' + end + end + + describe 'enabling Merge when pipeline succeeds via dropdown' do + it 'activates the Merge when pipeline succeeds feature' do + find('.js-merge-moment').click + click_link 'Merge when pipeline succeeds' + + expect(page).to have_content "Set by #{user.name} to be merged automatically when the pipeline succeeds" + expect(page).to have_content "The source branch will not be removed" + expect(page).to have_link "Cancel automatic merge" + end + end + end + + context 'when merge when pipeline succeeds is enabled' do + let(:merge_request) do + create(:merge_request_with_diffs, :simple, source_project: project, + author: user, + merge_user: user, + title: 'MepMep', + merge_when_pipeline_succeeds: true) + end + let!(:build) do + create(:ci_build, pipeline: pipeline) + end + + before do + sign_in user + visit project_merge_request_path(project, merge_request) + end + + it 'allows to cancel the automatic merge' do + click_link "Cancel automatic merge" + + expect(page).to have_button "Merge when pipeline succeeds" + + refresh + + expect(page).to have_content "canceled the automatic merge" + end + + context 'when pipeline succeeds' do + before do + build.success + refresh + end + + it 'merges merge request' do + expect(page).to have_content 'The changes were merged' + expect(merge_request.reload).to be_merged + end + end + + context 'view merge request with MWPS enabled but automatically merge fails' do + before do + merge_request.update( + merge_user: merge_request.author, + merge_error: 'Something went wrong' + ) + refresh + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Something went wrong') + end + end + end + + context 'view merge request with MWPS enabled but automatically merge fails' do + before do + merge_request.update( + merge_user: merge_request.author, + merge_error: 'Something went wrong' + ) + refresh + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Something went wrong') + end + end + end + end + + context 'when pipeline is not active' do + it 'does not allow to enable merge when pipeline succeeds' do + visit project_merge_request_path(project, merge_request) + + expect(page).not_to have_link 'Merge when pipeline succeeds' + end + end +end diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb new file mode 100644 index 00000000000..2b4623d6dc9 --- /dev/null +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -0,0 +1,281 @@ +require 'rails_helper' + +describe 'Merge request > User posts diff notes', :js do + include MergeRequestDiffHelpers + + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.source_project } + let(:user) { project.creator } + let(:comment_button_class) { '.add-diff-note' } + let(:notes_holder_input_class) { 'js-temp-notes-holder' } + let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' } + let(:test_note_comment) { 'this is a test note!' } + + before do + set_cookie('sidebar_collapsed', 'true') + + project.add_developer(user) + sign_in(user) + end + + context 'when hovering over a parallel view diff file' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: 'parallel') + end + + context 'with an old line on the left and no line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right') + end + end + + context 'with no line on the left and a new line on the right' do + it 'does not allow commenting on the left side' do + should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right') + end + end + + context 'with an old line on the left and a new line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right') + end + end + + context 'with an unchanged line on the left and an unchanged line on the right' do + it 'allows commenting on the left side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left') + end + + it 'allows commenting on the right side' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'right') + end + end + + context 'with a match line' do + it 'does not allow commenting on the left side' do + should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(find('.match', match: :first).find(:xpath, '..'), 'right') + end + end + + context 'with an unfolded line' do + before do + find('.js-unfold', match: :first).click + wait_for_requests + end + + # The first `.js-unfold` unfolds upwards, therefore the first + # `.line_holder` will be an unfolded line. + let(:line_holder) { first('.line_holder[id="1"]') } + + it 'does not allow commenting on the left side' do + should_not_allow_commenting(line_holder, 'left') + end + + it 'does not allow commenting on the right side' do + should_not_allow_commenting(line_holder, 'right') + end + end + end + + context 'when hovering over an inline view diff file' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: 'inline') + end + + context 'after deleteing a note' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + + accept_confirm do + first('button.more-actions-toggle').click + first('.js-note-delete').click + end + + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + + context 'with a new line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + + context 'with an old line' do + it 'allows commenting' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + end + + context 'with an unchanged line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + end + end + + context 'with a match line' do + it 'does not allow commenting' do + should_not_allow_commenting(find('.match', match: :first)) + end + end + + context 'with an unfolded line' do + before do + find('.js-unfold', match: :first).click + wait_for_requests + end + + # The first `.js-unfold` unfolds upwards, therefore the first + # `.line_holder` will be an unfolded line. + let(:line_holder) { first('.line_holder[id="1"]') } + + it 'does not allow commenting' do + should_not_allow_commenting line_holder + end + end + + context 'when hovering over a diff discussion' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: 'inline') + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + visit project_merge_request_path(project, merge_request) + end + + it 'does not allow commenting' do + should_not_allow_commenting(find('.line_holder', match: :first)) + end + end + end + + context 'when cancelling the comment addition' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: 'inline') + end + + context 'with a new line' do + it 'allows dismissing a comment' do + should_allow_dismissing_a_comment(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + end + + describe 'with muliple note forms' do + before do + visit diffs_project_merge_request_path(project, merge_request, view: 'inline') + click_diff_line(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + click_diff_line(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + + describe 'posting a note' do + it 'adds as discussion' do + expect(page).to have_css('.js-temp-notes-holder', count: 2) + + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false) + expect(page).to have_css('.notes_holder .note', count: 1) + expect(page).to have_css('.js-temp-notes-holder', count: 1) + expect(page).to have_button('Reply...') + end + end + end + + context 'when the MR only supports legacy diff notes' do + before do + merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) + visit diffs_project_merge_request_path(project, merge_request, view: 'inline') + end + + context 'with a new line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) + end + end + + context 'with an old line' do + it 'allows commenting' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) + end + end + + context 'with an unchanged line' do + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) + end + end + + context 'with a match line' do + it 'does not allow commenting' do + should_not_allow_commenting(find('.match', match: :first)) + end + end + end + + def should_allow_commenting(line_holder, diff_side = nil, asset_form_reset: true) + write_comment_on_line(line_holder, diff_side) + + click_button 'Comment' + + wait_for_requests + + assert_comment_persistence(line_holder, asset_form_reset: asset_form_reset) + end + + def should_allow_dismissing_a_comment(line_holder, diff_side = nil) + write_comment_on_line(line_holder, diff_side) + + find('.js-close-discussion-note-form').click + + assert_comment_dismissal(line_holder) + end + + def should_not_allow_commenting(line_holder, diff_side = nil) + line = get_line_components(line_holder, diff_side) + line[:content].hover + expect(line[:num]).not_to have_css comment_button_class + end + + def write_comment_on_line(line_holder, diff_side) + click_diff_line(line_holder, diff_side) + + notes_holder_input = line_holder.find(:xpath, notes_holder_input_xpath) + + expect(notes_holder_input[:class]).to include(notes_holder_input_class) + + notes_holder_input.fill_in 'note[note]', with: test_note_comment + end + + def assert_comment_persistence(line_holder, asset_form_reset:) + notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath) + + expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class) + expect(notes_holder_saved).to have_content test_note_comment + + assert_form_is_reset if asset_form_reset + end + + def assert_comment_dismissal(line_holder) + expect(line_holder).not_to have_xpath notes_holder_input_xpath + expect(page).not_to have_content test_note_comment + + assert_form_is_reset + end + + def assert_form_is_reset + expect(page).to have_no_css('.js-temp-notes-holder') + end +end diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb new file mode 100644 index 00000000000..50d06565fc0 --- /dev/null +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -0,0 +1,168 @@ +require 'rails_helper' + +describe 'Merge request > User posts notes', :js do + include NoteInteractionHelpers + + let(:project) { create(:project, :repository) } + let(:user) { project.creator } + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + let!(:note) do + create(:note_on_merge_request, :with_attachment, noteable: merge_request, + project: project) + end + + before do + project.add_master(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + subject { page } + + describe 'the note form' do + it 'is valid' do + is_expected.to have_css('.js-main-target-form', visible: true, count: 1) + expect(find('.js-main-target-form .js-comment-button').value) + .to eq('Comment') + page.within('.js-main-target-form') do + expect(page).not_to have_link('Cancel') + end + end + + describe 'with text' do + before do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: 'This is awesome' + end + end + + it 'has enable submit button and preview button' do + page.within('.js-main-target-form') do + expect(page).not_to have_css('.js-comment-button[disabled]') + expect(page).to have_css('.js-md-preview-button', visible: true) + end + end + end + end + + describe 'when posting a note' do + before do + page.within('.js-main-target-form') do + fill_in 'note[note]', with: 'This is awesome!' + find('.js-md-preview-button').click + click_button 'Comment' + end + end + + it 'is added and form reset' do + is_expected.to have_content('This is awesome!') + page.within('.js-main-target-form') do + expect(page).to have_no_field('note[note]', with: 'This is awesome!') + expect(page).to have_css('.js-md-preview', visible: :hidden) + end + page.within('.js-main-target-form') do + is_expected.to have_css('.js-note-text', visible: true) + end + end + end + + describe 'when previewing a note' do + it 'shows the toolbar buttons when editing a note' do + page.within('.js-main-target-form') do + expect(page).to have_css('.md-header-toolbar.active') + end + end + + it 'hides the toolbar buttons when previewing a note' do + find('.js-md-preview-button').click + page.within('.js-main-target-form') do + expect(page).not_to have_css('.md-header-toolbar.active') + end + end + end + + describe 'when editing a note' do + it 'there should be a hidden edit form' do + is_expected.to have_css('.note-edit-form:not(.mr-note-edit-form)', visible: false, count: 1) + is_expected.to have_css('.note-edit-form.mr-note-edit-form', visible: false, count: 1) + end + + describe 'editing the note' do + before do + find('.note').hover + + find('.js-note-edit').click + end + + it 'shows the note edit form and hide the note body' do + page.within("#note_#{note.id}") do + expect(find('.current-note-edit-form', visible: true)).to be_visible + expect(find('.note-edit-form', visible: true)).to be_visible + expect(find(:css, '.note-body > .note-text', visible: false)).not_to be_visible + end + end + + it 'resets the edit note form textarea with the original content of the note if cancelled' do + within('.current-note-edit-form') do + fill_in 'note[note]', with: 'Some new content' + find('.btn-cancel').click + expect(find('.js-note-text', visible: false).text).to eq '' + end + end + + it 'allows using markdown buttons after saving a note and then trying to edit it again' do + page.within('.current-note-edit-form') do + fill_in 'note[note]', with: 'This is the new content' + find('.btn-save').click + end + + wait_for_requests + find('.note').hover + + find('.js-note-edit').click + + page.within('.current-note-edit-form') do + expect(find('#note_note').value).to eq('This is the new content') + find('.js-md:first-child').click + expect(find('#note_note').value).to eq('This is the new content****') + end + end + + it 'appends the edited at time to the note' do + page.within('.current-note-edit-form') do + fill_in 'note[note]', with: 'Some new content' + find('.btn-save').click + end + + page.within("#note_#{note.id}") do + is_expected.to have_css('.note_edited_ago') + expect(find('.note_edited_ago').text) + .to match(/less than a minute ago/) + end + end + end + + describe 'deleting an attachment' do + before do + find('.note').hover + + find('.js-note-edit').click + end + + it 'shows the delete link' do + page.within('.note-attachment') do + is_expected.to have_css('.js-note-attachment-delete') + end + end + + it 'removes the attachment div and resets the edit form' do + accept_confirm { find('.js-note-attachment-delete').click } + is_expected.not_to have_css('.note-attachment') + is_expected.not_to have_css('.current-note-edit-form') + wait_for_requests + end + end + end +end diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb new file mode 100644 index 00000000000..61861d33952 --- /dev/null +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -0,0 +1,195 @@ +require 'rails_helper' + +describe 'Merge request > User resolves conflicts', :js do + let(:project) { create(:project, :repository) } + let(:user) { project.creator } + + before do + # In order to have the diffs collapsed, we need to disable the increase feature + stub_feature_flags(gitlab_git_diff_size_limit_increase: false) + end + + def create_merge_request(source_branch) + create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', source_project: project) do |mr| + mr.mark_as_unmergeable + end + end + + shared_examples "conflicts are resolved in Interactive mode" do + it 'conflicts are resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Use ours' + end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + all('button', text: 'Use ours').each do |button| + button.send_keys(:return) + end + end + + find_button('Commit conflict resolution').send_keys(:return) + + expect(page).to have_content('All merge conflicts were resolved') + merge_request.reload_diff + + wait_for_requests + + click_on 'Changes' + wait_for_requests + + within find('.diff-file', text: 'files/ruby/popen.rb') do + expect(page).to have_selector('.line_content.new', text: "vars = { 'PWD' => path }") + expect(page).to have_selector('.line_content.new', text: "options = { chdir: path }") + end + + within find('.diff-file', text: 'files/ruby/regex.rb') do + expect(page).to have_selector('.line_content.new', text: "def username_regexp") + expect(page).to have_selector('.line_content.new', text: "def project_name_regexp") + expect(page).to have_selector('.line_content.new', text: "def path_regexp") + expect(page).to have_selector('.line_content.new', text: "def archive_formats_regexp") + expect(page).to have_selector('.line_content.new', text: "def git_reference_regexp") + expect(page).to have_selector('.line_content.new', text: "def default_regexp") + end + end + end + + shared_examples "conflicts are resolved in Edit inline mode" do + it 'conflicts are resolved in Edit inline mode' do + expect(find('#conflicts')).to have_content('popen.rb') + + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Edit inline' + wait_for_requests + find('.files-wrapper .diff-file pre') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");') + end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + click_button 'Edit inline' + wait_for_requests + find('.files-wrapper .diff-file pre') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') + end + + find_button('Commit conflict resolution').send_keys(:return) + + expect(page).to have_content('All merge conflicts were resolved') + merge_request.reload_diff + + wait_for_requests + + click_on 'Changes' + wait_for_requests + + expect(page).to have_content('One morning') + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end + + context 'can be resolved in the UI' do + before do + project.add_developer(user) + sign_in(user) + end + + context 'the conflicts are resolvable' do + let(:merge_request) { create_merge_request('conflict-resolvable') } + + before do + visit project_merge_request_path(project, merge_request) + end + + it 'shows a link to the conflict resolution page' do + expect(page).to have_link('conflicts', href: /\/conflicts\Z/) + end + + context 'in Inline view mode' do + before do + click_link('conflicts', href: /\/conflicts\Z/) + end + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end + + context 'in Parallel view mode' do + before do + click_link('conflicts', href: /\/conflicts\Z/) + click_button 'Side-by-side' + end + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end + end + + context 'the conflict contain markers' do + let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } + + before do + visit project_merge_request_path(project, merge_request) + click_link('conflicts', href: /\/conflicts\Z/) + end + + it 'conflicts can not be resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do + expect(page).not_to have_content 'Interactive mode' + expect(page).not_to have_content 'Edit inline' + end + end + + it 'conflicts are resolved in Edit inline mode' do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do + wait_for_requests + find('.files-wrapper .diff-file pre') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') + end + + click_button 'Commit conflict resolution' + + expect(page).to have_content('All merge conflicts were resolved') + + merge_request.reload_diff + + wait_for_requests + + click_on 'Changes' + wait_for_requests + click_link 'Expand all' + wait_for_requests + + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end + end + + UNRESOLVABLE_CONFLICTS = { + 'conflict-too-large' => 'when the conflicts contain a large file', + 'conflict-binary-file' => 'when the conflicts contain a binary file', + 'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another', + 'conflict-non-utf8' => 'when the conflicts contain a non-UTF-8 file' + }.freeze + + UNRESOLVABLE_CONFLICTS.each do |source_branch, description| + context description do + let(:merge_request) { create_merge_request(source_branch) } + + before do + project.add_developer(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'does not show a link to the conflict resolution page' do + expect(page).not_to have_link('conflicts', href: /\/conflicts\Z/) + end + + it 'shows an error if the conflicts page is visited directly' do + visit current_url + '/conflicts' + wait_for_requests + + expect(find('#conflicts')).to have_content('Please try to resolve them locally.') + end + end + end +end 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 new file mode 100644 index 00000000000..590210d44ef --- /dev/null +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -0,0 +1,526 @@ +require 'rails_helper' + +describe 'Merge request > User resolves diff notes and discussions', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:guest) { create(:user) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") } + let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } + let(:path) { "files/ruby/popen.rb" } + let(:position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: nil, + new_line: 9, + diff_refs: merge_request.diff_refs + ) + end + + context 'no discussions' do + before do + project.add_master(user) + sign_in(user) + note.destroy + visit_merge_request + end + + it 'displays no discussion resolved data' do + expect(page).not_to have_content('discussion resolved') + expect(page).not_to have_selector('.discussion-next-btn') + end + end + + context 'as authorized user' do + before do + project.add_master(user) + sign_in(user) + visit_merge_request + end + + context 'single discussion' do + it 'shows text with how many discussions' do + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'allows user to mark a note as resolved' do + page.within '.diff-content .note' do + find('.line-resolve-btn').click + + expect(page).to have_selector('.line-resolve-btn.is-active') + expect(find('.line-resolve-btn')['data-original-title']).to eq("Resolved by #{user.name}") + end + + page.within '.diff-content' do + expect(page).to have_selector('.btn', text: 'Unresolve discussion') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to mark discussion as resolved' do + page.within '.diff-content' do + click_button 'Resolve discussion' + end + + expect(page).to have_selector('.discussion-body', visible: false) + + page.within '.diff-content .note' do + expect(page).to have_selector('.line-resolve-btn.is-active') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to unresolve discussion' do + page.within '.diff-content' do + click_button 'Resolve discussion' + click_button 'Unresolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + describe 'resolved discussion' do + before do + page.within '.diff-content' do + click_button 'Resolve discussion' + end + + visit_merge_request + end + + describe 'timeline view' do + it 'hides when resolve discussion is clicked' do + expect(page).to have_selector('.discussion-body', visible: false) + end + + it 'shows resolved discussion when toggled' do + find(".timeline-content .discussion[data-discussion-id='#{note.discussion_id}'] .discussion-toggle-button").click + + expect(page.find(".timeline-content #note_#{note.noteable_id}")).to be_visible + end + end + + describe 'side-by-side view' do + before do + page.within('.merge-request-tabs') { click_link 'Changes' } + page.find('#parallel-diff-btn').click + end + + it 'hides when resolve discussion is clicked' do + expect(page).to have_selector('.diffs .diff-file .notes_holder', visible: false) + end + + it 'shows resolved discussion when toggled' do + find('.diff-comment-avatar-holders').click + + expect(find('.diffs .diff-file .notes_holder')).to be_visible + end + end + end + + it 'allows user to resolve from reply form without a comment' do + page.within '.diff-content' do + click_button 'Reply...' + + click_button 'Resolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to unresolve from reply form without a comment' do + page.within '.diff-content' do + click_button 'Resolve discussion' + sleep 1 + + click_button 'Reply...' + + click_button 'Unresolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + expect(page).not_to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to comment & resolve discussion' do + page.within '.diff-content' do + click_button 'Reply...' + + find('.js-note-text').set 'testing' + + click_button 'Comment & resolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to comment & unresolve discussion' do + page.within '.diff-content' do + click_button 'Resolve discussion' + + click_button 'Reply...' + + find('.js-note-text').set 'testing' + + click_button 'Comment & unresolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'allows user to quickly scroll to next unresolved discussion' do + page.within '.line-resolve-all-container' do + page.find('.discussion-next-btn').click + end + + expect(page.evaluate_script("window.pageYOffset")).to be > 0 + end + + it 'hides jump to next button when all resolved' do + page.within '.diff-content' do + click_button 'Resolve discussion' + end + + expect(page).to have_selector('.discussion-next-btn', visible: false) + end + + it 'updates updated text after resolving note' do + page.within '.diff-content .note' do + find('.line-resolve-btn').click + end + + expect(page).to have_content("Resolved by #{user.name}") + end + + it 'hides jump to next discussion button' do + page.within '.discussion-reply-holder' do + expect(page).not_to have_selector('.discussion-next-btn') + end + end + end + + context 'multiple notes' do + before do + create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: note) + visit_merge_request + end + + it 'does not mark discussion as resolved when resolving single note' do + page.within("#note_#{note.id}") do + first('.line-resolve-btn').click + + wait_for_requests + + expect(first('.line-resolve-btn')['data-original-title']).to eq("Resolved by #{user.name}") + end + + expect(page).to have_content('Last updated') + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'resolves discussion' do + page.all('.note .line-resolve-btn').each do |button| + button.click + end + + expect(page).to have_content('Resolved by') + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + end + end + end + + context 'muliple discussions' do + before do + create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) + visit_merge_request + end + + it 'shows text with how many discussions' do + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/2 discussions resolved') + end + end + + it 'allows user to mark a single note as resolved' do + click_button('Resolve discussion', match: :first) + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/2 discussions resolved') + end + end + + it 'allows user to mark all notes as resolved' do + page.all('.line-resolve-btn').each do |btn| + btn.click + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('2/2 discussions resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user user to mark all discussions as resolved' do + page.all('.discussion-reply-holder').each do |reply_holder| + page.within reply_holder do + click_button 'Resolve discussion' + end + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('2/2 discussions resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to quickly scroll to next unresolved discussion' do + page.within first('.discussion-reply-holder') do + click_button 'Resolve discussion' + end + + page.within '.line-resolve-all-container' do + page.find('.discussion-next-btn').click + end + + expect(page.evaluate_script("window.pageYOffset")).to be > 0 + end + + it 'updates updated text after resolving note' do + page.within first('.diff-content .note') do + find('.line-resolve-btn').click + end + + expect(page).to have_content("Resolved by #{user.name}") + end + + it 'shows jump to next discussion button' do + expect(page.all('.discussion-reply-holder')).to all(have_selector('.discussion-next-btn')) + end + + it 'displays next discussion even if hidden' do + page.all('.note-discussion').each do |discussion| + page.within discussion do + click_button 'Toggle discussion' + end + end + + page.within('.issuable-discussion #notes') do + expect(page).not_to have_selector('.btn', text: 'Resolve discussion') + end + + page.within '.line-resolve-all-container' do + page.find('.discussion-next-btn').click + end + + expect(find('.discussion-with-resolve-btn')).to have_selector('.btn', text: 'Resolve discussion') + end + end + + context 'changes tab' do + it 'shows text with how many discussions' do + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'allows user to mark a note as resolved' do + page.within '.diff-content .note' do + find('.line-resolve-btn').click + + expect(page).to have_selector('.line-resolve-btn.is-active') + end + + page.within '.diff-content' do + expect(page).to have_selector('.btn', text: 'Unresolve discussion') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to mark discussion as resolved' do + page.within '.diff-content' do + click_button 'Resolve discussion' + end + + page.within '.diff-content .note' do + expect(page).to have_selector('.line-resolve-btn.is-active') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to unresolve discussion' do + page.within '.diff-content' do + click_button 'Resolve discussion' + click_button 'Unresolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'allows user to comment & resolve discussion' do + page.within '.diff-content' do + click_button 'Reply...' + + find('.js-note-text').set 'testing' + + click_button 'Comment & resolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + + it 'allows user to comment & unresolve discussion' do + page.within '.diff-content' do + click_button 'Resolve discussion' + + click_button 'Reply...' + + find('.js-note-text').set 'testing' + + click_button 'Comment & unresolve discussion' + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + end + end + + context 'as a guest' do + before do + project.add_guest(guest) + sign_in(guest) + end + + context 'someone elses merge request' do + before do + visit_merge_request + end + + it 'does not allow user to mark note as resolved' do + page.within '.diff-content .note' do + expect(page).not_to have_selector('.line-resolve-btn') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + + it 'does not allow user to mark discussion as resolved' do + page.within '.diff-content .note' do + expect(page).not_to have_selector('.btn', text: 'Resolve discussion') + end + end + end + + context 'guest users merge request' do + let(:user) { guest } + + before do + visit_merge_request + end + + it 'allows user to mark a note as resolved' do + page.within '.diff-content .note' do + find('.line-resolve-btn').click + + expect(page).to have_selector('.line-resolve-btn.is-active') + end + + page.within '.diff-content' do + expect(page).to have_selector('.btn', text: 'Unresolve discussion') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_selector('.line-resolve-btn.is-active') + end + end + end + end + + context 'unauthorized user' do + context 'no resolved comments' do + before do + visit_merge_request + end + + it 'does not allow user to mark note as resolved' do + page.within '.diff-content .note' do + expect(page).not_to have_selector('.line-resolve-btn') + end + + page.within '.line-resolve-all-container' do + expect(page).to have_content('0/1 discussion resolved') + end + end + end + + context 'resolved comment' do + before do + note.resolve!(user) + visit_merge_request + end + + it 'shows resolved icon' do + expect(page).to have_content '1/1 discussion resolved' + + click_button 'Toggle discussion' + expect(page).to have_selector('.line-resolve-btn.is-active') + end + + it 'does not allow user to click resolve button' do + expect(page).to have_selector('.line-resolve-btn.is-disabled') + click_button 'Toggle discussion' + + expect(page).to have_selector('.line-resolve-btn.is-disabled') + end + end + end + + def visit_merge_request(mr = nil) + mr ||= merge_request + visit project_merge_request_path(mr.project, mr) + end +end 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 new file mode 100644 index 00000000000..9ba9e8b9585 --- /dev/null +++ b/spec/features/merge_request/user_resolves_outdated_diff_discussions_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +feature 'Merge request > User resolves outdated diff discussions', :js do + let(:project) { create(:project, :repository, :public) } + + let(:merge_request) do + create(:merge_request, source_project: project, source_branch: 'csv', target_branch: 'master') + end + + let(:outdated_diff_refs) { project.commit('926c6595b263b2a40da6b17f3e3b7ea08344fad6').diff_refs } + let(:current_diff_refs) { merge_request.diff_refs } + + let(:outdated_position) do + Gitlab::Diff::Position.new( + old_path: 'files/csv/Book1.csv', + new_path: 'files/csv/Book1.csv', + old_line: nil, + new_line: 9, + diff_refs: outdated_diff_refs + ) + end + + let(:current_position) do + Gitlab::Diff::Position.new( + old_path: 'files/csv/Book1.csv', + new_path: 'files/csv/Book1.csv', + old_line: nil, + new_line: 1, + diff_refs: current_diff_refs + ) + end + + let!(:outdated_discussion) do + create(:diff_note_on_merge_request, + project: project, + noteable: merge_request, + position: outdated_position).to_discussion + end + + let!(:current_discussion) do + create(:diff_note_on_merge_request, + noteable: merge_request, + project: project, + position: current_position).to_discussion + end + + before do + sign_in(merge_request.author) + end + + context 'when a discussion was resolved by a push' do + before do + project.update!(resolve_outdated_diff_discussions: true) + + merge_request.update_diff_discussion_positions( + old_diff_refs: outdated_diff_refs, + new_diff_refs: current_diff_refs, + current_user: merge_request.author + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows that as automatically resolved' do + within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do + expect(page).to have_css('.discussion-body', visible: false) + expect(page).to have_content('Automatically resolved') + end + end + + it 'does not show that for active discussions' do + within(".discussion[data-discussion-id='#{current_discussion.id}']") do + expect(page).to have_css('.discussion-body', visible: true) + expect(page).not_to have_content('Automatically resolved') + end + 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 new file mode 100644 index 00000000000..8a834adbf17 --- /dev/null +++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +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) } + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + let(:fragment_id) { "#note_#{note.id}" } + + before do + sign_in(user) + page.current_window.resize_to(1000, 300) + visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}" + end + + it 'scrolls down to fragment' do + page_height = page.current_window.size[1] + page_scroll_y = page.evaluate_script("window.scrollY") + fragment_position_top = page.evaluate_script("Math.round($('#{fragment_id}').offset().top)") + + expect(find('.js-toggle-content').visible?).to eq true + expect(find(fragment_id).visible?).to eq true + expect(fragment_position_top).to be >= page_scroll_y + expect(fragment_position_top).to be < (page_scroll_y + page_height) + end +end 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 new file mode 100644 index 00000000000..9c0a04405a6 --- /dev/null +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -0,0 +1,192 @@ +require 'rails_helper' + +describe 'Merge request > User sees avatars on diff notes', :js do + include NoteInteractionHelpers + + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") } + let(:path) { "files/ruby/popen.rb" } + let(:position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: nil, + new_line: 9, + diff_refs: merge_request.diff_refs + ) + end + let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } + + before do + project.add_master(user) + sign_in user + + set_cookie('sidebar_collapsed', 'true') + end + + context 'discussion tab' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'does not show avatars on discussion tab' do + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + + it 'does not render avatars after commening on discussion tab' do + click_button 'Reply...' + + page.within('.js-discussion-note-form') do + find('.note-textarea').native.send_keys('Test comment') + + click_button 'Comment' + end + + expect(page).to have_content('Test comment') + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + end + + context 'commit view' do + before do + visit project_commit_path(project, merge_request.commits.first.id) + end + + it 'does not render avatar after commenting' do + first('.diff-line-num').click + find('.js-add-diff-note-button').click + + page.within('.js-discussion-note-form') do + find('.note-textarea').native.send_keys('test comment') + + click_button 'Comment' + + wait_for_requests + end + + visit project_merge_request_path(project, merge_request) + + expect(page).to have_content('test comment') + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + end + + %w(inline parallel).each do |view| + context "#{view} view" do + before do + visit diffs_project_merge_request_path(project, merge_request, view: view) + + wait_for_requests + end + + it 'shows note avatar' do + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 1) + end + end + + it 'shows comment on note avatar' do + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + + expect(first('img.js-diff-comment-avatar')["data-original-title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}") + end + end + + it 'toggles comments when clicking avatar' do + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + end + + expect(page).to have_selector('.notes_holder', visible: false) + + page.within find_line(position.line_code(project.repository)) do + first('img.js-diff-comment-avatar').click + end + + expect(page).to have_selector('.notes_holder') + end + + it 'removes avatar when note is deleted' do + open_more_actions_dropdown(note) + + page.within find(".note-row-#{note.id}") do + accept_confirm { find('.js-note-delete').click } + end + + wait_for_requests + + page.within find_line(position.line_code(project.repository)) do + expect(page).not_to have_selector('img.js-diff-comment-avatar') + end + end + + it 'adds avatar when commenting' do + click_button 'Reply...' + + page.within '.js-discussion-note-form' do + find('.js-note-text').native.send_keys('Test') + + click_button 'Comment' + + wait_for_requests + end + + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 2) + end + end + + it 'adds multiple comments' do + 3.times do + click_button 'Reply...' + + page.within '.js-discussion-note-form' do + find('.js-note-text').native.send_keys('Test') + find('.js-comment-button').click + + wait_for_requests + end + end + + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) + expect(find('.diff-comments-more-count')).to have_content '+1' + end + end + + context 'multiple comments' do + before do + create_list(:diff_note_on_merge_request, 3, project: project, noteable: merge_request, in_reply_to: note) + visit diffs_project_merge_request_path(project, merge_request, view: view) + + wait_for_requests + end + + it 'shows extra comment count' do + page.within find_line(position.line_code(project.repository)) do + find('.diff-notes-collapse').send_keys(:return) + + expect(find('.diff-comments-more-count')).to have_content '+1' + end + end + end + end + end + + def find_line(line_code) + line = find("[id='#{line_code}']") + line = line.find(:xpath, 'preceding-sibling::*[1][self::td]') if line.tag_name == 'td' + line + end +end 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 new file mode 100644 index 00000000000..726f35557a7 --- /dev/null +++ b/spec/features/merge_request/user_sees_closing_issues_message_spec.rb @@ -0,0 +1,76 @@ +require 'rails_helper' + +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)} + let(:issue_2) { create(:issue, project: project)} + let(:merge_request) do + create( + :merge_request, + :simple, + source_project: project, + description: merge_request_description, + title: merge_request_title + ) + end + let(:merge_request_description) { 'Merge Request Description' } + let(:merge_request_title) { 'Merge Request Title' } + + before do + project.add_master(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + wait_for_requests + end + + context 'closing issues but not mentioning any other issue' do + let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Closes #{issue_1.to_reference} and #{issue_2.to_reference}") + end + end + + context 'mentioning issues but not closing them' do + let(:merge_request_description) { "Description\n\nRefers to #{issue_1.to_reference} and #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Mentions #{issue_1.to_reference} and #{issue_2.to_reference}") + end + end + + context 'closing some issues in title and mentioning, but not closing, others' do + let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Closes #{issue_1.to_reference}") + expect(page).to have_content("Mentions #{issue_2.to_reference}") + end + end + + context 'closing issues using title but not mentioning any other issue' do + let(:merge_request_title) { "closing #{issue_1.to_reference}, #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Closes #{issue_1.to_reference} and #{issue_2.to_reference}") + end + end + + context 'mentioning issues using title but not closing them' do + let(:merge_request_title) { "Refers to #{issue_1.to_reference} and #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Mentions #{issue_1.to_reference} and #{issue_2.to_reference}") + end + end + + context 'closing some issues using title and mentioning, but not closing, others' do + let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Closes #{issue_1.to_reference}") + expect(page).to have_content("Mentions #{issue_2.to_reference}") + end + end +end 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 new file mode 100644 index 00000000000..01115318370 --- /dev/null +++ b/spec/features/merge_request/user_sees_deleted_target_branch_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +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 } + + before do + project.add_master(user) + DeleteBranchService.new(project, user).execute('feature') + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'shows a message about missing target branch' do + expect(page).to have_content('Target branch does not exist') + end + + it 'does not show link to target branch' do + expect(page).not_to have_selector('.mr-widget-body .js-branch-text a') + end +end diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb new file mode 100644 index 00000000000..3abe363d523 --- /dev/null +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -0,0 +1,56 @@ +require 'rails_helper' + +describe 'Merge request > User sees deployment widget', :js do + describe 'when deployed to an environment' do + let(:user) { create(:user) } + let(:project) { merge_request.target_project } + let(:merge_request) { create(:merge_request, :merged) } + let(:environment) { create(:environment, project: project) } + let(:role) { :developer } + let(:sha) { project.commit('master').id } + let!(:deployment) { create(:deployment, environment: environment, sha: sha) } + let!(:manual) { } + + before do + project.add_user(user, role) + sign_in(user) + visit project_merge_request_path(project, merge_request) + wait_for_requests + end + + it 'displays that the environment is deployed' do + wait_for_requests + + expect(page).to have_content("Deployed to #{environment.name}") + expect(find('.js-deploy-time')['data-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium)) + end + + context 'with stop action' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + let(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } + let(:deployment) do + create(:deployment, environment: environment, ref: merge_request.target_branch, + sha: sha, deployable: build, on_stop: 'close_app') + end + + before do + wait_for_requests + end + + it 'does start build when stop button clicked' do + accept_confirm { click_button('Stop environment') } + + expect(page).to have_content('close_app') + end + + context 'for reporter' do + let(:role) { :reporter } + + it 'does not show stop button' do + expect(page).not_to have_button('Stop environment') + end + end + end + end +end diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb new file mode 100644 index 00000000000..a9063f2bcb3 --- /dev/null +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -0,0 +1,98 @@ +require 'rails_helper' + +describe 'Merge request > User sees diff', :js do + include ProjectForksHelper + + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + + context 'when visit with */* as accept header' do + it 'renders the notes' do + create :note_on_merge_request, project: project, noteable: merge_request, note: 'Rebasing with master' + + inspect_requests(inject_headers: { 'Accept' => '*/*' }) do + visit diffs_project_merge_request_path(project, merge_request) + end + + # Load notes and diff through AJAX + expect(page).to have_css('.note-text', visible: false, text: 'Rebasing with master') + expect(page).to have_css('.diffs.tab-pane.active') + end + end + + context 'when linking to note' do + describe 'with unresolved note' do + let(:note) { create :diff_note_on_merge_request, project: project, noteable: merge_request } + let(:fragment) { "#note_#{note.id}" } + + before do + visit "#{diffs_project_merge_request_path(project, merge_request)}#{fragment}" + end + + it 'shows expanded note' do + expect(page).to have_selector(fragment, visible: true) + end + end + + describe 'with resolved note' do + let(:note) { create :diff_note_on_merge_request, :resolved, project: project, noteable: merge_request } + let(:fragment) { "#note_#{note.id}" } + + before do + visit "#{diffs_project_merge_request_path(project, merge_request)}#{fragment}" + end + + it 'shows expanded note' do + expect(page).to have_selector(fragment, visible: true) + end + end + end + + context 'when merge request has overflow' do + it 'displays warning' do + allow(Commit).to receive(:max_diff_options).and_return(max_files: 3) + + visit diffs_project_merge_request_path(project, merge_request) + + page.within('.alert') do + expect(page).to have_text("Too many changes to show. Plain diff Email patch To preserve + performance only 3 of 3+ files are displayed.") + end + end + end + + context 'when editing file' do + let(:author_user) { create(:user) } + let(:user) { create(:user) } + let(:forked_project) { fork_project(project, author_user, repository: true) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) } + let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") } + + before do + forked_project.repository.after_import + end + + context 'as author' do + it 'shows direct edit link' do + sign_in(author_user) + visit diffs_project_merge_request_path(project, merge_request) + + # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax + expect(page).to have_selector("[id=\"#{changelog_id}\"] a.js-edit-blob") + end + end + + context 'as user who needs to fork' do + it 'shows fork/cancel confirmation' do + sign_in(user) + visit diffs_project_merge_request_path(project, merge_request) + + # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax + find("[id=\"#{changelog_id}\"] .js-edit-blob").click + + expect(page).to have_selector('.js-fork-suggestion-button', count: 1) + expect(page).to have_selector('.js-cancel-fork-suggestion-button', count: 1) + end + end + end +end diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb new file mode 100644 index 00000000000..d6e8c8e86ba --- /dev/null +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -0,0 +1,87 @@ +require 'rails_helper' + +describe 'Merge request > User sees discussions' do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:merge_request) { create(:merge_request, source_project: project) } + + before do + project.add_master(user) + sign_in(user) + end + + describe "Diff discussions" do + let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: outdated_diff_refs) } + let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create } + let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion } + let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + let(:outdated_position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 9, + diff_refs: outdated_diff_refs + ) + end + let(:outdated_diff_refs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs } + + before do + visit project_merge_request_path(project, merge_request) + end + + context 'active discussions' do + it 'shows a link to the diff' do + within(".discussion[data-discussion-id='#{active_discussion.id}']") do + path = diffs_project_merge_request_path(project, merge_request, anchor: active_discussion.line_code) + expect(page).to have_link('the diff', href: path) + end + end + end + + context 'outdated discussions' do + it 'shows a link to the outdated diff' do + within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do + path = diffs_project_merge_request_path(project, merge_request, diff_id: old_merge_request_diff.id, anchor: outdated_discussion.line_code) + expect(page).to have_link('an old version of the diff', href: path) + end + end + end + end + + describe 'Commit comments displayed in MR context', :js do + shared_examples 'a functional discussion' do + let(:discussion_id) { note.discussion_id(merge_request) } + + it 'is displayed' do + expect(page).to have_css(".discussion[data-discussion-id='#{discussion_id}']") + end + + it 'can be replied to' do + within(".discussion[data-discussion-id='#{discussion_id}']") do + click_button 'Reply...' + fill_in 'note[note]', with: 'Test!' + click_button 'Comment' + + expect(page).to have_css('.note', count: 2) + end + end + end + + before do + visit project_merge_request_path(project, merge_request) + end + + context 'a regular commit comment' do + let(:note) { create(:note_on_commit, project: project) } + + it_behaves_like 'a functional discussion' + end + + context 'a commit diff comment' do + let(:note) { create(:diff_note_on_commit, project: project) } + + it_behaves_like 'a functional discussion' + end + end +end diff --git a/spec/features/merge_request/user_sees_empty_state_spec.rb b/spec/features/merge_request/user_sees_empty_state_spec.rb new file mode 100644 index 00000000000..a939c7e9001 --- /dev/null +++ b/spec/features/merge_request/user_sees_empty_state_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +describe 'Merge request > User sees empty state' do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + + before do + project.add_master(user) + sign_in(user) + end + + it 'shows an empty state and a "New merge request" button' do + visit project_merge_requests_path(project) + + expect(page).to have_selector('.empty-state') + expect(page).to have_link 'New merge request', href: project_new_merge_request_path(project) + end + + context 'if there are merge requests' do + before do + create(:merge_request, source_project: project) + + visit project_merge_requests_path(project) + end + + it 'does not show an empty state' do + expect(page).not_to have_selector('.empty-state') + end + end +end 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 new file mode 100644 index 00000000000..85df43df38e --- /dev/null +++ b/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' + +describe 'Merge request > User sees merge button depending on unresolved discussions', :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) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true) + visit project_merge_request_path(project, merge_request) + end + + context 'with unresolved discussions' do + it 'does not allow to merge' do + expect(page).not_to have_button 'Merge' + expect(page).to have_content('There are unresolved discussions.') + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + visit project_merge_request_path(project, merge_request) + end + + it 'allows MR to be merged' do + expect(page).to have_button 'Merge' + end + end + end + + context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false) + visit project_merge_request_path(project, merge_request) + end + + context 'with unresolved discussions' do + it 'does not allow to merge' do + expect(page).to have_button 'Merge' + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + visit project_merge_request_path(project, merge_request) + end + + it 'allows MR to be merged' do + expect(page).to have_button 'Merge' + end + end + end +end diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb new file mode 100644 index 00000000000..56224e505d9 --- /dev/null +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -0,0 +1,304 @@ +require 'rails_helper' + +describe 'Merge request > User sees merge widget', :js do + let(:project) { create(:project, :repository) } + let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) } + let(:user) { project.creator } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:merge_request_in_only_mwps_project) { create(:merge_request, source_project: project_only_mwps) } + + before do + project.add_master(user) + project_only_mwps.add_master(user) + sign_in(user) + end + + context 'new merge request' do + before do + visit project_new_merge_request_path( + project, + merge_request: { + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'feature', + target_branch: 'master' + }) + end + + it 'shows widget status after creating new merge request' do + click_button 'Submit merge request' + + wait_for_requests + + expect(page).to have_selector('.accept-merge-request') + expect(find('.accept-merge-request')['disabled']).not_to be(true) + end + end + + context 'view merge request' do + let!(:environment) { create(:environment, project: project) } + + let!(:deployment) do + create(:deployment, environment: environment, + ref: 'feature', + sha: merge_request.diff_head_sha) + end + + before do + visit project_merge_request_path(project, merge_request) + end + + it 'shows environments link' do + wait_for_requests + + page.within('.mr-widget-heading') do + expect(page).to have_content("Deployed to #{environment.name}") + expect(find('.js-deploy-url')[:href]).to include(environment.formatted_external_url) + end + end + + it 'shows green accept merge request button' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + expect(page).to have_selector('.accept-merge-request') + expect(find('.accept-merge-request')['disabled']).not_to be(true) + end + + it 'allows me to merge, see cherry-pick modal and load branches list' do + wait_for_requests + click_button 'Merge' + + wait_for_requests + click_link 'Cherry-pick' + page.find('.js-project-refs-dropdown').click + wait_for_requests + + expect(page.all('.js-cherry-pick-form .dropdown-content li').size).to be > 1 + end + end + + context 'view merge request with external CI service' do + before do + create(:service, project: project, + active: true, + type: 'CiService', + category: 'ci') + + visit project_merge_request_path(project, merge_request) + end + + it 'has danger button while waiting for external CI status' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + expect(page).to have_selector('.accept-merge-request.btn-danger') + end + end + + context 'view merge request with failed GitLab CI pipelines' do + before do + commit_status = create(:commit_status, project: project, status: 'failed') + pipeline = create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: 'failed', + statuses: [commit_status], + head_pipeline_of: merge_request) + create(:ci_build, :pending, pipeline: pipeline) + + visit project_merge_request_path(project, merge_request) + end + + it 'has danger button when not succeeded' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + expect(page).to have_selector('.accept-merge-request.btn-danger') + end + end + + context 'when merge request is in the blocked pipeline state' do + before do + create( + :ci_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: :manual, + head_pipeline_of: merge_request) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about blocked pipeline' do + expect(page).to have_content("Pipeline blocked") + expect(page).to have_content( + "The pipeline for this merge request requires a manual action") + expect(page).to have_css('.ci-status-icon-manual') + end + end + + context 'view merge request with MWBS button' do + before do + commit_status = create(:commit_status, project: project, status: 'pending') + pipeline = create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: 'pending', + statuses: [commit_status], + head_pipeline_of: merge_request) + create(:ci_build, :pending, pipeline: pipeline) + + visit project_merge_request_path(project, merge_request) + end + + it 'has info button when MWBS button' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + expect(page).to have_selector('.accept-merge-request.btn-info') + end + end + + context 'view merge request where project has CI setup but no CI status' do + before do + pipeline = create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) + create(:ci_build, pipeline: pipeline) + + visit project_merge_request_path(project, merge_request) + end + + it 'has pipeline error text' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + expect(page).to have_text('Could not connect to the CI server. Please check your settings and try again') + end + end + + context 'view merge request in project with only-mwps setting enabled but no CI is setup' do + before do + visit project_merge_request_path(project_only_mwps, merge_request_in_only_mwps_project) + end + + it 'should be allowed to merge' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + expect(page).to have_selector('.accept-merge-request') + expect(find('.accept-merge-request')['disabled']).not_to be(true) + end + end + + context 'view merge request with MWPS enabled but automatically merge fails' do + before do + merge_request.update( + merge_when_pipeline_succeeds: true, + merge_user: merge_request.author, + merge_error: 'Something went wrong' + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Something went wrong') + end + end + end + + context 'view merge request with MWPS enabled but automatically merge fails' do + before do + merge_request.update( + merge_when_pipeline_succeeds: true, + merge_user: merge_request.author, + merge_error: 'Something went wrong' + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Something went wrong') + end + end + end + + context 'view merge request where fast-forward merge is not possible' do + before do + project.update(merge_requests_ff_only_enabled: true) + + merge_request.update( + merge_user: merge_request.author, + merge_status: :cannot_be_merged + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Fast-forward merge is not possible') + end + end + end + + context 'merge error' do + before do + allow_any_instance_of(Repository).to receive(:merge).and_return(false) + visit project_merge_request_path(project, merge_request) + end + + it 'updates the MR widget' do + click_button 'Merge' + + page.within('.mr-widget-body') do + expect(page).to have_content('Conflicts detected during merge') + end + end + end + + context 'user can merge into source project but cannot push to fork', :js do + let(:fork_project) { create(:project, :public, :repository) } + let(:user2) { create(:user) } + + before do + project.add_master(user2) + sign_out(:user) + sign_in(user2) + merge_request.update(target_project: fork_project) + visit project_merge_request_path(project, merge_request) + end + + it 'user can merge into the source project' do + expect(page).to have_button('Merge', disabled: false) + end + + it 'user cannot remove source branch' do + expect(page).to have_field('remove-source-branch-input', disabled: true) + end + end + + context 'ongoing merge process' do + it 'shows Merging state' do + allow_any_instance_of(MergeRequest).to receive(:merge_ongoing?).and_return(true) + + visit project_merge_request_path(project, merge_request) + + wait_for_requests + + expect(page).not_to have_button('Merge') + expect(page).to have_content('This merge request is in the process of being merged') + end + end +end 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 new file mode 100644 index 00000000000..a43ba05c64c --- /dev/null +++ b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb @@ -0,0 +1,124 @@ +require 'rails_helper' + +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) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running', sha: project.commit.id) } + let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') } + + before do + build.run + sign_in(user) + visit_merge_request + end + + def visit_merge_request(format: :html, serializer: nil) + visit project_merge_request_path(project, merge_request, format: format, serializer: serializer) + end + + it 'displays a mini pipeline graph' do + expect(page).to have_selector('.mr-widget-pipeline-graph') + end + + context 'as json' do + let(:artifacts_file1) { fixture_file_upload(Rails.root.join('spec/fixtures/banana_sample.gif'), 'image/gif') } + let(:artifacts_file2) { fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'), 'image/png') } + + before do + create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file1) + create(:ci_build, pipeline: pipeline, when: 'manual') + end + + it 'avoids repeated database queries' do + before = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } + + create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file2) + create(:ci_build, pipeline: pipeline, when: 'manual') + + after = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } + + expect(before.count).to eq(after.count) + expect(before.cached_count).to eq(after.cached_count) + end + end + + describe 'build list toggle' do + let(:toggle) do + find('.mini-pipeline-graph-dropdown-toggle') + first('.mini-pipeline-graph-dropdown-toggle') + end + + it 'expands when hovered' do + find('.mini-pipeline-graph-dropdown-toggle') + before_width = evaluate_script("$('.mini-pipeline-graph-dropdown-toggle:visible').outerWidth();") + + toggle.hover + + find('.mini-pipeline-graph-dropdown-toggle') + after_width = evaluate_script("$('.mini-pipeline-graph-dropdown-toggle:visible').outerWidth();") + + expect(before_width).to be < after_width + end + + it 'shows dropdown caret when hovered' do + toggle.hover + + expect(toggle).to have_selector('.fa-caret-down') + end + + it 'shows tooltip when hovered' do + toggle.hover + + expect(page).to have_selector('.tooltip') + end + end + + describe 'builds list menu' do + let(:toggle) do + find('.mini-pipeline-graph-dropdown-toggle') + first('.mini-pipeline-graph-dropdown-toggle') + end + + before do + toggle.click + wait_for_requests + end + + it 'pens when toggle is clicked' do + expect(toggle.find(:xpath, '..')).to have_selector('.mini-pipeline-graph-dropdown-menu') + end + + it 'closes when toggle is clicked again' do + toggle.click + + expect(toggle.find(:xpath, '..')).not_to have_selector('.mini-pipeline-graph-dropdown-menu') + end + + it 'closes when clicking somewhere else' do + find('body').click + + expect(toggle.find(:xpath, '..')).not_to have_selector('.mini-pipeline-graph-dropdown-menu') + end + + describe 'build list build item' do + let(:build_item) do + find('.mini-pipeline-graph-dropdown-item') + first('.mini-pipeline-graph-dropdown-item') + end + + it 'visits the build page when clicked' do + build_item.click + find('.build-page') + + expect(current_path).to eql(project_job_path(project, build)) + end + + it 'shows tooltip when hovered' do + build_item.hover + + expect(page).to have_selector('.tooltip') + end + end + end +end 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 new file mode 100644 index 00000000000..029b66b5e8e --- /dev/null +++ b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +describe 'Merge request > User sees MR from deleted forked project', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:fork_project) { create(:project, :public, :repository, forked_from_project: project) } + let!(:merge_request) do + create(:merge_request_with_diffs, source_project: fork_project, + target_project: project, + description: 'Test merge request') + end + + before do + MergeRequests::MergeService.new(project, user).execute(merge_request) + fork_project.destroy! + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'user can access merge request' do + expect(page).to have_content 'Test merge request' + expect(page).to have_content "(removed):#{merge_request.source_branch}" + end +end 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 new file mode 100644 index 00000000000..c1608be402a --- /dev/null +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -0,0 +1,34 @@ +require 'rails_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 + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:user) { project.creator } + + before do + merge_request.update!(source_branch: 'this-branch-does-not-exist') + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'shows a message about missing source branch' do + expect(page).to have_content('Source branch does not exist.') + end + + it 'still contains Discussion, Commits and Changes tabs' do + within '.merge-request-details' do + expect(page).to have_content('Discussion') + expect(page).to have_content('Commits') + expect(page).to have_content('Changes') + end + + click_on 'Changes' + wait_for_requests + + expect(page).to have_selector('.diffs.tab-pane .nothing-here-block') + expect(page).to have_content('Source branch does not exist.') + end +end 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 new file mode 100644 index 00000000000..b4cda269852 --- /dev/null +++ b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +describe 'Merge request > User sees notes from forked project', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:fork_project) { create(:project, :public, :repository, forked_from_project: project) } + let!(:merge_request) do + create(:merge_request_with_diffs, source_project: fork_project, + target_project: project, + description: 'Test merge request') + end + + before do + create(:note_on_commit, note: 'A commit comment', + project: fork_project, + commit_id: merge_request.commit_shas.first) + sign_in(user) + end + + it 'user can reply to the comment' do + visit project_merge_request_path(project, merge_request) + + expect(page).to have_content('A commit comment') + + page.within('.discussion-notes') do + find('.btn-text-field').click + find('#note_note').send_keys('A reply comment') + find('.comment-btn').click + end + + wait_for_requests + + expect(page).to have_content('A reply comment') + end +end 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 new file mode 100644 index 00000000000..d30dcefc6aa --- /dev/null +++ b/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +describe 'Merge request > User sees pipelines from forked project', :js do + let(:target_project) { create(:project, :public, :repository) } + let(:user) { target_project.creator } + let(:fork_project) { create(:project, :repository, forked_from_project: target_project) } + let!(:merge_request) do + create(:merge_request_with_diffs, source_project: fork_project, + target_project: target_project, + description: 'Test merge request') + end + let(:pipeline) do + create(:ci_pipeline, + project: fork_project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) + end + + before do + create(:ci_build, pipeline: pipeline, name: 'rspec') + create(:ci_build, pipeline: pipeline, name: 'spinach') + + sign_in(user) + visit project_merge_request_path(target_project, merge_request) + end + + it 'user visits a pipelines page' do + page.within('.merge-request-tabs') { click_link 'Pipelines' } + + page.within('.ci-table') do + expect(page).to have_content(pipeline.id) + end + end +end diff --git a/spec/features/merge_request/user_sees_pipelines_spec.rb b/spec/features/merge_request/user_sees_pipelines_spec.rb new file mode 100644 index 00000000000..a42c016392b --- /dev/null +++ b/spec/features/merge_request/user_sees_pipelines_spec.rb @@ -0,0 +1,102 @@ +require 'rails_helper' + +describe 'Merge request > User sees pipelines', :js do + describe 'pipeline tab' do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.target_project } + let(:user) { project.creator } + + before do + project.add_master(user) + sign_in(user) + end + + context 'with pipelines' do + let!(:pipeline) do + create(:ci_empty_pipeline, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + before do + merge_request.update_attribute(:head_pipeline_id, pipeline.id) + end + + it 'user visits merge request pipelines tab' do + visit project_merge_request_path(project, merge_request) + + expect(page.find('.ci-widget')).to have_content('pending') + + page.within('.merge-request-tabs') do + click_link('Pipelines') + end + wait_for_requests + + expect(page).to have_selector('.stage-cell') + end + + it 'pipeline sha does not equal last commit sha' do + pipeline.update_attribute(:sha, '19e2e9b4ef76b422ce1154af39a91323ccc57434') + visit project_merge_request_path(project, merge_request) + wait_for_requests + + expect(page.find('.ci-widget')).to have_content( + 'Could not connect to the CI server. Please check your settings and try again') + end + end + + context 'without pipelines' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'user visits merge request page' do + page.within('.merge-request-tabs') do + expect(page).to have_no_link('Pipelines') + end + end + end + end + + describe 'race condition' do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:build_push_data) { { ref: 'feature', checkout_sha: TestEnv::BRANCH_SHA['feature'] } } + + let(:merge_request_params) do + { "source_branch" => "feature", "source_project_id" => project.id, + "target_branch" => "master", "target_project_id" => project.id, "title" => "A" } + end + + before do + project.add_master(user) + sign_in user + end + + context 'when pipeline and merge request were created simultaneously' do + before do + stub_ci_pipeline_to_return_yaml_file + + threads = [] + + threads << Thread.new do + @merge_request = MergeRequests::CreateService.new(project, user, merge_request_params).execute + end + + threads << Thread.new do + @pipeline = Ci::CreatePipelineService.new(project, user, build_push_data).execute(:push) + end + + threads.each { |thr| thr.join } + end + + it 'user sees pipeline in merge request widget' do + visit project_merge_request_path(project, @merge_request) + + expect(page.find(".ci-widget")).to have_content(TestEnv::BRANCH_SHA['feature']) + expect(page.find(".ci-widget")).to have_content("##{@pipeline.id}") + end + end + end +end diff --git a/spec/features/merge_request/user_sees_system_notes_spec.rb b/spec/features/merge_request/user_sees_system_notes_spec.rb new file mode 100644 index 00000000000..a00a682757d --- /dev/null +++ b/spec/features/merge_request/user_sees_system_notes_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe 'Merge request > User sees system notes' do + let(:public_project) { create(:project, :public, :repository) } + let(:private_project) { create(:project, :private, :repository) } + let(:user) { private_project.creator } + let(:issue) { create(:issue, project: private_project) } + let(:merge_request) { create(:merge_request, source_project: public_project, source_branch: 'markdown') } + let!(:note) { create(:note_on_merge_request, :system, noteable: merge_request, project: public_project, note: "mentioned in #{issue.to_reference(public_project)}") } + + context 'when logged-in as a member of the private project' do + before do + private_project.add_developer(user) + sign_in(user) + end + + it 'shows the system note' do + visit project_merge_request_path(public_project, merge_request) + + expect(page).to have_css('.system-note') + end + end + + context 'when not logged-in' do + it 'hides the system note' do + visit project_merge_request_path(public_project, merge_request) + + expect(page).not_to have_css('.system-note') + end + end +end diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb new file mode 100644 index 00000000000..3a15d70979a --- /dev/null +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -0,0 +1,217 @@ +require 'rails_helper' + +describe 'Merge request > User sees versions', :js do + let(:merge_request) { create(:merge_request, importing: true) } + let(:project) { merge_request.source_project } + let(:user) { project.creator } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:params) { {} } + + before do + project.add_master(user) + sign_in(user) + visit diffs_project_merge_request_path(project, merge_request, params) + end + + shared_examples 'allows commenting' do |file_id:, line_code:, comment:| + it do + diff_file_selector = ".diff-file[id='#{file_id}']" + line_code = "#{file_id}_#{line_code}" + + page.within(diff_file_selector) do + find(".line_holder[id='#{line_code}'] td:nth-of-type(1)").hover + find(".line_holder[id='#{line_code}'] button").click + + page.within("form[data-line-code='#{line_code}']") do + fill_in "note[note]", with: comment + find(".js-comment-button").click + end + + wait_for_requests + + expect(page).to have_content(comment) + end + end + end + + describe 'compare with the latest version' do + it 'show the latest version of the diff' do + page.within '.mr-version-dropdown' do + expect(page).to have_content 'latest version' + end + + expect(page).to have_content '8 changed files' + end + + it_behaves_like 'allows commenting', + file_id: '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44', + line_code: '1_1', + comment: 'Typo, please fix.' + end + + describe 'switch between versions' do + before do + page.within '.mr-version-dropdown' do + find('.btn-default').click + click_link 'version 1' + end + + # Wait for the page to load + page.within '.mr-version-dropdown' do + expect(page).to have_content 'version 1' + end + end + + it 'shows comments that were last relevant at that version' do + expect(page).to have_content '5 changed files' + expect(page).to have_content 'Not all comments are displayed' + + position = Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: nil, + new_line: 4, + diff_refs: merge_request_diff1.diff_refs + ) + outdated_diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + outdated_diff_note.position = outdated_diff_note.original_position + outdated_diff_note.save! + + refresh + + expect(page).to have_css(".diffs .notes[data-discussion-id='#{outdated_diff_note.discussion_id}']") + end + + it_behaves_like 'allows commenting', + file_id: '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44', + line_code: '2_2', + comment: 'Typo, please fix.' + end + + describe 'compare with older version' do + before do + page.within '.mr-version-compare-dropdown' do + find('.btn-default').click + click_link 'version 1' + end + + # Wait for the page to load + page.within '.mr-version-compare-dropdown' do + expect(page).to have_content 'version 1' + end + end + + it 'has a path with comparison context and shows comments that were last relevant at that version' do + expect(page).to have_current_path diffs_project_merge_request_path( + project, + merge_request.iid, + diff_id: merge_request_diff3.id, + start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + ) + expect(page).to have_content '4 changed files with 15 additions and 6 deletions' + expect(page).to have_content 'Not all comments are displayed' + + position = Gitlab::Diff::Position.new( + old_path: ".gitmodules", + new_path: ".gitmodules", + old_line: 4, + new_line: 4, + diff_refs: merge_request_diff3.compare_with(merge_request_diff1.head_commit_sha).diff_refs + ) + outdated_diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + outdated_diff_note.position = outdated_diff_note.original_position + outdated_diff_note.save! + + refresh + wait_for_requests + + expect(page).to have_css(".diffs .notes[data-discussion-id='#{outdated_diff_note.discussion_id}']") + end + + it 'show diff between new and old version' do + expect(page).to have_content '4 changed files with 15 additions and 6 deletions' + end + + it 'returns to latest version when "Show latest version" button is clicked' do + click_link 'Show latest version' + page.within '.mr-version-dropdown' do + expect(page).to have_content 'latest version' + end + expect(page).to have_content '8 changed files' + end + + it_behaves_like 'allows commenting', + file_id: '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44', + line_code: '4_4', + comment: 'Typo, please fix.' + end + + describe 'compare with same version' do + before do + page.within '.mr-version-compare-dropdown' do + find('.btn-default').click + click_link 'version 1' + end + end + + it 'has 0 chages between versions' do + page.within '.mr-version-compare-dropdown' do + expect(find('.dropdown-toggle')).to have_content 'version 1' + end + + page.within '.mr-version-dropdown' do + find('.btn-default').click + click_link 'version 1' + end + expect(page).to have_content '0 changed files' + end + end + + describe 'compare with newer version' do + before do + page.within '.mr-version-compare-dropdown' do + find('.btn-default').click + click_link 'version 2' + end + end + + it 'sets the compared versions to be the same' do + page.within '.mr-version-compare-dropdown' do + expect(find('.dropdown-toggle')).to have_content 'version 2' + end + + page.within '.mr-version-dropdown' do + find('.btn-default').click + click_link 'version 1' + end + + page.within '.mr-version-compare-dropdown' do + expect(page).to have_content 'version 1' + end + + expect(page).to have_content '0 changed files' + end + end + + describe 'scoped in a commit' do + let(:params) { { commit_id: '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' } } + + before do + wait_for_requests + end + + it 'should only show diffs from the commit' do + diff_commit_ids = find_all('.diff-file [data-commit-id]').map {|diff| diff['data-commit-id']} + + expect(diff_commit_ids).not_to be_empty + expect(diff_commit_ids).to all(eq(params[:commit_id])) + end + + it_behaves_like 'allows commenting', + file_id: '2f6fcd96b88b36ce98c38da085c795a27d92a3dd', + line_code: '6_6', + comment: 'Typo, please fix.' + end +end 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 new file mode 100644 index 00000000000..bc25243244e --- /dev/null +++ b/spec/features/merge_request/user_sees_wip_help_message_spec.rb @@ -0,0 +1,59 @@ +require 'rails_helper' + +describe 'Merge request > User sees WIP help message' do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + + before do + project.add_master(user) + sign_in(user) + end + + context 'with WIP commits' do + it 'shows a specific WIP hint' do + visit project_new_merge_request_path( + project, + merge_request: { + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'wip', + target_branch: 'master' + }) + + within_wip_explanation do + expect(page).to have_text( + 'It looks like you have some WIP commits in this branch' + ) + end + end + end + + context 'without WIP commits' do + it 'shows the regular WIP message' do + visit project_new_merge_request_path( + project, + merge_request: { + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'fix', + target_branch: 'master' + }) + + within_wip_explanation do + expect(page).not_to have_text( + 'It looks like you have some WIP commits in this branch' + ) + expect(page).to have_text( + "Start the title with WIP: to prevent a Work In Progress merge \ +request from being merged before it's ready" + ) + end + end + end + + def within_wip_explanation(&block) + page.within '.js-no-wip-explanation' do + yield + end + end +end 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 new file mode 100644 index 00000000000..fb73ab05f87 --- /dev/null +++ b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb @@ -0,0 +1,180 @@ +require 'rails_helper' + +describe 'Merge request > User selects branches for new MR', :js do + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + + before do + project.add_master(user) + sign_in(user) + end + + it 'selects the source branch sha when a tag with the same name exists' do + visit project_merge_requests_path(project) + + page.within '.content' do + click_link 'New merge request' + end + expect(page).to have_content('Source branch') + expect(page).to have_content('Target branch') + + first('.js-source-branch').click + find('.dropdown-source-branch .dropdown-content a', match: :first).click + + expect(page).to have_content "b83d6e3" + end + + it 'selects the target branch sha when a tag with the same name exists' do + visit project_merge_requests_path(project) + + page.within '.content' do + click_link 'New merge request' + end + + expect(page).to have_content('Source branch') + expect(page).to have_content('Target branch') + + first('.js-target-branch').click + find('.dropdown-target-branch .dropdown-content a', text: 'v1.1.0', match: :first).click + + expect(page).to have_content "b83d6e3" + end + + it 'generates a diff for an orphaned branch' do + visit project_merge_requests_path(project) + + page.within '.content' do + click_link 'New merge request' + end + expect(page).to have_content('Source branch') + expect(page).to have_content('Target branch') + + find('.js-source-branch', match: :first).click + find('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch', match: :first).click + + click_button "Compare branches" + click_link "Changes" + + expect(page).to have_content "README.md" + expect(page).to have_content "wm.png" + + fill_in "merge_request_title", with: "Orphaned MR test" + click_button "Submit merge request" + + click_link "Check out branch" + + expect(page).to have_content 'git checkout -b orphaned-branch origin/orphaned-branch' + end + + it 'allows filtering multiple dropdowns' do + visit project_new_merge_request_path(project) + + first('.js-source-branch').click + + input = find('.dropdown-source-branch .dropdown-input-field') + input.click + input.send_keys('orphaned-branch') + + find('.dropdown-source-branch .dropdown-content li', match: :first) + source_items = all('.dropdown-source-branch .dropdown-content li') + + expect(source_items.count).to eq(1) + + first('.js-target-branch').click + + find('.dropdown-target-branch .dropdown-content li', match: :first) + target_items = all('.dropdown-target-branch .dropdown-content li') + + expect(target_items.count).to be > 1 + end + + context 'when target project cannot be viewed by the current user' do + it 'does not leak the private project name & namespace' do + private_project = create(:project, :private, :repository) + + visit project_new_merge_request_path(project, merge_request: { target_project_id: private_project.id }) + + expect(page).not_to have_content private_project.full_path + expect(page).to have_content project.full_path + end + end + + context 'when source project cannot be viewed by the current user' do + it 'does not leak the private project name & namespace' do + private_project = create(:project, :private, :repository) + + visit project_new_merge_request_path(project, merge_request: { source_project_id: private_project.id }) + + expect(page).not_to have_content private_project.full_path + expect(page).to have_content project.full_path + end + end + + it 'populates source branch button' do + visit project_new_merge_request_path(project, change_branches: true, merge_request: { target_branch: 'master', source_branch: 'fix' }) + + expect(find('.js-source-branch')).to have_content('fix') + end + + it 'allows to change the diff view' do + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'fix' }) + + click_link 'Changes' + + expect(page).to have_css('a.btn.active', text: 'Inline') + expect(page).not_to have_css('a.btn.active', text: 'Side-by-side') + + click_link 'Side-by-side' + + within '.merge-request' do + expect(page).not_to have_css('a.btn.active', text: 'Inline') + expect(page).to have_css('a.btn.active', text: 'Side-by-side') + end + end + + it 'does not allow non-existing branches' do + visit project_new_merge_request_path(project, merge_request: { target_branch: 'non-exist-target', source_branch: 'non-exist-source' }) + + expect(page).to have_content('The form contains the following errors') + expect(page).to have_content('Source branch "non-exist-source" does not exist') + expect(page).to have_content('Target branch "non-exist-target" does not exist') + end + + context 'when a branch contains commits that both delete and add the same image' do + it 'renders the diff successfully' do + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'deleted-image-test' }) + + click_link "Changes" + + expect(page).to have_content "6049019_460s.jpg" + end + end + + # Isolates a regression (see #24627) + it 'does not show error messages on initial form' do + visit project_new_merge_request_path(project) + expect(page).not_to have_selector('#error_explanation') + expect(page).not_to have_content('The form contains the following error') + end + + context 'when a new merge request has a pipeline' do + let!(:pipeline) do + create(:ci_pipeline, sha: project.commit('fix').id, + ref: 'fix', + project: project) + end + + it 'shows pipelines for a new merge request' do + visit project_new_merge_request_path( + project, + merge_request: { target_branch: 'master', source_branch: 'fix' }) + + page.within('.merge-request') do + click_link 'Pipelines' + wait_for_requests + + expect(page).to have_content "##{pipeline.id}" + end + end + end +end diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb new file mode 100644 index 00000000000..2e95a628013 --- /dev/null +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe 'Merge request > User toggles whitespace changes', :js do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:user) { project.creator } + + before do + project.add_master(user) + sign_in(user) + visit diffs_project_merge_request_path(project, merge_request) + end + + it 'has a button to toggle whitespace changes' do + expect(page).to have_content 'Hide whitespace changes' + end + + describe 'clicking "Hide whitespace changes" button' do + it 'toggles the "Hide whitespace changes" button' do + click_link 'Hide whitespace changes' + + expect(page).to have_content 'Show whitespace changes' + end + end +end diff --git a/spec/features/merge_request/user_uses_slash_commands_spec.rb b/spec/features/merge_request/user_uses_slash_commands_spec.rb new file mode 100644 index 00000000000..bd739e69d6c --- /dev/null +++ b/spec/features/merge_request/user_uses_slash_commands_spec.rb @@ -0,0 +1,202 @@ +require 'rails_helper' + +describe 'Merge request > User uses quick actions', :js do + include QuickActionsHelpers + + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:guest) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:milestone) { create(:milestone, project: project, title: 'ASAP') } + + it_behaves_like 'issuable record that supports quick actions in its description and notes', :merge_request do + let(:issuable) { create(:merge_request, source_project: project) } + let(:new_url_opts) { { merge_request: { source_branch: 'feature', target_branch: 'master' } } } + end + + describe 'merge-request-only commands' do + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:milestone) { create(:milestone, project: project, title: 'ASAP') } + + before do + project.add_master(user) + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + describe 'time tracking' do + it_behaves_like 'issuable time tracker' + end + + describe 'toggling the WIP prefix in the title from note' do + context 'when the current user can toggle the WIP prefix' do + it 'adds the WIP: prefix to the title' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Commands applied' + + expect(merge_request.reload.work_in_progress?).to eq true + end + + it 'removes the WIP: prefix from the title' do + merge_request.title = merge_request.wip_title + merge_request.save + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).to have_content 'Commands applied' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + + context 'when the current user cannot toggle the WIP prefix' do + before do + project.add_guest(guest) + sign_out(:user) + sign_in(guest) + visit project_merge_request_path(project, merge_request) + end + + it 'does not change the WIP prefix' do + write_note("/wip") + + expect(page).not_to have_content '/wip' + expect(page).not_to have_content 'Commands applied' + + expect(merge_request.reload.work_in_progress?).to eq false + end + end + end + + describe 'merging the MR from the note' do + context 'when the current user can merge the MR' do + it 'merges the MR' do + write_note("/merge") + + expect(page).to have_content 'Commands applied' + + expect(merge_request.reload).to be_merged + end + end + + context 'when the head diff changes in the meanwhile' do + before do + merge_request.source_branch = 'another_branch' + merge_request.save + end + + it 'does not merge the MR' do + write_note("/merge") + + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload).not_to be_merged + end + end + + context 'when the current user cannot merge the MR' do + before do + project.add_guest(guest) + sign_out(:user) + sign_in(guest) + visit project_merge_request_path(project, merge_request) + end + + it 'does not merge the MR' do + write_note("/merge") + + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload).not_to be_merged + end + end + end + + describe 'adding a due date from note' do + it 'does not recognize the command nor create a note' do + write_note('/due 2016-08-28') + + expect(page).not_to have_content '/due 2016-08-28' + end + end + + describe '/target_branch command in merge request' do + let(:another_project) { create(:project, :public, :repository) } + let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } } + + before do + sign_out(:user) + another_project.add_master(user) + sign_in(user) + end + + it 'changes target_branch in new merge_request' do + visit project_new_merge_request_path(another_project, new_url_opts) + + fill_in "merge_request_title", with: 'My brand new feature' + fill_in "merge_request_description", with: "le feature \n/target_branch fix\nFeature description:" + click_button "Submit merge request" + + merge_request = another_project.merge_requests.first + expect(merge_request.description).to eq "le feature \nFeature description:" + expect(merge_request.target_branch).to eq 'fix' + end + + it 'does not change target branch when merge request is edited' do + new_merge_request = create(:merge_request, source_project: another_project) + + visit edit_project_merge_request_path(another_project, new_merge_request) + fill_in "merge_request_description", with: "Want to update target branch\n/target_branch fix\n" + click_button "Save changes" + + new_merge_request = another_project.merge_requests.first + expect(new_merge_request.description).to include('/target_branch') + expect(new_merge_request.target_branch).not_to eq('fix') + end + end + + describe '/target_branch command from note' do + context 'when the current user can change target branch' do + it 'changes target branch from a note' do + write_note("message start \n/target_branch merge-test\n message end.") + + wait_for_requests + expect(page).not_to have_content('/target_branch') + expect(page).to have_content('message start') + expect(page).to have_content('message end.') + + expect(merge_request.reload.target_branch).to eq 'merge-test' + end + + it 'does not fail when target branch does not exists' do + write_note('/target_branch totally_not_existing_branch') + + expect(page).not_to have_content('/target_branch') + + expect(merge_request.target_branch).to eq 'feature' + end + end + + context 'when current user can not change target branch' do + before do + project.add_guest(guest) + sign_out(:user) + sign_in(guest) + visit project_merge_request_path(project, merge_request) + end + + it 'does not change target branch' do + write_note('/target_branch merge-test') + + expect(page).not_to have_content '/target_branch merge-test' + + expect(merge_request.target_branch).to eq 'feature' + end + end + end + end +end |