Merge branch 'id-create-diff-note-positions' into 'master'

Create a table for storing diff note positions

See merge request gitlab-org/gitlab!28113

app/models/diff_note_position.rb

+# frozen_string_literal: true
+
+class DiffNotePosition < ApplicationRecord
+  belongs_to :note
+
+  enum diff_content_type: {
+    text: 0,
+    image: 1
+  }
+
+  enum diff_type: {
+    head: 0
+  }
+
+  def position
+    Gitlab::Diff::Position.new(
+      old_path: old_path,
+      new_path: new_path,
+      old_line: old_line,
+      new_line: new_line,
+      position_type: diff_content_type,
+      diff_refs: Gitlab::Diff::DiffRefs.new(
+        base_sha: base_sha,
+        start_sha: start_sha,
+        head_sha: head_sha
+      )
+    )
+  end
+
+  def position=(position)
+    position_attrs = position.to_h
+    position_attrs[:diff_content_type] = position_attrs.delete(:position_type)
+
+    assign_attributes(position_attrs)
+  end
+end

db/migrate/20200326122700_create_diff_note_positions.rb

+# frozen_string_literal: true
+
+class CreateDiffNotePositions < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      create_table :diff_note_positions do |t|
+        t.references :note, foreign_key: { on_delete: :cascade }, null: false, index: false
+        t.integer :old_line
+        t.integer :new_line
+        t.integer :diff_content_type, limit: 2, null: false
+        t.integer :diff_type, limit: 2, null: false
+        t.string :line_code, limit: 255, null: false
+        t.binary :base_sha, null: false
+        t.binary :start_sha, null: false
+        t.binary :head_sha, null: false
+        t.text :old_path, null: false
+        t.text :new_path, null: false
+
+        t.index [:note_id, :diff_type], unique: true
+      end
+    end
+  end
+
+  def down
+    drop_table :diff_note_positions
+  end
+end

db/structure.sql

@@ -2138,6 +2138,30 @@ CREATE SEQUENCE public.design_user_mentions_id_seq
 
 ALTER SEQUENCE public.design_user_mentions_id_seq OWNED BY public.design_user_mentions.id;
 
+CREATE TABLE public.diff_note_positions (
+    id bigint NOT NULL,
+    note_id bigint NOT NULL,
+    old_line integer,
+    new_line integer,
+    diff_content_type smallint NOT NULL,
+    diff_type smallint NOT NULL,
+    line_code character varying(255) NOT NULL,
+    base_sha bytea NOT NULL,
+    start_sha bytea NOT NULL,
+    head_sha bytea NOT NULL,
+    old_path text NOT NULL,
+    new_path text NOT NULL
+);
+
+CREATE SEQUENCE public.diff_note_positions_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.diff_note_positions_id_seq OWNED BY public.diff_note_positions.id;
+
 CREATE TABLE public.draft_notes (
     id bigint NOT NULL,
     merge_request_id integer NOT NULL,
@@ -7124,6 +7148,8 @@ ALTER TABLE ONLY public.design_management_versions ALTER COLUMN id SET DEFAULT n
 
 ALTER TABLE ONLY public.design_user_mentions ALTER COLUMN id SET DEFAULT nextval('public.design_user_mentions_id_seq'::regclass);
 
+ALTER TABLE ONLY public.diff_note_positions ALTER COLUMN id SET DEFAULT nextval('public.diff_note_positions_id_seq'::regclass);
+
 ALTER TABLE ONLY public.draft_notes ALTER COLUMN id SET DEFAULT nextval('public.draft_notes_id_seq'::regclass);
 
 ALTER TABLE ONLY public.emails ALTER COLUMN id SET DEFAULT nextval('public.emails_id_seq'::regclass);
@@ -7829,6 +7855,9 @@ ALTER TABLE ONLY public.design_management_versions
 ALTER TABLE ONLY public.design_user_mentions
     ADD CONSTRAINT design_user_mentions_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.diff_note_positions
+    ADD CONSTRAINT diff_note_positions_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.draft_notes
     ADD CONSTRAINT draft_notes_pkey PRIMARY KEY (id);
 
@@ -9086,6 +9115,8 @@ CREATE UNIQUE INDEX index_design_management_versions_on_sha_and_issue_id ON publ
 
 CREATE UNIQUE INDEX index_design_user_mentions_on_note_id ON public.design_user_mentions USING btree (note_id);
 
+CREATE UNIQUE INDEX index_diff_note_positions_on_note_id_and_diff_type ON public.diff_note_positions USING btree (note_id, diff_type);
+
 CREATE INDEX index_draft_notes_on_author_id ON public.draft_notes USING btree (author_id);
 
 CREATE INDEX index_draft_notes_on_discussion_id ON public.draft_notes USING btree (discussion_id);
@@ -11068,6 +11099,9 @@ ALTER TABLE ONLY public.project_statistics
 ALTER TABLE ONLY public.user_details
     ADD CONSTRAINT fk_rails_12e0b3043d FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.diff_note_positions
+    ADD CONSTRAINT fk_rails_13c7212859 FOREIGN KEY (note_id) REFERENCES public.notes(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.users_security_dashboard_projects
     ADD CONSTRAINT fk_rails_150cd5682c FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -13068,6 +13102,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200325160952
 20200325183636
 20200326114443
+20200326122700
 20200326124443
 20200326134443
 20200326135443

spec/factories/diff_note_positions.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :diff_note_position do
+    association :note, factory: :diff_note_on_merge_request
+    line_code { note.line_code }
+    position { note.position }
+    diff_type { :head }
+  end
+end

spec/models/diff_note_position_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DiffNotePosition, type: :model do
+  it 'has a position attribute' do
+    diff_position = build(:diff_position)
+    line_code = 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521'
+    diff_note_position = build(:diff_note_position, line_code: line_code, position: diff_position)
+
+    expect(diff_note_position.position).to eq(diff_position)
+    expect(diff_note_position.line_code).to eq(line_code)
+    expect(diff_note_position.diff_content_type).to eq('text')
+  end
+
+  it 'unique by note_id and diff type' do
+    existing_diff_note_position = create(:diff_note_position)
+    diff_note_position = build(:diff_note_position, note: existing_diff_note_position.note)
+
+    expect { diff_note_position.save! }.to raise_error(ActiveRecord::RecordNotUnique)
+  end
+end