Skip to content

Prevent ActiveRecord from reloading associated records when the STI name is different#27

Merged
doliveirakn merged 2 commits intoclio:masterfrom
mctaylorpants:replace-keys-patch
Dec 18, 2019
Merged

Prevent ActiveRecord from reloading associated records when the STI name is different#27
doliveirakn merged 2 commits intoclio:masterfrom
mctaylorpants:replace-keys-patch

Conversation

@mctaylorpants
Copy link
Copy Markdown
Contributor

@mctaylorpants mctaylorpants commented Dec 18, 2019

⚠️ Depends on #25
⚠️ Depends on #26


When using classes with custom STI names, a bug exists where ActiveRecord will always reload the associated record from the database, even if a valid instance is already loaded.

This occurs because of how we are patching foreign_type=:

define_method "#{foreign_type}=" do |klass|
  mapping = self.class.send("#{foreign_type}_mapping")
  enum = mapping.key(klass.to_s)
  if klass.kind_of?(Class) && klass <= ActiveRecord::Base
    enum ||= mapping.key(klass.sti_name)
    enum ||= mapping.key(klass.base_class.to_s)
    enum ||= mapping.key(klass.base_class.sti_name)
  end
end

We expect that klass will be something that we can look up in the foreign type mapping; if we can't look it up, and it's a constant, then we can attempt to find it using its sti_name.

However, sometimes Rails passes this method a string instead of a constant. When that happens, our lookup doesn't work and we end up returning nil. As a result, the associated record is reloaded because ActiveRecord believes it to be stale.

This issue only exists in Rails < 5.2. Rails 5.2 introduced a .polymorphic_name method that allows the user to customize this as needed.

When a class' name does not match its STI name, ActiveRecord
will reload an associated record from the database even if it's
already in memory. This bug exists in Rails 4 and above.
@doliveirakn doliveirakn merged commit d2b2b31 into clio:master Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants