Commit 74708609 authored by Kerri Miller's avatar Kerri Miller

Merge branch 'kassio/avoid-invalid-notes-when-importing-a-project' into 'master'

Do not fail import of merge request when unsupported diff note occurs

See merge request gitlab-org/gitlab!48189
parents ef279047 3ae84c12
...@@ -19,7 +19,7 @@ class DiffNote < Note ...@@ -19,7 +19,7 @@ class DiffNote < Note
# EE might have added a type when the module was prepended # EE might have added a type when the module was prepended
validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } } validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } }
validate :positions_complete validate :positions_complete
validate :verify_supported validate :verify_supported, unless: :importing?
before_validation :set_line_code, if: :on_text?, unless: :importing? before_validation :set_line_code, if: :on_text?, unless: :importing?
after_save :keep_around_commits, unless: :importing? after_save :keep_around_commits, unless: :importing?
......
# frozen_string_literal: true # frozen_string_literal: true
class SystemNoteMetadata < ApplicationRecord class SystemNoteMetadata < ApplicationRecord
include Importable
# These notes's action text might contain a reference that is external. # These notes's action text might contain a reference that is external.
# We should always force a deep validation upon references that are found # We should always force a deep validation upon references that are found
# in this note type. # in this note type.
...@@ -23,7 +25,7 @@ class SystemNoteMetadata < ApplicationRecord ...@@ -23,7 +25,7 @@ class SystemNoteMetadata < ApplicationRecord
status alert_issue_added relate unrelate new_alert_added severity status alert_issue_added relate unrelate new_alert_added severity
].freeze ].freeze
validates :note, presence: true validates :note, presence: true, unless: :importing?
validates :action, inclusion: { in: :icon_types }, allow_nil: true validates :action, inclusion: { in: :icon_types }, allow_nil: true
belongs_to :note belongs_to :note
......
---
title: Avoid invalid notes on Project Import
merge_request: 48189
author:
type: fixed
...@@ -46,6 +46,18 @@ RSpec.describe DiffNote do ...@@ -46,6 +46,18 @@ RSpec.describe DiffNote do
expect(note.errors[:noteable]).to include("doesn't support new-style diff notes") expect(note.errors[:noteable]).to include("doesn't support new-style diff notes")
end 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 end
describe "#position=" do describe "#position=" do
......
...@@ -13,7 +13,7 @@ RSpec.describe SystemNoteMetadata do ...@@ -13,7 +13,7 @@ RSpec.describe SystemNoteMetadata do
context 'when action type is invalid' do context 'when action type is invalid' do
subject do subject do
build(:system_note_metadata, note: build(:note), action: 'invalid_type' ) build(:system_note_metadata, note: build(:note), action: 'invalid_type')
end end
it { is_expected.to be_invalid } it { is_expected.to be_invalid }
...@@ -21,7 +21,15 @@ RSpec.describe SystemNoteMetadata do ...@@ -21,7 +21,15 @@ RSpec.describe SystemNoteMetadata do
context 'when action type is valid' do context 'when action type is valid' do
subject do subject do
build(:system_note_metadata, note: build(:note), action: 'merge' ) build(:system_note_metadata, note: build(:note), action: 'merge')
end
it { is_expected.to be_valid }
end
context 'when importing' do
subject do
build(:system_note_metadata, note: nil, action: 'merge', importing: true)
end end
it { is_expected.to be_valid } it { is_expected.to be_valid }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment