search

bendrucker / ama

Ask me questions about building web applications
MIT License
6 stars 1 forks source link

Bookshelf relation() vs. related('relation') #3

Closed bsiddiqui closed 9 years ago

bsiddiqui commented 9 years ago

In describing attach, the Bookshelf docs show two ways to use the method.

// approach 1
new Site({id: 1}).related('admins').attach([1, 2]);

// approach 2
new Site({id: 1}).admins().attach([1, 2]);

Is there a difference in these two approach? Also, is attach supposed to respond with any data?

I looked through the source code and it seems like there is an "attached" event but I haven't seen what kind of response it should be giving - I've been getting an empty array.

bendrucker commented 9 years ago

No real difference. The difference between .related('admins') and.admins()is that the former callssite.admins()and stores the return value assite.relations.admins. The later just returns a new collection with the rightrelatedData` bound.

What do you mean by response for the event?

bsiddiqui commented 9 years ago

Sorry I mean what kind of response should .attach() return?

If I do:

return new Site({id: 1}).admins().attach([1, 2])
.then(function (res) {
   console.log(res); // empty array
});

Maybe I have my models setup incorrectly.

bendrucker commented 9 years ago

Yeah that's the resolution value of triggerThen I believe. It's really not resolving anything intentionally at the moment. What would you expect/want it to resolve?

bsiddiqui commented 9 years ago

I would expect that it would resolve with the model that was created or the collection it was added to

bsiddiqui commented 9 years ago

With the way it currently resolves, how would you recommend check if you successfully created the model and respond with it?

bendrucker commented 9 years ago

What exactly are you looking to get in the end? The Site? The admins with IDs 1 and 2?

bsiddiqui commented 9 years ago

I guess I'd be interested in both since this is hypothetical - how would you do it in either situation?

bendrucker commented 9 years ago

Well I'm asking because it would be sloppy right now with the need to store both the site and adminIds in a variable so you can use them later. I think it makes sense to resolve the collection and I can go commit that if it makes sense.

bsiddiqui commented 9 years ago

Yeah that makes sense :+1:

On Thu, Jan 22, 2015 at 3:38 AM Ben Drucker notifications@github.com wrote:

Well I'm asking because it would be sloppy right now with the need to store both the site and adminIds in a variable so you can use them later. I think it makes sense to resolve the collection and I can go commit that if it makes sense.

— Reply to this email directly or view it on GitHub https://github.com/bendrucker/discuss/issues/3#issuecomment-71008060.