diffs_controller_spec.rb 19.6 KB
Newer Older
1 2
# frozen_string_literal: true

3 4
require 'spec_helper'

5
RSpec.describe Projects::MergeRequests::DiffsController do
6
  include ProjectForksHelper
7
  include TrackingHelpers
8

9 10 11
  shared_examples '404 for unexistent diffable' do
    context 'when diffable does not exists' do
      it 'returns 404' do
12
        go(diff_id: non_existing_record_id)
13

14
        expect(MergeRequestDiff.find_by(id: non_existing_record_id)).to be_nil
15
        expect(response).to have_gitlab_http_status(:not_found)
16 17
      end
    end
18 19 20 21 22 23 24 25 26 27 28 29

    context 'when the merge_request_diff.id is blank' do
      it 'returns 404' do
        allow_next_instance_of(MergeRequest) do |instance|
          allow(instance).to receive(:merge_request_diff).and_return(MergeRequestDiff.new(merge_request_id: instance.id))

          go

          expect(response).to have_gitlab_http_status(:not_found)
        end
      end
    end
30 31
  end

32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51
  shared_examples 'forked project with submodules' do
    render_views

    let(:project) { create(:project, :repository) }
    let(:forked_project) { fork_project_with_submodules(project) }
    let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) }

    before do
      project.add_developer(user)

      merge_request.reload
      go
    end

    it 'renders' do
      expect(response).to be_successful
      expect(response.body).to have_content('Subproject commit')
    end
  end

52 53 54 55 56 57 58 59 60 61
  shared_examples 'cached diff collection' do
    it 'ensures diff highlighting cache writing' do
      expect_next_instance_of(Gitlab::Diff::HighlightCache) do |cache|
        expect(cache).to receive(:write_if_empty).once
      end

      go
    end
  end

62 63 64 65 66 67 68 69 70 71 72
  shared_examples "diff note on-demand position creation" do
    it "updates diff discussion positions" do
      service = double("service")

      expect(Discussions::CaptureDiffNotePositionsService).to receive(:new).with(merge_request).and_return(service)
      expect(service).to receive(:execute)

      go
    end
  end

73 74 75 76 77 78 79 80 81 82 83 84 85 86
  shared_examples 'show the right diff files with previous diff_id' do
    context 'with previous diff_id' do
      let!(:merge_request_diff_1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
      let!(:merge_request_diff_2) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e', diff_type: :merge_head) }

      subject { go(diff_id: merge_request_diff_1.id, diff_head: true) }

      it 'shows the right diff files' do
        subject
        expect(json_response["diff_files"].size).to eq(merge_request_diff_1.files_count)
      end
    end
  end

87
  let(:project) { create(:project, :repository) }
88
  let(:user) { create(:user) }
89
  let(:maintainer) { true }
90 91 92
  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }

  before do
93
    project.add_maintainer(user) if maintainer
94 95 96 97 98 99 100 101 102 103 104 105
    sign_in(user)
  end

  describe 'GET show' do
    def go(extra_params = {})
      params = {
        namespace_id: project.namespace.to_param,
        project_id: project,
        id: merge_request.iid,
        format: 'json'
      }

blackst0ne's avatar
blackst0ne committed
106
      get :show, params: params.merge(extra_params)
107 108 109 110 111
    end

    context 'with default params' do
      context 'for the same project' do
        before do
Felipe Artur's avatar
Felipe Artur committed
112
          allow(controller).to receive(:rendered_for_merge_request?).and_return(true)
113 114
        end

Felipe Artur's avatar
Felipe Artur committed
115
        it 'serializes merge request diff collection' do
116 117 118
          expect_next_instance_of(DiffsSerializer) do |instance|
            expect(instance).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash))
          end
Felipe Artur's avatar
Felipe Artur committed
119 120

          go
121 122 123
        end
      end

124
      context 'when note is a legacy diff note' do
125
        before do
126
          create(:legacy_diff_note_on_merge_request, project: project, noteable: merge_request)
127 128 129
        end

        it 'serializes merge request diff collection' do
130 131 132
          expect_next_instance_of(DiffsSerializer) do |instance|
            expect(instance).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash))
          end
133 134 135 136 137

          go
        end
      end

138
      it_behaves_like 'forked project with submodules'
139 140
    end

141
    it_behaves_like 'cached diff collection'
142
    it_behaves_like 'diff note on-demand position creation'
143 144
  end

145
  describe 'GET diffs_metadata' do
146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163
    shared_examples_for 'serializes diffs metadata with expected arguments' do
      it 'returns success' do
        subject

        expect(response).to have_gitlab_http_status(:ok)
      end

      it 'serializes paginated merge request diff collection' do
        expect_next_instance_of(DiffsMetadataSerializer) do |instance|
          expect(instance).to receive(:represent)
            .with(an_instance_of(collection), expected_options)
            .and_call_original
        end

        subject
      end
    end

164 165 166 167 168 169 170 171 172 173 174
    def go(extra_params = {})
      params = {
        namespace_id: project.namespace.to_param,
        project_id: project,
        id: merge_request.iid,
        format: 'json'
      }

      get :diffs_metadata, params: params.merge(extra_params)
    end

175 176
    it_behaves_like '404 for unexistent diffable'

177 178
    it_behaves_like 'show the right diff files with previous diff_id'

179 180 181 182 183 184 185 186 187 188
    context 'when not authorized' do
      let(:another_user) { create(:user) }

      before do
        sign_in(another_user)
      end

      it 'returns 404 when not a member' do
        go

189
        expect(response).to have_gitlab_http_status(:not_found)
190 191 192 193 194 195 196
      end

      it 'returns 404 when visibility level is not enough' do
        project.add_guest(another_user)

        go

197
        expect(response).to have_gitlab_http_status(:not_found)
198 199 200 201
      end
    end

    context 'with valid diff_id' do
202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219
      subject { go(diff_id: merge_request.merge_request_diff.id) }

      it_behaves_like 'serializes diffs metadata with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
        let(:expected_options) do
          {
            environment: nil,
            merge_request: merge_request,
            merge_request_diff: merge_request.merge_request_diff,
            merge_request_diffs: merge_request.merge_request_diffs,
            start_version: nil,
            start_sha: nil,
            commit: nil,
            latest_diff: true,
            only_context_commits: false,
            allow_tree_conflicts: true,
            merge_ref_head_diff: false
          }
220 221 222 223
        end
      end
    end

224 225 226 227 228 229 230 231 232 233
    context "with the :default_merge_ref_for_diffs flag on" do
      let(:diffable_merge_ref) { true }

      subject do
        go(diff_head: true,
           diff_id: merge_request.merge_request_diff.id,
           start_sha: merge_request.merge_request_diff.start_commit_sha)
      end

      it "correctly generates the right diff between versions" do
234
        MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author).execute(merge_request)
235 236 237 238 239 240 241 242 243 244 245 246

        expect_next_instance_of(CompareService) do |service|
          expect(service).to receive(:execute).with(
            project,
            merge_request.merge_request_diff.head_commit_sha,
            straight: true)
        end

        subject
      end
    end

247 248 249 250 251 252 253 254 255 256
    context 'with diff_head param passed' do
      before do
        allow(merge_request).to receive(:diffable_merge_ref?)
          .and_return(diffable_merge_ref)
      end

      context 'the merge request can be compared with head' do
        let(:diffable_merge_ref) { true }

        it 'compares diffs with the head' do
257
          create(:merge_request_diff, :merge_head, merge_request: merge_request)
258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275

          go(diff_head: true)

          expect(response).to have_gitlab_http_status(:ok)
        end
      end

      context 'the merge request cannot be compared with head' do
        let(:diffable_merge_ref) { false }

        it 'compares diffs with the base' do
          go(diff_head: true)

          expect(response).to have_gitlab_http_status(:ok)
        end
      end
    end

276
    context 'with MR regular diff params' do
277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295
      subject { go }

      it_behaves_like 'serializes diffs metadata with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
        let(:expected_options) do
          {
            environment: nil,
            merge_request: merge_request,
            merge_request_diff: merge_request.merge_request_diff,
            merge_request_diffs: merge_request.merge_request_diffs,
            start_version: nil,
            start_sha: nil,
            commit: nil,
            latest_diff: true,
            only_context_commits: false,
            allow_tree_conflicts: true,
            merge_ref_head_diff: nil
          }
        end
296
      end
297
    end
298

299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317
    context 'with commit param' do
      subject { go(commit_id: merge_request.diff_head_sha) }

      it_behaves_like 'serializes diffs metadata with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::Commit }
        let(:expected_options) do
          {
            environment: nil,
            merge_request: merge_request,
            merge_request_diff: nil,
            merge_request_diffs: merge_request.merge_request_diffs,
            start_version: nil,
            start_sha: nil,
            commit: merge_request.diff_head_commit,
            latest_diff: nil,
            only_context_commits: false,
            allow_tree_conflicts: true,
            merge_ref_head_diff: nil
          }
318 319 320 321
        end
      end
    end

322 323
    context 'when display_merge_conflicts_in_diff is disabled' do
      subject { go }
324

