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

Implement the populate methods from Mongoose. #137

Closed
6 of 7 tasks
lucianopf opened this issue May 9, 2017 · 26 comments
Closed
6 of 7 tasks

Implement the populate methods from Mongoose. #137

lucianopf opened this issue May 9, 2017 · 26 comments

Comments

@lucianopf
Copy link
Contributor

lucianopf commented May 9, 2017

Reason

Since this is heavily inspired at Mongoose I think we should also implement the populate methods inspired by them.

This is their ref: Official docs

Also that feature makes coding way faster without the need of implementing consecutive queries to get the full result.

Proposal

Since I'm a new contributor I don't understand every aspect of this library yet, so I'll purpose small and incremental changes until we have a very similar structure of mongoose's populate method.

By that I'll list goals here so we can keep track and if anyone else wants to get involved follow up the development.

  • Implement the populate method
  • Attach the method to the Model definition
  • Create tests to test this initial feature
  • Implement the ObjectId + ref options in an attribute
  • Implement a better API, such as Dog.get(1).populate('father')
  • Implement a better API, such as Dog.get(1).populate('father.father')
  • Improve the documentation
@brandongoode
Copy link
Contributor

Sounds great!

Thanks for breaking the change up into smaller parts. It makes it much easier to review and merge.

brandongoode added a commit that referenced this issue May 18, 2017
@chops318
Copy link

This will be a great addition to dynamoose. What's the status on populate, if you'd like help, I'd love to try and implement. Thanks for the work!

@lucianopf
Copy link
Contributor Author

Yay, glad you like the proposal @chops318 ! 😁

Go for it buddy, I'm kindda stuck with a lot of work to be done and I might take longer than expected to finish this feature, so, help is always welcome!! =D

@troyswanson
Copy link
Contributor

Would this eliminate the need for nested queries, e.g.:

Product.get(id, (err, product) => {
  if (!product) { /* no product found */ }
  else {
    Category.get(id, (err, category) => {
      if (!category) { /* no category found */ }
      else {
        product.category = category
      }
    })
  }
})

/ref #120 (comment)

@lucianopf
Copy link
Contributor Author

@troyswanson Yeah yeah, definitely!!

My goal is to make:

Product.get('id').populate('category')

@troyswanson
Copy link
Contributor

AHH! I love it! Thank you!

@chops318
Copy link

That's my intention aswell. I'm pretty much going to model the API as close to mongoose as possible

@brandongoode
Copy link
Contributor

@chops318 Thanks for offering to help. I'm looking forward to your PR.

@brandongoode
Copy link
Contributor

@chops318 Do you know when you plan to have this PR ready. I'm planning to release 0.8 sometime over the next week. I would like to include this in that release if you think it will be ready.

@brandongoode brandongoode mentioned this issue Jun 1, 2017
4 tasks
@chops318
Copy link

chops318 commented Jun 1, 2017

@brandongoode I will be done sometime this weekend. What do ya'll think about using the bson module to generate the ObjectIds? Seeing as that is what mongoose uses, I figure we would follow suit.

@brandongoode
Copy link
Contributor

I would prefer not to populate ObjectIds. With dynamoDB it really a hash/range key pair with the range optional. There may not even be an ID attribute. If someone wants to have keys auto-generated, they should use the default for the key attributes.

@brandongoode brandongoode added this to the v0.8.0 milestone Jun 2, 2017
@lucianopf
Copy link
Contributor Author

I agree with you @brandongoode, but we'll still need the ref to make possible the simpler version of the populate, don't you think?

Such as Product.get('id').populate('category'), cuz the model as defined that the category attribute has a model ref to it called Category

@brandongoode
Copy link
Contributor

@lucianopf Yes. ref is needed to know which model to use.

@brandongoode
Copy link
Contributor

@chops318 If there is anything I can do to help let me know.

@brandongoode brandongoode modified the milestones: v0.9.0, v0.8.0 Jul 2, 2017
@dimzeta
Copy link

dimzeta commented Aug 7, 2017

Is it possible to contribute to this feature? How can I do it?

@lucianopf
Copy link
Contributor Author

lucianopf commented Nov 29, 2017

Hello guys, I'm back! hehehe

After this AWS Reinvent, let's keep this rocking and release that!!

@dimitribocquet and @brandongoode!!

That's already partially implement at the master branch @dimitribocquet as stated here

@brandongoode
Copy link
Contributor

@lucianopf - great to hear from you. This is one of a few items i want to make sure is fully supported in the 0.9 release.

@lucianopf
Copy link
Contributor Author

Leave stuff to be done later screws the mind flow! hahaha
I'm taking soo much time to get used to the general idea again! 😂

It will be done! 💪

@lucianopf
Copy link
Contributor Author

lucianopf commented Jan 16, 2018

I think the last step to get this done is to enhance the docs, right @brandongoode ?
I'm now using dynamoose at a personal project and I'm missing lots of stuff from the docs as well =/
I keep reaching out the tests folder to check for demos and all that stuff =(

@fishcharlie
Copy link
Member

@lucianopf @brandongoode If I have some extra time over the next few weeks I plan on working on the documentation. Anyone maybe up for creating some issues about what should be added/improved (especially @lucianopf maybe since you said you kept having to check the demos and tests)? What specifically should I focus on if I get time?

@lucianopf
Copy link
Contributor Author

Nice @fishcharlie !!
I'll take some time to dig a bit again at the codebase, find out stuff to be documented and create issues so contributors can help at that!! ^^

❤️ open-source!

@fishcharlie
Copy link
Member

@lucianopf Tag me in any issues you create so I get alerted to them. Thanks for your contributions as well!! Really looking forward to this feature. Will need some good documentation tho before I know how to use it haha.

@lucianopf
Copy link
Contributor Author

Hey buddy I haven't forgotten to do that, I'm just in a rush at my other projects and I wasn't able to point stuff for you to document yet =/

I'll try to do that during this weekend! ^^ @fishcharlie

@fishcharlie
Copy link
Member

fishcharlie commented Mar 10, 2018

This can be closed via #250, correct?

@fishcharlie fishcharlie modified the milestones: v0.9.0, v1.0 Jun 13, 2018
@fishcharlie
Copy link
Member

Closing this due to no recent activity. If this is still a problem or you have questions please feel free to comment again and we can try to help further.

@lucianopf
Copy link
Contributor Author

Thanks buddy, and I'm sorry for not replying! 😬

The documentation is way better than I actually thought hehehe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants