Merge branch 'pl-rubocop-inline-association-notes' into 'master'

Fix cop FactoryBot/InlineAssociation offenses for notes

See merge request gitlab-org/gitlab!52923

.rubocop_manual_todo.yml

@@ -25,7 +25,6 @@ FactoryBot/InlineAssociation:
     - 'spec/factories/go_modules.rb'
     - 'spec/factories/group_group_links.rb'
     - 'spec/factories/import_export_uploads.rb'
-    - 'spec/factories/notes.rb'
     - 'spec/factories/uploads.rb'
     - 'spec/factories/wiki_pages.rb'
 

spec/factories/notes.rb

@@ -8,7 +8,7 @@ FactoryBot.define do
   factory :note do
     project
     note { generate(:title) }
-    author { project&.creator || create(:user) }
+    author { project&.creator || association(:user) }
     on_issue
 
     factory :note_on_commit,             traits: [:on_commit]
@@ -55,7 +55,7 @@ FactoryBot.define do
       end
 
       position do
-        build(:text_diff_position,
+        association(:text_diff_position,
               file: "files/ruby/popen.rb",
               old_line: nil,
               new_line: line_number,
@@ -64,7 +64,7 @@ FactoryBot.define do
 
       trait :folded_position do
         position do
-          build(:text_diff_position,
+          association(:text_diff_position,
                 file: "files/ruby/popen.rb",
                 old_line: 1,
                 new_line: 1,
@@ -74,7 +74,7 @@ FactoryBot.define do
 
       factory :image_diff_note_on_merge_request do
         position do
-          build(:image_diff_position,
+          association(:image_diff_position,
                 file: "files/images/any_image.png",
                 diff_refs: diff_refs)
         end
@@ -90,7 +90,7 @@ FactoryBot.define do
       end
 
       position do
-        build(:text_diff_position,
+        association(:text_diff_position,
           file: "files/ruby/popen.rb",
           old_line: nil,
           new_line: line_number,
@@ -100,7 +100,11 @@ FactoryBot.define do
     end
 
     factory :diff_note_on_design, parent: :note, traits: [:on_design], class: 'DiffNote' do
-      position { build(:image_diff_position, file: noteable.full_path, diff_refs: noteable.diff_refs) }
+      position do
+        association(:image_diff_position,
+              file: noteable.full_path,
+              diff_refs: noteable.diff_refs)
+      end
     end
 
     trait :on_commit do

spec/models/note_spec.rb

@@ -890,35 +890,31 @@ RSpec.describe Note do
   describe '#cache_markdown_field' do
     let(:html) { '<p>some html</p>'}
 
+    before do
+      allow(Banzai::Renderer).to receive(:cacheless_render_field).and_call_original
+    end
+
     context 'note for a project snippet' do
       let(:snippet) { create(:project_snippet) }
-      let(:note) { build(:note_on_project_snippet, project: snippet.project, noteable: snippet) }
+      let(:note) { create(:note_on_project_snippet, project: snippet.project, noteable: snippet) }
 
-      before do
+      it 'skips project check' do
         expect(Banzai::Renderer).to receive(:cacheless_render_field)
-          .with(note, :note, { skip_project_check: false }).and_return(html)
+          .with(note, :note, { skip_project_check: false })
 
-        note.save
-      end
-
-      it 'creates a note' do
-        expect(note.note_html).to eq(html)
+        note.update!(note: html)
       end
     end
 
     context 'note for a personal snippet' do
       let(:snippet) { create(:personal_snippet) }
-      let(:note) { build(:note_on_personal_snippet, noteable: snippet) }
+      let(:note) { create(:note_on_personal_snippet, noteable: snippet) }
 
-      before do
+      it 'does not skip project check' do
         expect(Banzai::Renderer).to receive(:cacheless_render_field)
-          .with(note, :note, { skip_project_check: true }).and_return(html)
-
-        note.save
-      end
+          .with(note, :note, { skip_project_check: true })
 
-      it 'creates a note' do
-        expect(note.note_html).to eq(html)
+        note.update!(note: html)
       end
     end
   end

spec/support/factory_bot.rb

@@ -3,3 +3,16 @@
 FactoryBot::SyntaxRunner.class_eval do
   include RSpec::Mocks::ExampleMethods
 end
+
+# Patching FactoryBot to allow stubbing non AR models
+# See https://github.com/thoughtbot/factory_bot/pull/1466
+module Gitlab
+  module FactoryBotStubPatch
+    def has_settable_id?(result_instance)
+      result_instance.class.respond_to?(:primary_key) &&
+        result_instance.class.primary_key
+    end
+  end
+end
+
+FactoryBot::Strategy::Stub.prepend(Gitlab::FactoryBotStubPatch)