Repository navigation
Conversation
Rails 7.1+ CollectionAssociation#merge_target_lists copies reloaded values onto in-memory records with _write_attribute(name, record[name]). The gem's [] returns the mapped class name for polymorphic integer type columns, so the integer column ended up cast to 0 and the in-memory record pointed at the wrong type. Fixes clio#69.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #69.
On Rails 7.1+,
CollectionAssociation#merge_target_listscopies reloaded rows onto records the caller already holds withmem_record._write_attribute(name, record[name]). For polymorphic integer type columns the gem's[]returns the mapped class name, and_write_attributeskips the gem's[]=, so the string goes straight into the integer column and is cast to0. The database row is fine, but the in-memory record now points at whatever type is mapped to0(or nothing).This adds a
_write_attributeoverride next to[]/[]=: when the attribute is one of the model's polymorphic foreign types and the value is a String, it's mapped back through<foreign_type>_mappingbefore callingsuper. Integer values (what the generated setters pass) are untouched, and an unknown string falls through unchanged.Tests:
cat.source_links.new(target: ...), saves them, triggers the reload with.length, and checks the held records keeptarget_type2 / "Drink". It fails on master (got: 0) and passes with the fix.bundle exec rspec: 43 examples, 0 failures on ActiveRecord 8.1.4 and on 7.2.4 (Ruby 4.0.6 locally).I didn't run the 7.0 gemfile locally; that version doesn't go through
_write_attributeinmerge_target_lists, so the override should be a no-op there. Added a CHANGELOG entry under Unreleased and left the version alone.