Dynamoid / dynamoid

Ruby ORM for Amazon's DynamoDB.
MIT License
580 stars 195 forks source link

Question about support for associations #122

Open mattwelke opened 7 years ago

mattwelke commented 7 years ago

In the readme.md, it says first:

association methods that build new models will not work correctly -- for example, user.addresses.new returns an address that is not associated to the user. We'll be correcting this soon.

But then later it says:

To use associations, you use association methods very similar to ActiveRecord's:


address = u.addresses.create
address.city = 'Chicago'
address.save

I was wondering if anyone could clarify whether or not you can form associations using foreign keys implicitly like this a la ActiveRecord.

Thanks.

mattwelke commented 7 years ago

Tested this myself, and it works great! I had the following two models:

class User
  include Dynamoid::Document

  field :name

  has_many :comments
end

class Comment
  include Dynamoid::Document

  field :body

  belongs_to :user
end

And I was able to create comments in two ways:

I was able to create a user, then create a comment, then associate the comment with the user:

bob = User.create(name: 'Bob')
hello = Comment.create(body: 'Hello.')
bob.comments << hello

But I was also able to do this same association inline:

bob = User.create(name: 'Bob')
hello = bob.comments.create(body: 'Hello.')

I did this test with Rails 5 (with Dynamoid pulled from the master branch of the Git repo) and the local version of DynamoDB. I was going to create a pull request to modify the readme to indicate that this is now working, but then again this is just master where I've confirmed it working, not the published gem. Should we wait for this until the next version is published?

pboling commented 7 years ago

Perhaps you did not notice the subtlety in the Readme. Your comment indicates you tested with .create and as the Readme currently states, this works.

What the Readme says does not work is .new. Did you test that? It may be that the Readme is still correct.

If .new is working(!), go ahead and submit the PR I can release a new patch directly after merging.

mattwelke commented 7 years ago

Ahh that is true. I did misinterpret the readme. .new is not working, only .create is. Sorry about that. :(

I have an idea for how this might be implemented though. I might take a look and try it out! I've never contributed code to an open source project before so I'll read up on how this all works first, then give it a go and submit a pull request if I get it working.

pboling commented 7 years ago

@welkie Also feel free to make the Readme more clear in your PR, if you make one!