Skip to content

Use a dynamic module to include the writing methods that are generated from using belongs_to#24

Merged
doliveirakn merged 2 commits intomasterfrom
prepend-to-prevent-overriding-the-dynamic-method
Dec 9, 2019
Merged

Use a dynamic module to include the writing methods that are generated from using belongs_to#24
doliveirakn merged 2 commits intomasterfrom
prepend-to-prevent-overriding-the-dynamic-method

Conversation

@doliveirakn
Copy link
Copy Markdown
Contributor

Prior to this, if we have something like

class Link < ActiveRecord::Base
  include PolymorphicIntegerType::Extensions

  belongs_to :source, polymorphic: true, integer_type: true
  belongs_to :target, polymorphic: true, integer_type: true

  def source=(val)
    super(val)
  end

  def source_type=(val)
    super(val)
  end
end

the source= will override the method defined by the belongs_to and what will end up happening is that the source_type will not end up being created properly.

By created an anonymous module, we can prepend it to the class and then the super behaviour will work as expected.

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)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is also new.

Previously, we checked klass.to_s, then klass.base_class.to_s, then klass.base_class.sti_name. This ensures that we check klass.sti_name before we move to the base class.

component_extension = Module.new do
define_method foreign_type do
t = super()
self.class.send("#{foreign_type}_mapping")[t]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapping is a a local variable in the context of the belongs_to method. It was captured in the context of the dynamic methods. We have the ability to access the mapping through a class variable, so we can use that method instead of capturing the local variable.

@doliveirakn doliveirakn force-pushed the prepend-to-prevent-overriding-the-dynamic-method branch from 959c681 to 3fdfb3e Compare December 9, 2019 22:01
…errides a method, we can still use the super behaviour
@doliveirakn doliveirakn force-pushed the prepend-to-prevent-overriding-the-dynamic-method branch from 3fdfb3e to 967dbf2 Compare December 9, 2019 23:21
@doliveirakn doliveirakn merged commit 90fad83 into master Dec 9, 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