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

When Many-to-Many Through Associations release? #5291

Closed
cuongcua90 opened this issue Oct 29, 2014 · 32 comments
Closed

When Many-to-Many Through Associations release? #5291

cuongcua90 opened this issue Oct 29, 2014 · 32 comments

Comments

@cuongcua90
Copy link

The docs said it is coming soon. while I wait it is released is the alternative way to attack addition attributes?

@devinivy
Copy link

I am able to get this mostly working (not fully tested). It's a pretty manual job, and finnicky! For now, don't bother with attribute methods or lifecycle callbacks, as they'll get overwritten. I have an issue about that here: #707. Here's an example. I would love for someone to let me know if there's any easier way! In particular, I'm confused by the via attribute on the through table's foreign keys, and something might not be quite right. I suppose a lookup might happen, as the through table never references the other tables' foreign keys; thus, there's no clear connection between the humanFriends and dogFriends attributes. Nonetheless, it's working!

var Dogs= {
        identity: 'dogs',     
        attributes: {
            humanFriends: {
                collection: 'humans',
                through: 'friendships'
            } 
        }
},

var Friendships = {
        identity: 'friendships',
        attributes: {
            someOtherAttr: 'string',
            dog: {
                columnName: 'dog',
                type: 'integer',
                foreignKey: true,
                references: 'dogs',
                on: 'id',
                onKey: 'id',
                via: 'human'
            },
            human: {
                columnName: 'human',
                type: 'integer',
                foreignKey: true,
                references: 'humans',
                on: 'id',
                onKey: 'id',
                via: 'dog'
            }
        }
}

var Humans = {
        identity: 'humans',
        attributes: {
            dogFriends: {
                collection: 'dogs',
                through: 'friendships'
            } 
        }
}

@mewben
Copy link
Contributor

mewben commented Oct 30, 2014

