Commit 4ee3412a authored by Kassio Borges's avatar Kassio Borges

Gitlab Importer - Skip some validations in Project/Group import

Some Project/Group imports are not importing all the relations due to
some *subrelations* (nested relations) being invalid. To avoid that, and
fix exceptions like
https://gitlab.com/gitlab-org/gitlab/-/issues/285107, every relation
created will, from now on, be validated.

Though, while working on
[enabling](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48179)
this validation, some tests failed, exposing that some validations are
actually blocking some imports.

Validation to check if a _major_ association exists, or follow some kind
of requirements, like "DiffNote validating if the MergeRequest is
associated", doesn't work in the import process because the
relations are created in a _nested_ way. Something like:

```
diff_note = DiffNote.create(diff_note_attributes)
mr = MergeRequest.create(mr_attributes, diff_notes: [diff_note])
```

This commit skip these validations on the importing process, using the
`validates :validation, unless: :importing?` pattern.

Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/293705
parent 1515f833
# frozen_string_literal: true
class LabelPriority < ApplicationRecord
include Importable
belongs_to :project
belongs_to :label
validates :project, :label, :priority, presence: true
validates :label, presence: true, unless: :importing?
validates :project, :priority, presence: true
validates :label_id, uniqueness: { scope: :project_id }
validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
end
......@@ -30,14 +30,6 @@ class ResourceEvent < ApplicationRecord
return true if issuable_count == 1
# if none of issuable IDs is set, check explicitly if nested issuable
# object is set, this is used during project import
if issuable_count == 0 && importing?
issuable_count = self.class.issuable_attrs.count { |attr| self.public_send(attr) } # rubocop:disable GitlabSecurity/PublicSend
return true if issuable_count == 1
end
errors.add(
:base, _("Exactly one of %{attributes} is required") %
{ attributes: self.class.issuable_attrs.join(', ') }
......
......@@ -12,7 +12,7 @@ class ResourceLabelEvent < ResourceEvent
scope :inc_relations, -> { includes(:label, :user) }
validates :label, presence: { unless: :importing? }, on: :create
validate :exactly_one_issuable
validate :exactly_one_issuable, unless: :importing?
after_save :expire_etag_cache
after_destroy :expire_etag_cache
......
# frozen_string_literal: true
class SentryIssue < ApplicationRecord
include Importable
belongs_to :issue
validates :issue, uniqueness: true, presence: true
validates :issue, uniqueness: true
validates :issue, presence: true, unless: :importing?
validates :sentry_issue_identifier, presence: true
validate :ensure_sentry_issue_identifier_is_unique_per_project
......
# frozen_string_literal: true
class Suggestion < ApplicationRecord
include Importable
include Suggestible
belongs_to :note, inverse_of: :suggestions
validates :note, presence: true
validates :note, presence: true, unless: :importing?
validates :commit_id, presence: true, if: :applied?
delegate :position, :noteable, to: :note
......
# frozen_string_literal: true
class Timelog < ApplicationRecord
include Importable
validates :time_spent, :user, presence: true
validate :issuable_id_is_present
validate :issuable_id_is_present, unless: :importing?
belongs_to :issue, touch: true
belongs_to :merge_request, touch: true
......
# frozen_string_literal: true
class ZoomMeeting < ApplicationRecord
include Importable
include UsageStatistics
belongs_to :project, optional: false
belongs_to :issue, optional: false
belongs_to :project
belongs_to :issue
validates :project, presence: true, unless: :importing?
validates :issue, presence: true, unless: :importing?
validates :url, presence: true, length: { maximum: 255 }, zoom_url: true
validates :issue, same_project_association: true
validates :issue, same_project_association: true, unless: :importing?
enum issue_status: {
added: 1,
......
......@@ -18,5 +18,11 @@ RSpec.describe LabelPriority do
expect(subject).to validate_uniqueness_of(:label_id).scoped_to(:project_id)
end
describe 'when importing' do
subject { create(:label_priority, importing: true) }
it { is_expected.not_to validate_presence_of(:label) }
end
end
end
......@@ -27,6 +27,12 @@ RSpec.describe SentryIssue do
expect(duplicate_sentry_issue).to be_invalid
expect(duplicate_sentry_issue.errors.full_messages.first).to include('is already associated')
end
describe 'when importing' do
subject { create(:sentry_issue, importing: true) }
it { is_expected.not_to validate_presence_of(:issue) }
end
end
describe 'callbacks' do
......
......@@ -12,6 +12,12 @@ RSpec.describe Suggestion do
describe 'validations' do
it { is_expected.to validate_presence_of(:note) }
context 'when importing' do
subject { create(:suggestion, importing: true) }
it { is_expected.not_to validate_presence_of(:note) }
end
context 'when suggestion is applied' do
before do
allow(subject).to receive(:applied?).and_return(true)
......
......@@ -40,6 +40,14 @@ RSpec.describe Timelog do
expect(subject).to be_valid
end
describe 'when importing' do
it 'is valid if issue_id and merge_request_id are missing' do
subject.attributes = { issue: nil, merge_request: nil, importing: true }
expect(subject).to be_valid
end
end
end
describe 'scopes' do
......
......@@ -12,8 +12,8 @@ RSpec.describe ZoomMeeting do
end
describe 'Associations' do
it { is_expected.to belong_to(:project).required }
it { is_expected.to belong_to(:issue).required }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:issue) }
end
describe 'scopes' do
......@@ -40,6 +40,16 @@ RSpec.describe ZoomMeeting do
end
describe 'Validations' do
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:issue) }
describe 'when importing' do
subject { build(:zoom_meeting, importing: true) }
it { is_expected.not_to validate_presence_of(:project) }
it { is_expected.not_to validate_presence_of(:issue) }
end
describe 'url' do
it { is_expected.to validate_presence_of(:url) }
it { is_expected.to validate_length_of(:url).is_at_most(255) }
......
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