325 326
      before do
        stub_feature_flags(display_merge_conflicts_in_diff: false)
327 328
      end

329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344
      it_behaves_like 'serializes diffs metadata with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
        let(:expected_options) do
          {
            environment: nil,
            merge_request: merge_request,
            merge_request_diff: merge_request.merge_request_diff,
            merge_request_diffs: merge_request.merge_request_diffs,
            start_version: nil,
            start_sha: nil,
            commit: nil,
            latest_diff: true,
            only_context_commits: false,
            allow_tree_conflicts: false,
            merge_ref_head_diff: nil
          }
345 346 347 348 349
        end
      end
    end
  end

350 351 352 353 354 355 356 357 358
  describe 'GET diff_for_path' do
    def diff_for_path(extra_params = {})
      params = {
        namespace_id: project.namespace.to_param,
        project_id: project,
        id: merge_request.iid,
        format: 'json'
      }

blackst0ne's avatar
blackst0ne committed
359
      get :diff_for_path, params: params.merge(extra_params)
360 361 362 363 364 365 366 367 368 369 370 371
    end

    let(:existing_path) { 'files/ruby/popen.rb' }

    context 'when the merge request exists' do
      context 'when the user can view the merge request' do
        context 'when the path exists in the diff' do
          it 'enables diff notes' do
            diff_for_path(old_path: existing_path, new_path: existing_path)

            expect(assigns(:diff_notes_disabled)).to be_falsey
            expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'MergeRequest',
372 373
                                                        noteable_id: merge_request.id,
                                                        commit_id: nil)
374 375 376 377
          end

          it 'only renders the diffs for the path given' do
            diff_for_path(old_path: existing_path, new_path: existing_path)
Felipe Artur's avatar
Felipe Artur committed
378

379
            paths = json_response['diff_files'].map { |file| file['new_path'] }
Felipe Artur's avatar
Felipe Artur committed
380 381

            expect(paths).to include(existing_path)
382 383 384 385 386
          end
        end
      end

      context 'when the user cannot view the merge request' do
387 388
        let(:maintainer) { false }

389 390 391 392 393
        before do
          diff_for_path(old_path: existing_path, new_path: existing_path)
        end

        it 'returns a 404' do
394
          expect(response).to have_gitlab_http_status(:not_found)
395 396 397 398 399 400 401 402 403 404
        end
      end
    end

    context 'when the merge request does not exist' do
      before do
        diff_for_path(id: merge_request.iid.succ, old_path: existing_path, new_path: existing_path)
      end

      it 'returns a 404' do
405
        expect(response).to have_gitlab_http_status(:not_found)
406 407 408 409
      end
    end

    context 'when the merge request belongs to a different project' do
410
      let(:other_project) { create(:project) }
411 412

      before do
413
        other_project.add_maintainer(user)
414 415 416 417
        diff_for_path(old_path: existing_path, new_path: existing_path, project_id: other_project)
      end

      it 'returns a 404' do
418
        expect(response).to have_gitlab_http_status(:not_found)
419 420 421
      end
    end
  end
422 423

  describe 'GET diffs_batch' do
424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439
    shared_examples_for 'serializes diffs with expected arguments' do
      it 'serializes paginated merge request diff collection' do
        expect_next_instance_of(PaginatedDiffSerializer) do |instance|
          expect(instance).to receive(:represent)
            .with(an_instance_of(collection), expected_options)
            .and_call_original
        end

        subject
      end
    end

    shared_examples_for 'successful request' do
      it 'returns success' do
        subject

440
        expect(response).to have_gitlab_http_status(:ok)
441
      end
442 443 444 445 446 447 448 449 450 451 452

      it 'tracks mr_diffs event' do
        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
          .to receive(:track_mr_diffs_action)
          .with(merge_request: merge_request)

        subject
      end

      context 'when DNT is enabled' do
        before do
453
          stub_do_not_track('1')
454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492
        end

        it 'does not track any mr_diffs event' do
          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
            .not_to receive(:track_mr_diffs_action)

          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
            .not_to receive(:track_mr_diffs_single_file_action)

          subject
        end
      end

      context 'when user has view_diffs_file_by_file set to false' do
        before do
          user.update!(view_diffs_file_by_file: false)
        end

        it 'does not track single_file_diffs events' do
          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
            .not_to receive(:track_mr_diffs_single_file_action)

          subject
        end
      end

      context 'when user has view_diffs_file_by_file set to true' do
        before do
          user.update!(view_diffs_file_by_file: true)
        end

        it 'tracks single_file_diffs events' do
          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
            .to receive(:track_mr_diffs_single_file_action)
            .with(merge_request: merge_request, user: user)

          subject
        end
      end
