lukejagodzinski / meteor-astronomy-relations

https://atmospherejs.com/jagi/astronomy-relations
MIT License
7 stars 5 forks source link

Relations in V2 #6

Closed gabriel-dehan closed 7 years ago

gabriel-dehan commented 8 years ago

Will this package still be working with the upcoming v2 of astronomy ? Do you recommend using it or should I handle my relations myself ?

Thanks

lukejagodzinski commented 8 years ago

I've planned relations module to be integrated with Astronomy 2.0 but I won't be able to finish it for 2.0. I will appear in Astronomy 2.1 so very soon after release of 2.0. There will be no need for additional package to be added.

gabriel-dehan commented 8 years ago

Great! Will the API change a great deal or can I use this package for now and easily upgrade afterward?

lukejagodzinski commented 8 years ago

In Astronomy 2.0 and 1.0 there is not a lot changes in API however the relations module will change a lot. However it will be much simpler than now.

brtdwchtr commented 8 years ago

any (ball park) idea on when we could expect to have a 2.1 with relations, @jagi?

lukejagodzinski commented 8 years ago

For now it's not on my high priority list as it's quite easily to implement by yourself. So I can't give any approximate date.

patekuru commented 8 years ago

Can u please give a clue or link as to how to "quite easily implement it" while waiting for it to be official? Thanks.

lukejagodzinski commented 8 years ago

Just the way you would do it without Astronomy.

methods: {
  getRelatedDocs() {
    return RelatedCollection.find({
      _id: this.relationId
    }).fetch();
  }
}
JulianKingman commented 7 years ago

I'm thinking of creating a package for this, with the following API:

doc.relateTo('ClassName', otherId) Adds the respective ids to the documents.

doc.getRelated('ClassName', otherId) Gets related document, or all related documents if otherId is omitted

For setup, something like:

relations: {
  type: 'many',
  // thinking I can omit this and just assume many-to-many
  key: '_id',
  // default is _id
  class: 'ClassName',
}

Any thoughts, or anything you would add/remove?

JulianKingman commented 7 years ago

Here's my progress: https://github.com/DesignmanIO/meteor-astronomy-relations

One main bug, but it's pretty slick when it works (I got rid of a lot of spagetti code with this).

lukejagodzinski commented 7 years ago

Hmmm I don't quite understand what for are relateTo and getRelated methods and why they take "ClassName" as the first arguments. Could you provide some example with class definition and real relations?

JulianKingman commented 7 years ago

Here's the idea. Let's say we have Author and Post classes. Normally, to create a relationship, you do something like this:

author = Author.findOne();
postId = new Post({author: author._id});
author.posts.push(postId);
authorPosts = Post.find({_id: {$in: author.posts}});
// or
authorPosts = Post.find({author: author._id});

But it gets messy sometimes, like if you forget to push the _id to one collection, it's not a very "safe" way to do it. Instead, consider this:

author = Author.findOne();
postId = new Post();
author.relateTo('Post', postId);
authorPosts = author.getRelated('Post');

Relations are handled behind the scenes.

In other comments you mentioned modules, I'm unfamiliar with how to use them, could you point me to an example?

lukejagodzinski commented 7 years ago

For Astronomy v1 there was the relations module that worked similarly but had different API and it was only intended to retrieve related documents and not to store them. You're API looks ok but I would improve it a little bit.

  1. First of all relation should not be uppercase and not related with a class but a field. author.getRelated('posts') - single author can author of more than one post or post.getRelated('author') - single post can only have one author
  2. Storing relation could be made using ES2016 setters, as long as you don't care about IE (don't remember version)
    const author = Author.findOne();
    author.posts.push(new Post());
    author.save();

    The posts field could be transient field that is not stored in collection. And in the beforeSave event you could check whether there are some new posts in the posts field. If so, then it should save all related posts.

That's how I would implement it. What do you think about that approach?

JulianKingman commented 7 years ago
  1. Since both the field and class names are needed, I'm curious why you would choose the field name, and not the class name? It seems less intuitive.
  2. I like it. If I used getters too, then I could just use author.posts instead of author.getRelated('posts') to get the posts, right?
  3. For the way I'm using it, I'd like to store the _id for both collections in the related collections. It makes things so much easier to be able to search by either id in either collection.
lukejagodzinski commented 7 years ago
  1. class name can be store in the schema, so by knowing field name you can tell what class is related with a given field name
  2. Yes exactly
  3. The way you store id depends on situation and type of relation. I suggest reading this https://docs.mongodb.com/manual/applications/data-models-relationships/ article about relations in MongoDB
laosb commented 7 years ago

Any updates on bringing this back to Astronomy 2.0?

lukejagodzinski commented 7 years ago

No plans to do that anytime soon. I'm a little bit busy with other projects.