From 1e69b167b8cdc09002c164dd445c92e52c51ad9b Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Mon, 9 Nov 2020 11:01:43 +0100 Subject: [PATCH 1/2] Ensure that object is present via BatchLoader --- .../resolvers/design_management/design_at_version_resolver.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/graphql/resolvers/design_management/design_at_version_resolver.rb b/app/graphql/resolvers/design_management/design_at_version_resolver.rb index b0da43abd547d6..1b69efebe4e93c 100644 --- a/app/graphql/resolvers/design_management/design_at_version_resolver.rb +++ b/app/graphql/resolvers/design_management/design_at_version_resolver.rb @@ -38,7 +38,7 @@ def self.single # that the DesignAtVersion as found by its ID does in fact belong # to this issue. def consistent?(dav) - issue.nil? || (dav&.design&.issue_id == issue.id) + issue.nil? || (dav.present? && dav.design&.issue_id == issue.id) end def issue -- GitLab From b47d0a6b56133433f2ee984ecc09a0a41f38e99a Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Wed, 21 Oct 2020 14:33:03 +0200 Subject: [PATCH 2/2] Fix cop FactoryBot/InlineAssociation for designs_management factories This commit changes to validate a issue object for a design version rather than issue_id. --- .rubocop_manual_todo.yml | 3 --- app/models/design_management/version.rb | 5 +---- spec/factories/design_management/design_at_version.rb | 6 +++--- spec/factories/design_management/designs.rb | 4 ++-- spec/factories/design_management/versions.rb | 4 ++-- spec/models/design_management/design_at_version_spec.rb | 2 +- spec/models/design_management/version_spec.rb | 2 +- 7 files changed, 10 insertions(+), 16 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index a236073c089508..cf8d25d2476f77 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -6,9 +6,6 @@ FactoryBot/InlineAssociation: - 'ee/spec/factories/merge_request_blocks.rb' - 'ee/spec/factories/vulnerabilities/feedback.rb' - 'spec/factories/atlassian_identities.rb' - - 'spec/factories/design_management/design_at_version.rb' - - 'spec/factories/design_management/designs.rb' - - 'spec/factories/design_management/versions.rb' - 'spec/factories/events.rb' - 'spec/factories/git_wiki_commit_details.rb' - 'spec/factories/gitaly/commit.rb' diff --git a/app/models/design_management/version.rb b/app/models/design_management/version.rb index 55c9084caf274f..49aec8b972043c 100644 --- a/app/models/design_management/version.rb +++ b/app/models/design_management/version.rb @@ -43,10 +43,7 @@ def sentry_extra_data validates :sha, presence: true validates :sha, uniqueness: { case_sensitive: false, scope: :issue_id } validates :author, presence: true - # We are not validating the issue object as it incurs an extra query to fetch - # the record from the DB. Instead, we rely on the foreign key constraint to - # ensure referential integrity. - validates :issue_id, presence: true, unless: :importing? + validates :issue, presence: true, unless: :importing? sha_attribute :sha diff --git a/spec/factories/design_management/design_at_version.rb b/spec/factories/design_management/design_at_version.rb index b73df71595c372..3d85269ee27ac9 100644 --- a/spec/factories/design_management/design_at_version.rb +++ b/spec/factories/design_management/design_at_version.rb @@ -9,13 +9,13 @@ version { nil } transient do - issue { design&.issue || version&.issue || create(:issue) } + issue { design&.issue || version&.issue || association(:issue) } end initialize_with do attrs = attributes.dup - attrs[:design] ||= create(:design, issue: issue) - attrs[:version] ||= create(:design_version, issue: issue) + attrs[:design] ||= association(:design, issue: issue) + attrs[:version] ||= association(:design_version, issue: issue) new(attrs) end diff --git a/spec/factories/design_management/designs.rb b/spec/factories/design_management/designs.rb index 38d0545483c129..c58763791cc207 100644 --- a/spec/factories/design_management/designs.rb +++ b/spec/factories/design_management/designs.rb @@ -2,8 +2,8 @@ FactoryBot.define do factory :design, class: 'DesignManagement::Design' do - issue { create(:issue) } - project { issue&.project || create(:project) } + issue { association(:issue) } + project { issue&.project || association(:project) } sequence(:filename) { |n| "homescreen-#{n}.jpg" } transient do diff --git a/spec/factories/design_management/versions.rb b/spec/factories/design_management/versions.rb index a5c0e7076e914d..0233a3b567d6fb 100644 --- a/spec/factories/design_management/versions.rb +++ b/spec/factories/design_management/versions.rb @@ -3,8 +3,8 @@ FactoryBot.define do factory :design_version, class: 'DesignManagement::Version' do sha - issue { designs.first&.issue || create(:issue) } - author { issue&.author || create(:user) } + issue { designs.first&.issue || association(:issue) } + author { issue&.author || association(:user) } transient do designs_count { 1 } diff --git a/spec/models/design_management/design_at_version_spec.rb b/spec/models/design_management/design_at_version_spec.rb index 220de80a52aa89..a7cf6a9652bf0a 100644 --- a/spec/models/design_management/design_at_version_spec.rb +++ b/spec/models/design_management/design_at_version_spec.rb @@ -185,7 +185,7 @@ end describe 'validations' do - subject(:design_at_version) { build(:design_at_version) } + subject(:design_at_version) { build_stubbed(:design_at_version) } it { is_expected.to be_valid } diff --git a/spec/models/design_management/version_spec.rb b/spec/models/design_management/version_spec.rb index cd52f4129dca88..e004ad024bc432 100644 --- a/spec/models/design_management/version_spec.rb +++ b/spec/models/design_management/version_spec.rb @@ -31,7 +31,7 @@ it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:sha) } it { is_expected.to validate_presence_of(:designs) } - it { is_expected.to validate_presence_of(:issue_id) } + it { is_expected.to validate_presence_of(:issue) } it { is_expected.to validate_uniqueness_of(:sha).scoped_to(:issue_id).case_insensitive } end -- GitLab