diff_note_spec.rb 7.55 KB
Newer Older
Douwe Maan's avatar
Douwe Maan committed
1 2 3 4 5
require 'spec_helper'

describe DiffNote, models: true do
  include RepoHelpers

6 7
  let(:merge_request) { create(:merge_request) }
  let(:project) { merge_request.project }
Douwe Maan's avatar
Douwe Maan committed
8 9 10 11
  let(:commit) { project.commit(sample_commit.id) }

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

12
  let!(:position) do
Douwe Maan's avatar
Douwe Maan committed
13 14 15 16 17 18 19 20 21
    Gitlab::Diff::Position.new(
      old_path: path,
      new_path: path,
      old_line: nil,
      new_line: 14,
      diff_refs: merge_request.diff_refs
    )
  end

22
  let!(:new_position) do
Douwe Maan's avatar
Douwe Maan committed
23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59
    Gitlab::Diff::Position.new(
      old_path: path,
      new_path: path,
      old_line: 16,
      new_line: 22,
      diff_refs: merge_request.diff_refs
    )
  end

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

  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

60
  describe "#original_position=" do
Douwe Maan's avatar
Douwe Maan committed
61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83
    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
84 85
  end

Douwe Maan's avatar
Douwe Maan committed
86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131
  describe "#diff_file" do
    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

  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.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::Diff::LineCode.generate(position.file_path, position.new_line, 15)

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

  describe "#for_line?" do
    context "when provided the correct diff line" do
      it "returns true" do
        expect(subject.for_line?(subject.diff_line)).to be true
      end
    end

    context "when provided a different diff line" do
      it "returns false" do
        some_line = subject.diff_file.diff_lines.first

        expect(subject.for_line?(some_line)).to be false
      end
    end
  end

  describe "#active?" do
    context "when noteable is a commit" do
Douwe Maan's avatar
Douwe Maan committed
132
      subject { build(:diff_note_on_commit, project: project, position: position) }
Douwe Maan's avatar
Douwe Maan committed
133 134 135 136 137 138 139 140 141 142 143 144 145 146 147

      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
148
          allow(subject.noteable).to receive(:diff_sha_refs).and_return(commit.diff_refs)
Douwe Maan's avatar
Douwe Maan committed
149 150 151 152 153 154 155 156 157
        end

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

158 159 160 161
  describe "creation" do
    describe "updating of position" do
      context "when noteable is a commit" do
        let(:diff_note) { create(:diff_note_on_commit, project: project, position: position) }
Douwe Maan's avatar
Douwe Maan committed
162 163 164 165

        it "doesn't use the DiffPositionUpdateService" do
          expect(Notes::DiffPositionUpdateService).not_to receive(:new)

166
          diff_note
Douwe Maan's avatar
Douwe Maan committed
167 168 169
        end

        it "doesn't update the position" do
170
          diff_note
Douwe Maan's avatar
Douwe Maan committed
171

172 173
          expect(diff_note.original_position).to eq(position)
          expect(diff_note.position).to eq(position)
Douwe Maan's avatar
Douwe Maan committed
174 175 176
        end
      end

177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192
      context "when noteable is a merge request" do
        let(:diff_note) { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }

        context "when the note is active" do
          it "doesn't use the DiffPositionUpdateService" do
            expect(Notes::DiffPositionUpdateService).not_to receive(:new)

            diff_note
          end

          it "doesn't update the position" do
            diff_note

            expect(diff_note.original_position).to eq(position)
            expect(diff_note.position).to eq(position)
          end
Douwe Maan's avatar
Douwe Maan committed
193 194
        end

195 196
        context "when the note is outdated" do
          before do
197
            allow(merge_request).to receive(:diff_sha_refs).and_return(commit.diff_refs)
198 199 200 201 202 203 204 205 206 207 208 209 210 211 212
          end

          it "uses the DiffPositionUpdateService" do
            service = instance_double("Notes::DiffPositionUpdateService")
            expect(Notes::DiffPositionUpdateService).to receive(:new).with(
              project,
              nil,
              old_diff_refs: position.diff_refs,
              new_diff_refs: commit.diff_refs,
              paths: [path]
            ).and_return(service)
            expect(service).to receive(:execute)

            diff_note
          end
Douwe Maan's avatar
Douwe Maan committed
213 214 215 216
        end
      end
    end
  end
217

218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241
  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
242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276

  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
Douwe Maan's avatar
Douwe Maan committed
277
end