DEV: Clear custom field preload proxy on preload_custom_fields (#15671)

If a model class calls preload_custom_fields twice then
we have to clear this otherwise the fields are cached inside the
already existing proxy and no new ones are added, so when we check
for custom_fields[KEY] an error is likely to occur

app/models/concerns/has_custom_fields.rb

@@ -64,6 +64,8 @@ module HasCustomFields
     has_many :_custom_fields, dependent: :destroy, class_name: "#{name}CustomField"
     after_save :save_custom_fields
 
+    # TODO (martin) Post 2.8 release, change to attr_reader because this is
+    # set by set_preloaded_custom_fields
     attr_accessor :preloaded_custom_fields
 
     def custom_fields_fk
@@ -114,7 +116,7 @@ module HasCustomFields
 
         objects.each do |obj|
           map[obj.id] = obj
-          obj.preloaded_custom_fields = empty.dup
+          obj.set_preloaded_custom_fields(empty.dup)
         end
 
         fk = (name.underscore << "_id")
@@ -177,6 +179,15 @@ module HasCustomFields
     end
   end
 
+  def set_preloaded_custom_fields(custom_fields)
+    @preloaded_custom_fields = custom_fields
+
+    # we have to clear this otherwise the fields are cached inside the
+    # already existing proxy and no new ones are added, so when we check
+    # for custom_fields[KEY] an error is likely to occur
+    @preloaded_proxy = nil
+  end
+
   def custom_fields
     if @preloaded_custom_fields
       return @preloaded_proxy ||= PreloadedProxy.new(@preloaded_custom_fields, self.class.to_s)

spec/components/concern/has_custom_fields_spec.rb

@@ -31,6 +31,33 @@ describe HasCustomFields do
       Object.send(:remove_const, :CustomFieldsTestItemCustomField)
     end
 
+    it "allows preloading of custom fields" do
+      test_item = CustomFieldsTestItem.new
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["test_field"])
+      expect(test_item.preloaded_custom_fields).to eq({ "test_field" => nil })
+    end
+
+    it "errors if a custom field is not preloaded" do
+      test_item = CustomFieldsTestItem.new
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["test_field"])
+      expect { test_item.custom_fields["other_field"] }.to raise_error(HasCustomFields::NotPreloadedError)
+    end
+
+    it "resets the preloaded_custom_fields if preload_custom_fields is called twice" do
+      test_item = CustomFieldsTestItem.new
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["test_field"])
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["other_field"])
+      expect(test_item.preloaded_custom_fields).to eq({ "other_field" => nil })
+    end
+
+    it "does not error with NotPreloadedError if preload_custom_fields is called twice" do
+      test_item = CustomFieldsTestItem.new
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["test_field"])
+      expect { test_item.custom_fields["test_field"] }.not_to raise_error
+      CustomFieldsTestItem.preload_custom_fields([test_item], ["other_field"])
+      expect { test_item.custom_fields["other_field"] }.not_to raise_error
+    end
+
     it "allows simple modification of custom fields" do
       test_item = CustomFieldsTestItem.new
 
@@ -355,24 +382,24 @@ describe HasCustomFields do
         test_item.custom_fields["bob"] = "marley"
         test_item.custom_fields["jack"] = "black"
 
-         # In memory
+        # In memory
-         expect(test_item.custom_fields[:bob]).to eq('marley')
+        expect(test_item.custom_fields[:bob]).to eq('marley')
-         expect(test_item.custom_fields[:jack]).to eq('black')
+        expect(test_item.custom_fields[:jack]).to eq('black')
 
-         # Persisted
+        # Persisted
-         test_item.save
+        test_item.save
-         test_item.reload
+        test_item.reload
-         expect(test_item.custom_fields[:bob]).to eq('marley')
+        expect(test_item.custom_fields[:bob]).to eq('marley')
-         expect(test_item.custom_fields[:jack]).to eq('black')
+        expect(test_item.custom_fields[:jack]).to eq('black')
 
-         # Update via string index again
+        # Update via string index again
-         test_item.custom_fields['bob'] = 'the builder'
+        test_item.custom_fields['bob'] = 'the builder'
 
-         expect(test_item.custom_fields[:bob]).to eq('the builder')
+        expect(test_item.custom_fields[:bob]).to eq('the builder')
-         test_item.save
+        test_item.save
-         test_item.reload
+        test_item.reload
 
-         expect(test_item.custom_fields[:bob]).to eq('the builder')
+        expect(test_item.custom_fields[:bob]).to eq('the builder')
       end
     end
   end