gitlab-ce/spec/models/diff_note_spec.rb

# frozen_string_literal: true

require 'spec_helper'

RSpec.describe DiffNote do
  include RepoHelpers

  let_it_be(:merge_request) { create(:merge_request) }
  let_it_be(:project) { merge_request.project }
  let_it_be(:commit) { project.commit(sample_commit.id) }

  let_it_be(:path) { "files/ruby/popen.rb" }

  let(:diff_refs) { merge_request.diff_refs }
  let!(:position) do
    Gitlab::Diff::Position.new(
      old_path: path,
      new_path: path,
      old_line: nil,
      new_line: 14,
      diff_refs: diff_refs
    )
  end

  let!(:new_position) do
    Gitlab::Diff::Position.new(
      old_path: path,
      new_path: path,
      old_line: 16,
      new_line: 22,
      diff_refs: diff_refs
    )
  end

  subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }

  describe 'validations' do
    it_behaves_like 'a valid diff positionable note' do
      subject { build(:diff_note_on_commit, project: project, commit_id: commit_id, position: position) }
    end

    it "is not valid when noteable is empty" do
      note = build(:diff_note_on_merge_request, project: project, noteable: nil)

      note.valid?

      expect(note.errors[:noteable]).to include("doesn't support new-style diff notes")
    end

    context 'when importing' do
      it "does not check if it's supported" do
        note = build(:diff_note_on_merge_request, project: project, noteable: nil)
        note.importing = true
        note.valid?

        expect(note.errors.full_messages).not_to include(
          "Noteable doesn't support new-style diff notes"
        )
      end
    end
  end

  describe "#position=" do
    context "when provided a string" do
      it "sets the position" do
        subject.position = new_position.to_json

        expect(subject.position).to eq(new_position)
      end
    end

    context "when provided a hash" do
      it "sets the position" do
        subject.position = new_position.to_h

        expect(subject.position).to eq(new_position)
      end
    end

    context "when provided a position object" do
      it "sets the position" do
        subject.position = new_position

        expect(subject.position).to eq(new_position)
      end
    end
  end

  describe "#original_position=" do
    context "when provided a string" do
      it "sets the original position" do
        subject.original_position = new_position.to_json

        expect(subject.original_position).to eq(new_position)
      end
    end

    context "when provided a hash" do
      it "sets the original position" do
        subject.original_position = new_position.to_h

        expect(subject.original_position).to eq(new_position)
      end
    end

    context "when provided a position object" do
      it "sets the original position" do
        subject.original_position = new_position

        expect(subject.original_position).to eq(new_position)
      end
    end
  end

  describe '#create_diff_file callback' do
    context 'merge request' do
      let(: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: merge_request.diff_refs
        )
      end

      subject { build(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }

      let(:diff_file_from_repository) do
        position.diff_file(project.repository)
      end

      let(:diff_file) do
        diffs = merge_request.diffs
        raw_diff = diffs.diffable.raw_diffs(diffs.diff_options.merge(paths: ['files/ruby/popen.rb'])).first
        Gitlab::Diff::File.new(
          raw_diff,
          repository: diffs.project.repository,
          diff_refs: diffs.diff_refs,
          fallback_diff_refs: diffs.fallback_diff_refs
        )
      end

      let(:diff_line) { diff_file.diff_lines.first }

      let(:line_code) { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14' }

      before do
        allow(subject.position).to receive(:line_code).and_return('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14')
      end

      context 'when diffs are already created' do
        before do
          allow(subject).to receive(:created_at_diff?).and_return(true)
        end

        context 'when diff_file is found in persisted diffs' do
          before do
            allow(merge_request).to receive_message_chain(:diffs, :diff_files, :first).and_return(diff_file)
          end

          context 'when importing' do
            before do
              subject.importing = true
              subject.line_code = line_code
            end

            context 'when diff_line is found in persisted diff_file' do
              before do
                allow(diff_file).to receive(:line_for_position).with(position).and_return(diff_line)
              end

              it 'creates a diff note file' do
                subject.save!
                expect(subject.note_diff_file).to be_present
              end
            end

            context 'when diff_line is not found in persisted diff_file' do
              before do
                allow(diff_file).to receive(:line_for_position).and_return(nil)
              end

              it_behaves_like 'a valid diff note with after commit callback'
            end
          end

          context 'when not importing' do
            context 'when diff_line is not found' do
              before do
                allow(diff_file).to receive(:line_for_position).with(position).and_return(nil)
              end

              it 'raises an error' do
                expect { subject.save! }.to raise_error(
                  ::DiffNote::NoteDiffFileCreationError,
                  "Failed to find diff line for: #{diff_file.file_path}, "\
                  "old_line: #{position.old_line}"\
                  ", new_line: #{position.new_line}"
                )
              end
            end

            context 'when diff_line is found' do
              before do
                allow(diff_file).to receive(:line_for_position).with(position).and_return(diff_line)
              end

              it 'creates a diff note file' do
                subject.save!
                expect(subject.reload.note_diff_file).to be_present
              end
            end
          end
        end

        context 'when diff file is not found in persisted diffs' do
          before do
            allow_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff) do |merge_request_diff|
              allow(merge_request_diff).to receive(:diff_files).and_return([])
            end
          end

          it_behaves_like 'a valid diff note with after commit callback'
        end
      end

      context 'when diffs are not already created' do
        before do
          allow(subject).to receive(:created_at_diff?).and_return(false)
        end

        it_behaves_like 'a valid diff note with after commit callback'
      end

      it 'does not create diff note file if it is a reply' do
        diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request)

        expect { create(:diff_note_on_merge_request, noteable: merge_request, in_reply_to: diff_note) }
          .not_to change(NoteDiffFile, :count)
      end
    end

    context 'commit' do
      let!(:diff_note) { create(:diff_note_on_commit, project: project) }

      it 'creates a diff note file' do
        expect(diff_note.reload.note_diff_file).to be_present
      end

      it 'does not create diff note file if it is a reply' do
        expect { create(:diff_note_on_commit, in_reply_to: diff_note) }
          .not_to change(NoteDiffFile, :count)
      end
    end
  end

  describe '#diff_file', :clean_gitlab_redis_shared_state do
    context 'when note_diff_file association exists' do
      it 'returns persisted diff file data' do
        diff_file = subject.diff_file

        expect(diff_file.diff.to_hash.with_indifferent_access)
          .to include(subject.note_diff_file.to_hash)
      end
    end

    context 'when the discussion was created in the diff' do
      it 'returns correct diff file' do
        diff_file = subject.diff_file

        expect(diff_file.old_path).to eq(position.old_path)
        expect(diff_file.new_path).to eq(position.new_path)
        expect(diff_file.diff_refs).to eq(position.diff_refs)
      end
    end

    context 'when discussion is outdated or not created in the diff' do
      let(:diff_refs) { project.commit(sample_commit.id).diff_refs }
      let(:position) do
        Gitlab::Diff::Position.new(
          old_path: "files/ruby/popen.rb",
          new_path: "files/ruby/popen.rb",
          old_line: nil,
          new_line: 14,
          diff_refs: diff_refs
        )
      end

      it 'returns the correct diff file' do
        diff_file = subject.diff_file

        expect(diff_file.old_path).to eq(position.old_path)
        expect(diff_file.new_path).to eq(position.new_path)
        expect(diff_file.diff_refs).to eq(position.diff_refs)
      end
    end

    context 'note diff file creation enqueuing' do
      it 'enqueues CreateNoteDiffFileWorker if it is the first note of a discussion' do
        subject.note_diff_file.destroy!

        expect(CreateNoteDiffFileWorker).to receive(:perform_async).with(subject.id)

        subject.reload.diff_file
      end

      it 'does not enqueues CreateNoteDiffFileWorker if not first note of a discussion' do
        mr = create(:merge_request)
        diff_note = create(:diff_note_on_merge_request, project: mr.project, noteable: mr)
        reply_diff_note = create(:diff_note_on_merge_request, in_reply_to: diff_note)

        expect(CreateNoteDiffFileWorker).not_to receive(:perform_async).with(reply_diff_note.id)

        reply_diff_note.reload.diff_file
      end
    end

    context 'when noteable is a Design' do
      it 'does not return a diff file' do
        diff_note = create(:diff_note_on_design)

        expect(diff_note.diff_file).to be_nil
      end
    end
  end

  describe '#latest_diff_file' do
    context 'when noteable is a Design' do
      it 'does not return a diff file' do
        diff_note = create(:diff_note_on_design)

        expect(diff_note.latest_diff_file).to be_nil
      end
    end
  end

  describe "#diff_line" do
    it "returns the correct diff line" do
      diff_line = subject.diff_line

      expect(diff_line.added?).to be true
      expect(diff_line.new_line).to eq(position.formatter.new_line)
      expect(diff_line.text).to eq("+    vars = {")
    end
  end

  describe "#line_code" do
    it "returns the correct line code" do
      line_code = Gitlab::Git.diff_line_code(position.file_path, position.formatter.new_line, 15)

      expect(subject.line_code).to eq(line_code)
    end
  end

  describe "#active?" do
    context "when noteable is a commit" do
      subject { build(:diff_note_on_commit, project: project, position: position) }

      it "returns true" do
        expect(subject.active?).to be true
      end
    end

    context "when noteable is a merge request" do
      context "when the merge request's diff refs match that of the diff note" do
        it "returns true" do
          expect(subject.active?).to be true
        end
      end

      context "when the merge request's diff refs don't match that of the diff note" do
        before do
          allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs)
        end

        it "returns false" do
          expect(subject.active?).to be false
        end
      end
    end
  end

  describe "creation" do
    describe "updating of position" do
      context "when noteable is a commit" do
        let(:diff_refs) { commit.diff_refs }

        subject { create(:diff_note_on_commit, project: project, position: position, commit_id: commit.id) }

        it "doesn't update the position" do
          is_expected.to have_attributes(original_position: position, position: position)
        end
      end

      context "when noteable is a merge request" do
        context "when the note is active" do
          it "doesn't update the position" do
            expect(subject.original_position).to eq(position)
            expect(subject.position).to eq(position)
          end
        end

        context "when the note is outdated" do
          before do
            allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs)
          end

          it "updates the position" do
            expect(subject.original_position).to eq(position)
            expect(subject.position).not_to eq(position)
          end
        end
      end
    end
  end

  describe "#discussion_id" do
    let(:note) { create(:diff_note_on_merge_request) }

    context "when it is newly created" do
      it "has a discussion id" do
        expect(note.discussion_id).not_to be_nil
        expect(note.discussion_id).to match(/\A\h{40}\z/)
      end
    end

    context "when it didn't store a discussion id before" do
      before do
        note.update_column(:discussion_id, nil)
      end

      it "has a discussion id" do
        # The discussion_id is set in `after_initialize`, so `reload` won't work
        reloaded_note = Note.find(note.id)

        expect(reloaded_note.discussion_id).not_to be_nil
        expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/)
      end
    end
  end

  describe '#created_at_diff?' do
    let(:diff_refs) { project.commit(sample_commit.id).diff_refs }
    let(:position) do
      Gitlab::Diff::Position.new(
        old_path: "files/ruby/popen.rb",
        new_path: "files/ruby/popen.rb",
        old_line: nil,
        new_line: 14,
        diff_refs: diff_refs
      )
    end

    context "when noteable is a commit" do
      subject { build(:diff_note_on_commit, project: project, position: position) }

      it "returns true" do
        expect(subject.created_at_diff?(diff_refs)).to be true
      end
    end

    context "when noteable is a merge request" do
      context "when the diff refs match the original one of the diff note" do
        it "returns true" do
          expect(subject.created_at_diff?(diff_refs)).to be true
        end
      end

      context "when the diff refs don't match the original one of the diff note" do
        it "returns false" do
          expect(subject.created_at_diff?(merge_request.diff_refs)).to be false
        end
      end
    end
  end

  describe '#supports_suggestion?' do
    context 'when noteable does not exist' do
      it 'returns false' do
        allow(subject).to receive(:noteable) { nil }

        expect(subject.supports_suggestion?).to be(false)
      end
    end

    context 'when noteable does not support suggestions' do
      it 'returns false' do
        allow(subject.noteable).to receive(:supports_suggestion?) { false }

        expect(subject.supports_suggestion?).to be(false)
      end
    end

    context 'when line is not suggestible' do
      it 'returns false' do
        allow_next_instance_of(Gitlab::Diff::Line) do |instance|
          allow(instance).to receive(:suggestible?) { false }
        end

        expect(subject.supports_suggestion?).to be(false)
      end
    end
  end

  describe '#banzai_render_context' do
    let(:note) { create(:diff_note_on_merge_request) }

    it 'includes expected context' do
      context = note.banzai_render_context(:note)

      expect(context).to include(suggestions_filter_enabled: true, noteable: note.noteable, project: note.project)
    end
  end

  describe "image diff notes" do
    subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) }

    describe "validations" do
      it { is_expected.not_to validate_presence_of(:line_code) }

      it "does not validate diff line" do
        diff_line = subject.diff_line

        expect(diff_line).to be nil
        expect(subject).to be_valid
      end

      it "does not update the position" do
        expect(subject).not_to receive(:update_position)

        subject.save!
      end
    end

    it "returns true for on_image?" do
      expect(subject.on_image?).to be_truthy
    end
  end

  describe '#to_ability_name' do
    subject { described_class.new.to_ability_name }

    it { is_expected.to eq('note') }
  end

  describe '#shas' do
    it 'returns list of SHAs based on original_position' do
      expect(subject.shas).to match_array(
        [
          position.base_sha,
          position.start_sha,
          position.head_sha
        ])
    end

    context 'when position changes' do
      before do
        subject.position = new_position
      end

      it 'includes the new position SHAs' do
        expect(subject.shas).to match_array(
          [
            position.base_sha,
            position.start_sha,
            position.head_sha,
            new_position.base_sha,
            new_position.start_sha,
            new_position.head_sha
          ])
      end
    end
  end
end