493 494 495 496
    end

    def collection_arguments(pagination_data = {})
      {
497
        environment: nil,
498
        merge_request: merge_request,
499
        commit: nil,
500
        diff_view: :inline,
501
        merge_ref_head_diff: nil,
502
        allow_tree_conflicts: true,
503 504 505 506 507 508
        pagination_data: {
          total_pages: nil
        }.merge(pagination_data)
      }
    end

509 510 511 512 513
    def go(extra_params = {})
      params = {
        namespace_id: project.namespace.to_param,
        project_id: project,
        id: merge_request.iid,
514
        page: 0,
515
        per_page: 20,
516 517 518 519 520 521
        format: 'json'
      }

      get :diffs_batch, params: params.merge(extra_params)
    end

522 523
    it_behaves_like '404 for unexistent diffable'

524 525
    it_behaves_like 'show the right diff files with previous diff_id'

526 527 528 529 530 531 532 533 534 535
    context 'when not authorized' do
      let(:other_user) { create(:user) }

      before do
        sign_in(other_user)
      end

      it 'returns 404' do
        go

536
        expect(response).to have_gitlab_http_status(:not_found)
537 538 539
      end
    end

540 541 542 543 544
    context 'with valid diff_id' do
      subject { go(diff_id: merge_request.merge_request_diff.id) }

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
David Kim's avatar
David Kim committed
545
        let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_ref_head_diff: false) }
546 547
      end

548 549
      it_behaves_like 'successful request'
    end
550

551 552 553 554 555
    context 'with commit_id param' do
      subject { go(commit_id: merge_request.diff_head_sha) }

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::Commit }
556
        let(:expected_options) { collection_arguments.merge(commit: merge_request.commits(limit: 1).first) }
557 558 559
      end
    end

560 561 562 563
    context 'with diff_id and start_sha params' do
      subject do
        go(diff_id: merge_request.merge_request_diff.id,
           start_sha: merge_request.merge_request_diff.start_commit_sha)
564 565
      end

566 567
      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::Compare }
568
        let(:expected_options) { collection_arguments.merge(merge_ref_head_diff: false) }
569 570 571 572
      end

      it_behaves_like 'successful request'
    end
573

574 575 576 577 578 579 580 581 582 583 584
    context 'with paths param' do
      let(:example_file_path) { "README" }
      let(:file_path_option) { { paths: [example_file_path] } }

      subject do
        go(file_path_option)
      end

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
        let(:expected_options) do
David Kim's avatar
David Kim committed
585
          collection_arguments(total_pages: 20)
586 587 588 589 590 591 592 593 594 595 596 597 598
        end
      end

      it_behaves_like 'successful request'

      it 'filters down the response to the expected file path' do
        subject

        expect(json_response["diff_files"].size).to eq(1)
        expect(json_response["diff_files"].first["file_path"]).to eq(example_file_path)
      end
    end

599 600 601 602 603
    context 'with default params' do
      subject { go }

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
David Kim's avatar
David Kim committed
604
        let(:expected_options) { collection_arguments(total_pages: 20) }
605 606 607 608 609 610
      end

      it_behaves_like 'successful request'
    end

    context 'with smaller diff batch params' do
611
      subject { go(page: 5, per_page: 5) }
612 613 614

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
David Kim's avatar
David Kim committed
615
        let(:expected_options) { collection_arguments(total_pages: 20) }
616
      end
617 618

      it_behaves_like 'successful request'
619 620
    end

621 622 623 624 625 626 627 628 629 630 631 632 633 634 635
    context 'when display_merge_conflicts_in_diff is disabled' do
      before do
        stub_feature_flags(display_merge_conflicts_in_diff: false)
      end

      subject { go }

      it_behaves_like 'serializes diffs with expected arguments' do
        let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
        let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) }
      end

      it_behaves_like 'successful request'
    end

636
    it_behaves_like 'forked project with submodules'
637
    it_behaves_like 'cached diff collection'
638 639 640 641 642 643 644 645 646 647 648 649 650 651 652 653 654 655 656 657 658

    context 'diff unfolding' do
      let!(:unfoldable_diff_note) do
        create(:diff_note_on_merge_request, :folded_position, project: project, noteable: merge_request)
      end

      let!(:diff_note) do
        create(:diff_note_on_merge_request, project: project, noteable: merge_request)
      end

      it 'unfolds correct diff file positions' do
        expect_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiffBatch) do |instance|
          expect(instance)
            .to receive(:unfold_diff_files)
            .with([unfoldable_diff_note.position])
            .and_call_original
        end

        go
      end
    end
659
  end
660
end