Not working for me... TypeError: Cannot read property of 'via' of undefined :(

@devinivy
Copy link

@mewben, do you get that error right when waterline is initialized / when your sails lift (if you're using sails)? And which version of waterline are you using? I'd love to find a reproducible way to hack through-associations into working correctly!

@mewben
Copy link
Contributor

mewben commented Oct 31, 2014

I'm using sails v0.10.5... sails lifts okay though.. the error triggers when using .add() in the association and then .save() to commit...

@devinivy
Copy link

@mewben using that exact example I gave you above, this is working for me. I'm on waterline 0.10.12 using sails-disk. Perhaps if you follow that error's stack trace, we can figure out if it's simply the version of waterline that you're running.

@mewben
Copy link
Contributor

mewben commented Oct 31, 2014

I'm using the waterline v0.10.9 which sails v0.10.5 is using.. I guess I'll have to wait for sails to bump their waterline dependency then...

@dmarcelino
Copy link
Member

I've tried a similar code with v0.10.12 and it worked for me, thanks @devinivy.

@NiclasLindqvist
Copy link

The issue title did not correspond to the question in the issue body here.. Is there a date for the 'official' associations-attributes implementation? Or is it just the docs that are missing, and @devinivy has the right way to do it?

@devinivy
Copy link

I'm not sure about an official date/release– sadly it doesn't seem to be in the 0.11 milestone: https://github.com/balderdashy/waterline/milestones/0.11 . My method works, but it's undocumented. If you peek through the waterline-schema codebase you'll see some references to the through keyword. I expect when this is officially supported that the model configuration on the junction-collection will be simpler than my example above.

@dmarcelino
Copy link
Member

@NiclasLindqvist, the waterline-adapter-tests give a hint as to what may be the right way to describe many-to-many through associations in the future. Check hasManyThrough.venue.fixture.js (join table) and hasManyThrough.team.fixture.js. This approach works for joins (relevant test) but it seems to fail when creating an association. Using @devinivy's method does work for both creating associations and joins which makes me think that many-to-many through associations will be officially supported soon (fingers crossed).

@kirkaleze
Copy link

@devinivy what statement are you using to populate your join model once humans and dogs are populated? Also, is declaring the variables necessary?

@devinivy
Copy link

@kirkaleze declaring the variables is merely for illustrative purposes. The populate query would then look quite normal:
DogCollection.find().populate('humanFriends').exec(function(err, dogs) {});

But you should also be able to query the junction table:
FriendshipsCollection.find().exec(function(err, friendships) {});

@dmarcelino
Copy link
Member

It would also be good to be able to store data in the join table (association) while saving. Quoting @elennaro (#957):

title: extra fields on many to many relationships join table
It's a widely used approach when we save extra data with relation.
Like quantity for sold products in order-product relation.
Like expiration on user-post-ban. etc. etc.

@devinivy
Copy link

Yes, indeedy! Info on the join table should also be included appear when populating records during a find.

@elennaro
Copy link

Sorry for a noob question, but is there any manual way to overwrite find for humans somehow to get both Dogs and identity field from friendship?

@dmarcelino
Copy link
Member

One of the things I'm really missing is the ability of pulling the join table record for each through association. I was thinking about a possible API for this and I came up with:

// joinRecord or some other keyword to enable this behaviour
Model.findOne(100).populate('assocAttribute', { joinRecord: true })

Would return something like:

{
  id: 100,
  assocAttribute: [
    { id: 2, name: 'blah', createdAt: '04/05/2015', _joinRecord: { id: 10, createdAt: '05/05/2015', joinAttribute: 'something' } }
  ]
}

What do you think about this API? Does anybody knows how other ORMs handle this?

@chrisns
Copy link

chrisns commented May 26, 2015

@dmarcelino that API looks similar to the suggested approach in Rails with the proxy_association ActiveRecord Association Extension (from my very limited Rails understanding).

It would be amazing as part of the extension to the .populate() api to be able to have criteria on the join, e.g. createdAt > 04/04/2015
maybe:

# To get all
Model.findOne(100).populate('assocAttribute', { joinRecord: true })

and

# to get joins made since a date *criteria syntax is probably wrong
Model.findOne(100).populate('assocAttribute', { joinRecord: {where: {createdAt: {'>=': '04/04/2015'}}}})

and further to optimize for large join tables, one could also populate only the fields you were interested in maybe:

# to get only id from join record 
Model.findOne(100).populate('assocAttribute', { joinRecord: { select: ['id']}})
# to return 
{
  id: 100,
  assocAttribute: [
    { id: 2, name: 'blah', createdAt: '04/05/2015', _joinRecord: { id: 10} }
  ]
}

That all said, I'd be extremely happy to get the implementation you suggested for now 😄 👍

@jettwein
Copy link

Anyone have a working example of this that they'd care to share? I can't seem to get this working properly. I'm sure I'm missing something quite simple.

@armellarcier
Copy link
Contributor

+1 Need this feature desperately. Did anyone manually implement it with success?

@ksylvan
Copy link

ksylvan commented Aug 9, 2015

👍 Please.

@nicco
Copy link

nicco commented Aug 26, 2015

I'm using 0.10.26 and all seems to work fine. I have a question thou how can you populate someOtherAttr in Friendship? is there a way to do it from one of the 2 tables part of the relationship or I do need to call explicitly Friendship?

@elennaro
Copy link

Hello.
Any progress here?
Any help needed?

@sailsbot
Copy link

Thanks for posting, @cuongcua90. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy
Copy link

Reopening, as this is in a feature request in an active milestone. We can close once it's PRed into the roadmap.

@devinivy devinivy reopened this Oct 23, 2015
@mukk85
Copy link

mukk85 commented Nov 16, 2015

@devinivy Any ETA on when this will be available? Any patches we can integrate today?

@devinivy
Copy link

@mukk85 this is essentially usable– the configuration for it is just ugly and it's considered experimental. @atiertant might have a patch that you can integrate?

@atiertant
Copy link

@devinivy got some work on it,got something working a little,need more test...
there is an other problem:

  {
    identity: 'Taxi',
    connection: 'foo',
    tableName: 'taxi_table',
    attributes: {
      "taxiId": {
        "type": "integer",
        "primaryKey": true
      },
      "taxiMatricule": {
        "type": "string"
      },
      "drivers": {
        "collection": "Driver",
        "through": "ride",
        "via": "taxi"
      }
    }
  }

  {
    identity: 'Ride',
    connection: 'foo',
    tableName: 'ride_table',
    attributes: {
      "rideId": {
        "type": "integer",
        "primaryKey": true
      },
      "taxi": {
        "model": "Taxi"
      },
      "new_driver": {
        "model": "Driver"
      },
      "last_driver": {
        "model": "Driver"
      },
    }
  }

like this this not possible to define if relation on taxi should use new_driver or last_driver on ride ...
maybe spec could had forward key to specify when this ambiguous like this :

      "drivers": {
        "collection": "Driver",
        "through": "ride",
        "via": "taxi",
        "forward": "new_driver"
      }

@atiertant
Copy link

@particlebanana merges patch to correct many bugs on this assocations type but not this one.
@particlebanana what do you think about adding optional forward key ?

@sailsbot
Copy link

Thanks for posting, @cuongcua90. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@particlebanana
Copy link
Contributor

Re-opening until this is added to the roadmap

@atiertant
Copy link

@particlebanana the real problem is we don't know what must be done... update doc? correct a bug ? add some tests,...just add a line in roadmap to say "something like this should work later"

@particlebanana
Copy link
Contributor

Ok this is on the Roadmap. Waterline is taking the contribution guide from Sails. So basically what needs to happen is the following:

  • First, write out a high-level summary of the feature you are proposing as a concise description (3-5 sentences) focused around a convincing real-world use case where the app you are building or maintaining for your job, your clients, your company, your non-profit work, or your independent hobby project would be made easier by this feature or change.
  • Next, describe in clear prose with relevant links to code files exactly why it would be difficult or impossible to implement the feature without changing Waterline core. If this is not the case, and this feature could be implemented as an adapter, then please reconsider writing your proposal (it is unlikely the core team will be able to accept it).
  • Finally, if you have time, take a first pass at proposing a spec for this feature (its configuration, usage, and how it would be implemented). If you do not have time to write out a first draft of a thorough specification, please make that point in your feature request, and clarify that it would be up to other contributors with the same or a similar use case to finish this proposal.

This has a somewhat defined spec with the forward key and we are transitioning over to the new contribution guide for features so I just added it.

So TLDR: We have a loose spec of how to do M:M through with extra columns and it has been added to the roadmap. It could be scheduled for 0.11 or beyond depending on time constraints.

@raqem raqem transferred this issue from balderdashy/waterline May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests