diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 4a37dfe5c19ffc3d46bb71a053635266ca8b60a4..42f9c0522a3417f3052bfb27c36834f57df3a342 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -31,6 +31,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = { merge_request: @merge_request, + diff_view: diff_view, pagination_data: diffs.pagination_data } @@ -60,7 +61,9 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic render: ->(partial, locals) { view_to_html_string(partial, locals) } } - render json: DiffsSerializer.new(request).represent(@diffs, additional_attributes) + options = additional_attributes.merge(diff_view: diff_view) + + render json: DiffsSerializer.new(request).represent(@diffs, options) end def define_diff_vars diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 2a5121a22662043340b79e10d0545b1cc450e8ce..af7d1172f1761512b8596aec1cd7bd2fddc2e4cd 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -53,7 +53,7 @@ class DiffFileEntity < DiffFileBaseEntity end # Used for inline diffs - expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, _) { diff_file.text? } do |diff_file| + expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, options) { inline_diff_view?(options) && diff_file.text? } do |diff_file| diff_file.diff_lines_for_serializer end @@ -62,5 +62,21 @@ class DiffFileEntity < DiffFileBaseEntity end # Used for parallel diffs - expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, _) { diff_file.text? } + expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, options) { parallel_diff_view?(options) && diff_file.text? } + + private + + def parallel_diff_view?(options) + return true unless Feature.enabled?(:single_mr_diff_view) + + # If we're not rendering inline, we must be rendering parallel + !inline_diff_view?(options) + end + + def inline_diff_view?(options) + return true unless Feature.enabled?(:single_mr_diff_view) + + # If nothing is present, inline will be the default. + options.fetch(:diff_view, :inline).to_sym == :inline + end end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index e09f4345308cd3a6a8403338fae4cdef77e21a06..2e8c401b8efad3a3eab1ef222038e6e5150d8c98 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -5,6 +5,10 @@ module ErrorTracking DEFAULT_ISSUE_STATUS = 'unresolved' DEFAULT_LIMIT = 20 + def external_url + project_error_tracking_setting&.sentry_external_url + end + private def fetch @@ -15,10 +19,6 @@ module ErrorTracking { issues: response[:issues] } end - def external_url - project_error_tracking_setting&.sentry_external_url - end - def issue_status params[:issue_status] || DEFAULT_ISSUE_STATUS end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 5c02e8d6461b4a8e6430ddf12009a54733e1562e..4f6a7de5ae7b5f849d9feb183c5b05e3eb462e27 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -34,6 +34,16 @@ describe Projects::MergeRequests::DiffsController do it 'saves the preferred diff view in a cookie' do expect(response.cookies['diff_view']).to eq('parallel') end + + it 'only renders the required view', :aggregate_failures do + diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] } + have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file| + !diff_file.has_key?('highlighted_diff_lines') + end + + expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines')) + expect(diff_files_without_deletions).to all(have_no_inline_diff_lines) + end end context 'when the user cannot view the merge request' do @@ -259,7 +269,7 @@ describe Projects::MergeRequests::DiffsController do it 'only renders the diffs for the path given' do diff_for_path(old_path: existing_path, new_path: existing_path) - paths = json_response["diff_files"].map { |file| file['new_path'] } + paths = json_response['diff_files'].map { |file| file['new_path'] } expect(paths).to include(existing_path) end @@ -344,6 +354,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 1, next_page: nil, @@ -367,6 +378,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 2, next_page: 3, diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index fa53682b024abbcd54494274fb77297b798c46fa..4e161d530d3f6e85479a9a2c32dce0ead15522a7 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -20,7 +20,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js end before do - stub_feature_flags(web_ide_default: false) + stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) target_project.add_maintainer(user) sign_in(user) @@ -32,6 +32,8 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js wait_for_requests end + it_behaves_like 'rendering a single diff version' + it 'mentions commits will go to the source branch' do expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') end diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index 19b8a7f74b73d3b025c33a77f7249df96d252623..6a23b6cdf60f7b102ebac52a23b20c8c88846019 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -13,12 +13,15 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end + it_behaves_like 'rendering a single diff version' + context 'when viewing comments' do context 'when toggling inline comments' do context 'in a single file' do diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index e0724a04ea34814c93da76989b456c149842715a..e6634a8ff39ae2cceb4db94c3e352c9ba17db308 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -9,6 +9,7 @@ describe 'Merge request > User creates image diff notes', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) sign_in(user) # Stub helper to return any blob file as image from public app folder. @@ -17,6 +18,8 @@ describe 'Merge request > User creates image diff notes', :js do allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.png') end + it_behaves_like 'rendering a single diff version' + context 'create commit diff notes' do commit_id = '2f63565e7aac07bcdadb654e253078b727143ec4' diff --git a/spec/features/merge_request/user_expands_diff_spec.rb b/spec/features/merge_request/user_expands_diff_spec.rb index f7317ec5ca7522fed527bfda6d86d1e8a2957c75..ba7abd3af2ca91139072eba7e593bc058525ce76 100644 --- a/spec/features/merge_request/user_expands_diff_spec.rb +++ b/spec/features/merge_request/user_expands_diff_spec.rb @@ -7,6 +7,8 @@ describe 'User expands diff', :js do let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) } before do + stub_feature_flags(single_mr_diff_view: false) + allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes) allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes) @@ -15,6 +17,8 @@ describe 'User expands diff', :js do wait_for_requests end + it_behaves_like 'rendering a single diff version' + it 'allows user to expand diff' do page.within find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do click_link 'Click to expand it.' diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 8b16760606ceadf5bd559d1198a7cff2bc2ac75f..6328c0a51335c2264fae748d50b1d08a2dab9d37 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -14,12 +14,15 @@ describe 'Merge request > User posts diff notes', :js do let(:test_note_comment) { 'this is a test note!' } before do + stub_feature_flags(single_mr_diff_view: false) set_cookie('sidebar_collapsed', 'true') project.add_developer(user) sign_in(user) end + it_behaves_like 'rendering a single diff version' + context 'when hovering over a parallel view diff file' do before do visit diffs_project_merge_request_path(project, merge_request, view: 'parallel') diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index e3ee80a47d7ecc209d4fbb31e14984c26230cd5e..f0949fefa3b25a393a0d66a01ad958c278e8d9db 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -9,6 +9,7 @@ describe 'Merge request > User resolves conflicts', :js do 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) + stub_feature_flags(single_mr_diff_view: false) end def create_merge_request(source_branch) @@ -17,7 +18,9 @@ describe 'Merge request > User resolves conflicts', :js do end end - shared_examples "conflicts are resolved in Interactive mode" do + it_behaves_like 'rendering a single diff version' + + 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' diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 8b41ef86791bbd6edd6b95deb5717f83c9d4be91..7cb46d90092747a3a141503c91fba87649d2b95d 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -19,6 +19,12 @@ describe 'Merge request > User resolves diff notes and threads', :js do ) end + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it_behaves_like 'rendering a single diff version' + context 'no threads' do before do project.add_maintainer(user) diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index baef831c40e4f25ffd638a94e9627b89004d370d..e882b4011227bbcfee2ee3378f1795481ba6d88d 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -7,8 +7,8 @@ describe 'Merge request > User sees avatars on diff notes', :js do 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(: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, @@ -21,12 +21,15 @@ describe 'Merge request > User sees avatars on diff notes', :js do let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in user set_cookie('sidebar_collapsed', 'true') end + it_behaves_like 'rendering a single diff version' + context 'discussion tab' do before do visit project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index d9783f3388c7232f6197f73f67fd457efb617cfc..82dd779577cddc875181abf6e0e43d5873e3b6ed 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -9,6 +9,12 @@ describe 'Merge request > User sees diff', :js do let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it_behaves_like 'rendering a single diff version' + context 'when linking to note' do describe 'with unresolved note' do let(:note) { create :diff_note_on_merge_request, project: project, noteable: merge_request } diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb index db0d632cdf20e36289d233a318b83867b97aa057..3d25611e1ea6bf8921f09d61d096b05b366b7a61 100644 --- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -11,11 +11,14 @@ describe 'Merge request > User sees MR with deleted source branch', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) merge_request.update!(source_branch: 'this-branch-does-not-exist') sign_in(user) visit project_merge_request_path(project, merge_request) end + it_behaves_like 'rendering a single diff version' + it 'shows a message about missing source branch' do expect(page).to have_content('Source branch does not exist.') end diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index 62abcff7bdafae4dcfd2e901d82d7748c322149e..c3fce9761df7956c51c172c8230b5a3b20f4c0f1 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -16,11 +16,15 @@ describe 'Merge request > User sees versions', :js do let!(:params) { {} } before do + stub_feature_flags(single_mr_diff_view: false) + project.add_maintainer(user) sign_in(user) visit diffs_project_merge_request_path(project, merge_request, params) end + it_behaves_like 'rendering a single diff version' + shared_examples 'allows commenting' do |file_id:, line_code:, comment:| it do diff_file_selector = ".diff-file[id='#{file_id}']" diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index 3d26ff3ed94aca935c6fb3800b1d9abaa5f6e9bd..e2bcdfd1e2bfe2f1864c605625dd323a31f76cc9 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -25,12 +25,15 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end + it_behaves_like 'rendering a single diff version' + context 'single suggestion note' do it 'hides suggestion popover' do click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb index 4db067a4e41c9fc3f34b040d34896df54a741808..5e59bc87e68b84e82fa1e3eea3e1693a1cfa2123 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -8,6 +8,7 @@ describe 'Merge request > User toggles whitespace changes', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit diffs_project_merge_request_path(project, merge_request) @@ -15,6 +16,8 @@ describe 'Merge request > User toggles whitespace changes', :js do find('.js-show-diff-settings').click end + it_behaves_like 'rendering a single diff version' + it 'has a button to toggle whitespace changes' do expect(page).to have_content 'Show whitespace changes' end diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index 2d1eb260236521ec1f18686ca7c85b6440f9ed7a..5a29477e597b02a1d11f654d478708f338c86f4a 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -9,6 +9,7 @@ describe 'User views diffs', :js do let(:project) { create(:project, :public, :repository) } before do + stub_feature_flags(single_mr_diff_view: false) visit(diffs_project_merge_request_path(project, merge_request)) wait_for_requests @@ -16,6 +17,8 @@ describe 'User views diffs', :js do find('.js-toggle-tree-list').click end + it_behaves_like 'rendering a single diff version' + shared_examples 'unfold diffs' do it 'unfolds diffs upwards' do first('.js-unfold').click diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index e02a45790951b77909400f617e2194a832dc690b..b81f52a541ba768b680e685b8e8eac58138582c9 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -12,9 +12,11 @@ describe 'Editing file blob', :js do let(:readme_file_path) { 'README.md' } before do - stub_feature_flags(web_ide_default: false) + stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) end + it_behaves_like 'rendering a single diff version' + context 'as a developer' do let(:user) { create(:user) } let(:role) { :developer } @@ -27,14 +29,14 @@ describe 'Editing file blob', :js do def edit_and_commit(commit_changes: true) wait_for_requests find('.js-edit-blob').click - fill_editor(content: "class NextFeature\\nend\\n") + fill_editor(content: 'class NextFeature\\nend\\n') if commit_changes click_button 'Commit changes' end end - def fill_editor(content: "class NextFeature\\nend\\n") + def fill_editor(content: 'class NextFeature\\nend\\n') wait_for_requests find('#editor') execute_script("ace.edit('editor').setValue('#{content}')") @@ -95,13 +97,13 @@ describe 'Editing file blob', :js do context 'when rendering the preview' do it 'renders content with CommonMark' do visit project_edit_blob_path(project, tree_join(branch, readme_file_path)) - fill_editor(content: "1. one\\n - sublist\\n") + fill_editor(content: '1. one\\n - sublist\\n') click_link 'Preview' wait_for_requests # the above generates two separate lists (not embedded) in CommonMark - expect(page).to have_content("sublist") - expect(page).not_to have_xpath("//ol//li//ul") + expect(page).to have_content('sublist') + expect(page).not_to have_xpath('//ol//li//ul') end end end diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb index beb32104809c84f896c6f08128979b86f60b6a4f..832985f1a301842f929960e1fae12771b29321cc 100644 --- a/spec/features/projects/view_on_env_spec.rb +++ b/spec/features/projects/view_on_env_spec.rb @@ -9,9 +9,13 @@ describe 'View on environment', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) + project.add_maintainer(user) end + it_behaves_like 'rendering a single diff version' + context 'when the branch has a route map' do let(:route_map) do <<-MAP.strip_heredoc @@ -26,7 +30,7 @@ describe 'View on environment', :js do user, start_branch: branch_name, branch_name: branch_name, - commit_message: "Add .gitlab/route-map.yml", + commit_message: 'Add .gitlab/route-map.yml', file_path: '.gitlab/route-map.yml', file_content: route_map ).execute @@ -37,9 +41,9 @@ describe 'View on environment', :js do user, start_branch: branch_name, branch_name: branch_name, - commit_message: "Update feature", + commit_message: 'Update feature', file_path: file_path, - file_content: "# Noop" + file_content: '# Noop' ).execute end diff --git a/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb index 05541972f871977b1d5e33edb3f4865fc41881e4..adb8e138ca7e197433a7252ba792322448ff73ef 100644 --- a/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Auth::LDAP::AuthHash do @@ -91,7 +93,7 @@ describe Gitlab::Auth::LDAP::AuthHash do let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' } before do - raw_info[:uid] = ['JOHN'] + raw_info[:uid] = [+'JOHN'] end it 'enabled the username attribute is lower cased' do diff --git a/spec/lib/gitlab/auth/ldap/person_spec.rb b/spec/lib/gitlab/auth/ldap/person_spec.rb index 1527fe60fb94d360e522a1587ab391856ac9aa7e..985732e69f9627f8720125ea6ae3cc57bc1befcc 100644 --- a/spec/lib/gitlab/auth/ldap/person_spec.rb +++ b/spec/lib/gitlab/auth/ldap/person_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Auth::LDAP::Person do @@ -135,7 +137,7 @@ describe Gitlab::Auth::LDAP::Person do let(:username_attribute) { 'uid' } before do - entry[username_attribute] = 'JOHN' + entry[username_attribute] = +'JOHN' @person = described_class.new(entry, 'ldapmain') end diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb index 0c2e7c1e3ebc71ee4a05175f73b54dff8f9ca6ba..65b62f8aa165fa21cf7680a24216de8015796492 100644 --- a/spec/serializers/diff_file_entity_spec.rb +++ b/spec/serializers/diff_file_entity_spec.rb @@ -11,7 +11,8 @@ describe DiffFileEntity do let(:diff_refs) { commit.diff_refs } let(:diff) { commit.raw_diffs.first } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) } - let(:entity) { described_class.new(diff_file, request: {}) } + let(:options) { {} } + let(:entity) { described_class.new(diff_file, options.reverse_merge(request: {})) } subject { entity.as_json } @@ -23,7 +24,7 @@ describe DiffFileEntity do let(:user) { create(:user) } let(:request) { EntityRequest.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:entity) { described_class.new(diff_file, request: request, merge_request: merge_request) } + let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request)) } let(:exposed_urls) { %i(edit_path view_path context_lines_path) } it_behaves_like 'diff file entity' @@ -49,6 +50,8 @@ describe DiffFileEntity do end context '#parallel_diff_lines' do + let(:options) { { diff_view: :parallel } } + it 'exposes parallel diff lines correctly' do response = subject diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb index fce790f708df7d61841b994862f7823cdbd519ef..5b73bc91478f0a363f2b328eed67c61ce43f68f3 100644 --- a/spec/services/error_tracking/list_issues_service_spec.rb +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -45,4 +45,12 @@ describe ErrorTracking::ListIssuesService do include_examples 'error tracking service unauthorized user' include_examples 'error tracking service disabled' end + + describe '#external_url' do + it 'calls the project setting sentry_external_url' do + expect(error_tracking_setting).to receive(:sentry_external_url).and_return(sentry_url) + + expect(subject.external_url).to eql sentry_url + end + end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 7b3b966bd508976e485b96986209e88160c0ae2e..2bd4750dffab74cbd908e48ae82834de2fbae3c1 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -101,8 +101,12 @@ RSpec.configure do |config| config.after(:example, :js) do |example| # when a test fails, display any messages in the browser's console - if example.exception + # but fail don't add the message if the failure is a pending test that got + # fixed. If we raised the `JSException` the fixed test would be marked as + # failed again. + if example.exception && !example.exception.is_a?(RSpec::Core::Pending::PendingExampleFixedError) console = page.driver.browser.manage.logs.get(:browser)&.reject { |log| log.message =~ JS_CONSOLE_FILTER } + if console.present? message = "Unexpected browser console output:\n" + console.map(&:message).join("\n") raise JSConsoleError, message diff --git a/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb b/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb new file mode 100644 index 0000000000000000000000000000000000000000..80120629a32366d7ff30b161bf3f2e42037f8603 --- /dev/null +++ b/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# This pending test can be removed when `single_mr_diff_view` is enabled by default +# disabling the feature flag above is then not needed anymore. +RSpec.shared_examples 'rendering a single diff version' do |attribute| + pending 'allows editing diff settings single_mr_diff_view is enabled' do + project = create(:project, :repository) + user = project.creator + merge_request = create(:merge_request, source_project: project) + stub_feature_flags(single_mr_diff_view: true) + sign_in(user) + + visit(diffs_project_merge_request_path(project, merge_request)) + + expect(page).to have_selector('.js-show-diff-settings') + end +end diff --git a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb index 96cb71be7374944683b68afd6f3787e0ed00d202..d2c269c597cbfbf508ce83166c1d8e63cdb7288a 100644 --- a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb +++ b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb @@ -31,14 +31,43 @@ shared_examples 'diff file entity' do it 'exposes correct attributes' do expect(subject).to include(:added_lines, :removed_lines, - :context_lines_path, :highlighted_diff_lines, - :parallel_diff_lines) + :context_lines_path) end it 'includes viewer' do expect(subject[:viewer].with_indifferent_access) .to match_schema('entities/diff_viewer') end + + context 'diff files' do + context 'when diff_view is parallel' do + let(:options) { { diff_view: :parallel } } + + it 'contains only the parallel diff lines', :aggregate_failures do + expect(subject).to include(:parallel_diff_lines) + expect(subject).not_to include(:highlighted_diff_lines) + end + end + + context 'when diff_view is parallel' do + let(:options) { { diff_view: :inline } } + + it 'contains only the inline diff lines', :aggregate_failures do + expect(subject).not_to include(:parallel_diff_lines) + expect(subject).to include(:highlighted_diff_lines) + end + end + + context 'when the `single_mr_diff_view` feature is disabled' do + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it 'contains both kinds of diffs' do + expect(subject).to include(:highlighted_diff_lines, :parallel_diff_lines) + end + end + end end shared_examples 'diff file discussion entity' do