Skip to content
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

Support relationships between classes with a common module parent #576

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshmenden
Copy link

I came about this problem because I am using Dynamoid in a Rails Engine, and I discovered issues where the relationships between models were not functioning correctly due to the isolate_namespace functionality of engines.

For example, piggybacking off the example in the README, if I had classes like this, but in an engine named MyEngine...

module MyEngine
  class User
    include Dynamoid::Document
  
    # ...
  
    has_many :addresses
  end
end

module MyEngine
  class Address
    include Dynamoid::Document
  
    # ...
  
    belongs_to :user # Automatically links up with the user model
  end
end

I would get the following in a rails console

> u = MyEngine::User.create! 
> address = MyEngine::Address.create(user: u)
> u.addresses.count # gives 0

In other words, the MyEngine::Address ID would not be saved on the MyEngine::User dynamo record.

This is because in this line of code in belongs_to.rb, the MyEngine::Address class would evaluate to :"my_engine/addresses" which of course is incorrect.

My solution is to check if both the target_class and the source_class share the same module_parent, check that that module parent custom to the app, and then use the demodulize functionality to strip the class down to its bare name.

@@ -39,8 +39,14 @@ def associate(hash_key)
#
# @since 0.2.0
def target_association
has_many_key_name = options[:inverse_of] || source.class.to_s.underscore.pluralize.to_sym
Copy link
Author

Choose a reason for hiding this comment

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

In every other file the pluralize comes before the underscore, so I changed this to be consistent

if target_class.module_parent != Object && (target_class.module_parent == source.class.module_parent)
has_many_key_name = source.class.to_s.demodulize.pluralize.underscore.to_sym
has_one_key_name = source.class.to_s.demodulize.underscore.to_sym
end
Copy link
Member

Choose a reason for hiding this comment

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

Can we unconditionally demodulize a class name?

@andrykonchin
Copy link
Member

Could you plz add specs for the fixed behaviour?

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.

None yet

2 participants