-
Notifications
You must be signed in to change notification settings - Fork 0
Fix bug causing polymorphic_name not to be respected with belongs_to associations not using polymorphic_integer_type
#3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f547cd2
46cdaa4
c01f473
5c56db4
592c057
03d89f9
a74a8ea
1ee36f1
957826d
1172e9c
ef12e69
3fc20e8
9200eb4
302a807
498a584
40e2b27
f77fc1e
2164bf3
148a2cc
c9ad377
246e6c9
6f9bfc3
ebb1d76
6253f67
f86162e
5a90993
b53d82c
edbf6c9
a501d33
2bfccb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,25 @@ | ||
| module ActiveRecord | ||
| module Associations | ||
| class BelongsToPolymorphicAssociation < BelongsToAssociation | ||
| private | ||
| module PolymorphicIntegerType | ||
| module BelongsToPolymorphicAssociationExtension | ||
| private | ||
|
|
||
| if Gem::Version.new(ActiveRecord::VERSION::STRING) < Gem::Version.new("6.1") | ||
| def replace_keys(record) | ||
| super | ||
| if Gem::Version.new(ActiveRecord::VERSION::STRING) < Gem::Version.new("6.1") | ||
| def replace_keys(record) | ||
| super | ||
|
|
||
| if reflection.options[:integer_type] || reflection.options[:polymorphic].is_a?(Hash) | ||
| owner[reflection.foreign_type] = record.class.base_class unless record.nil? | ||
| end | ||
| elsif | ||
| def replace_keys(record, force: false) | ||
| super | ||
| end | ||
| else | ||
| def replace_keys(record, force: false) | ||
| super | ||
|
|
||
| if reflection.options[:integer_type] || reflection.options[:polymorphic].is_a?(Hash) | ||
| owner[reflection.foreign_type] = record.class.base_class unless record.nil? | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ActiveRecord::Associations::BelongsToPolymorphicAssociation.prepend(PolymorphicIntegerType::BelongsToPolymorphicAssociationExtension) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to have |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| module PolymorphicIntegerType | ||
| module BelongsToValidOptionsExtension | ||
| def valid_options(options) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this makes sense - we need to be able to determine whether the association uses some polymorphic_integer_type-specific config at the time |
||
| valid = super | ||
| valid << :integer_type if options[:polymorphic] | ||
| valid | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ActiveRecord::Associations::Builder::BelongsTo.singleton_class.prepend(PolymorphicIntegerType::BelongsToValidOptionsExtension) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -179,6 +179,7 @@ | |
| end | ||
| end | ||
| end | ||
|
|
||
| context "When a link is given polymorphic record" do | ||
| let(:link) { Link.create(source: source) } | ||
| let(:source) { cat } | ||
|
|
@@ -193,6 +194,17 @@ | |
| end | ||
| end | ||
|
|
||
| context "When a link is given polymorphic record via a non-integer type association" do | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This spec mimic the behavior we're seeing in the main rma repo and fails on the main branch, but passes on this branch |
||
| let(:link) { Link.create(legacy_source: source) } | ||
| let(:source) { Place.create(name: "Main Street") } | ||
|
|
||
| it "appropriately sets the source_id and source_type to the polymorphic_name" do | ||
| expect(link.legacy_source_id).to eql source.id | ||
| expect(link.legacy_source_type).to eql Place.polymorphic_name | ||
| expect(link.legacy_source).to eql source | ||
| end | ||
| end | ||
|
|
||
| context "When a link is given polymorphic id and type" do | ||
| let(:link) { Link.create(source_id: source.id, source_type: source.class.to_s) } | ||
| let(:source) { cat } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,12 @@ | ||
| class Link < ActiveRecord::Base | ||
| include PolymorphicIntegerType::Extensions | ||
|
|
||
| def self.polymorphic_class_for(name) | ||
| name == 'Country' ? Place : super | ||
| end | ||
|
|
||
| belongs_to :source, polymorphic: true, integer_type: true | ||
| belongs_to :target, polymorphic: true, integer_type: true | ||
|
|
||
| belongs_to :legacy_source, polymorphic: true | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class AddLegacySourceReferenceToLinksTable < ActiveRecord::Migration[5.0] | ||
| def change | ||
| add_reference :links, :legacy_source, polymorphic: true | ||
| end | ||
| end | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| class Place < ActiveRecord::Base | ||
| has_many :source_links, as: :target, inverse_of: :legacy_source, class_name: "Link" | ||
|
|
||
| def self.polymorphic_name | ||
| 'Country' | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clauses here and on L17 are similar to the conditionals added to the
belongs_tohere to preventpolymorphic_integer_typebehavior from leaking into allbelongs_toassociations.