diff options
author | Paul Slaughter <pslaughter@gitlab.com> | 2019-08-02 01:46:45 +0000 |
---|---|---|
committer | Paul Slaughter <pslaughter@gitlab.com> | 2019-08-02 01:46:45 +0000 |
commit | d62227a38b1e43ca4687bd4782f10bab16a866eb (patch) | |
tree | f80145077d4017f1cbe33923cbc658d1d46b7449 | |
parent | 325c321c72520245406c5bdb0c4c3df7f2020d61 (diff) | |
parent | d90865a9607a8fed4ebc9669aad6f0cf939720b5 (diff) | |
download | gitlab-ce-d62227a38b1e43ca4687bd4782f10bab16a866eb.tar.gz |
Merge branch '10646-create-drafts-with-commit-id-ce' into 'master'
CE port of https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/14520
See merge request gitlab-org/gitlab-ce!30284
-rw-r--r-- | app/models/concerns/diff_positionable_note.rb | 8 | ||||
-rw-r--r-- | app/models/diff_note.rb | 7 | ||||
-rw-r--r-- | db/migrate/20190703043358_add_commit_id_to_draft_notes.rb | 11 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/db/schema_spec.rb | 2 | ||||
-rw-r--r-- | spec/javascripts/diffs/mock_data/diff_file.js | 2 | ||||
-rw-r--r-- | spec/javascripts/notes/mock_data.js | 2 | ||||
-rw-r--r-- | spec/models/diff_note_spec.rb | 4 | ||||
-rw-r--r-- | spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb | 52 |
9 files changed, 81 insertions, 8 deletions
diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb index 2d09eff0111..195d9e107c5 100644 --- a/app/models/concerns/diff_positionable_note.rb +++ b/app/models/concerns/diff_positionable_note.rb @@ -10,6 +10,8 @@ module DiffPositionableNote serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize + + validate :diff_refs_match_commit, if: :for_commit? end %i(original_position position change_position).each do |meth| @@ -71,4 +73,10 @@ module DiffPositionableNote self.position = result[:position] end end + + def diff_refs_match_commit + return if self.original_position.diff_refs == commit&.diff_refs + + errors.add(:commit_id, 'does not match the diff refs') + end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index f75c32633b1..861185dc222 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -20,7 +20,6 @@ class DiffNote < Note validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } } validate :positions_complete validate :verify_supported - validate :diff_refs_match_commit, if: :for_commit? before_validation :set_line_code, if: :on_text? after_save :keep_around_commits @@ -154,12 +153,6 @@ class DiffNote < Note errors.add(:position, "is invalid") end - def diff_refs_match_commit - return if self.original_position.diff_refs == self.commit.diff_refs - - errors.add(:commit_id, 'does not match the diff refs') - end - def keep_around_commits shas = [ self.original_position.base_sha, diff --git a/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb b/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb new file mode 100644 index 00000000000..022400ce585 --- /dev/null +++ b/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddCommitIdToDraftNotes < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :draft_notes, :commit_id, :binary + end +end diff --git a/db/schema.rb b/db/schema.rb index 3d229c2353b..709f9ce2541 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1136,6 +1136,7 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do t.text "position" t.text "original_position" t.text "change_position" + t.binary "commit_id" t.index ["author_id"], name: "index_draft_notes_on_author_id" t.index ["discussion_id"], name: "index_draft_notes_on_discussion_id" t.index ["merge_request_id"], name: "index_draft_notes_on_merge_request_id" diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 13fad1b6dc9..232890b1bba 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -27,7 +27,7 @@ describe 'Database schema' do cluster_providers_gcp: %w[gcp_project_id operation_id], deploy_keys_projects: %w[deploy_key_id], deployments: %w[deployable_id environment_id user_id], - draft_notes: %w[discussion_id], + draft_notes: %w[discussion_id commit_id], emails: %w[user_id], events: %w[target_id], epics: %w[updated_by_id last_edited_by_id start_date_sourcing_milestone_id due_date_sourcing_milestone_id], diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js index 27428197c1c..531686efff1 100644 --- a/spec/javascripts/diffs/mock_data/diff_file.js +++ b/spec/javascripts/diffs/mock_data/diff_file.js @@ -1,3 +1,5 @@ +// Copied to ee/spec/frontend/diffs/mock_data/diff_file.js + export default { submodule: false, submodule_link: null, diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 1df5cf9ef68..5f81a168498 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -1,3 +1,5 @@ +// Copied to ee/spec/frontend/notes/mock_data.js + export const notesDataMock = { discussionsPath: '/gitlab-org/gitlab-ce/issues/26/discussions.json', lastFetchedAt: 1501862675, diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index d9e1fe4b165..8d7dafc523d 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -34,6 +34,10 @@ describe DiffNote do 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', :diff_note_on_commit + end + describe "#position=" do context "when provided a string" do it "sets the position" do diff --git a/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb new file mode 100644 index 00000000000..8b298c5c974 --- /dev/null +++ b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +shared_examples_for 'a valid diff positionable note' do |factory_on_commit| + context 'for commit' do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit(sample_commit.id) } + let(:commit_id) { commit.id } + let(:diff_refs) { commit.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 + + subject { build(factory_on_commit, commit_id: commit_id, position: position) } + + context 'position diff refs matches commit diff refs' do + it 'is valid' do + expect(subject).to be_valid + expect(subject.errors).not_to have_key(:commit_id) + end + end + + context 'position diff refs does not match commit diff refs' do + let(:diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: "not_existing_sha", + head_sha: "existing_sha" + ) + end + + it 'is invalid' do + expect(subject).to be_invalid + expect(subject.errors).to have_key(:commit_id) + end + end + + context 'commit does not exist' do + let(:commit_id) { 'non-existing' } + + it 'is invalid' do + expect(subject).to be_invalid + expect(subject.errors).to have_key(:commit_id) + end + end + end +end |