Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Change in hasMany with sideloading #1316

Closed
gerry3 opened this Issue Sep 16, 2013 · 8 comments

Comments

Projects
None yet
3 participants
Contributor

gerry3 commented Sep 16, 2013

We are continuing our upgrade from 0.13 to // Last commit: 8e8ed7e (2013-09-15 17:24:13 -0700). This is the last issue our thorough Capybara specs are finding.

Models:

App.Post = DS.Model.extend({ comments: DS.hasMany('comment') });
App.Comment = DS.Model.extend({ post: DS.belongsTo('post') });

When we post a comment, we also send back the post and all its comments for sideloading e.g.
{"comment":{"id":247,"post_id":14},"posts":[{"id":14,"comment_ids":[247]}],"comments":[{"id":247,"post_id":14}]}

This still works fine except the client also kicks off an additional request for the comments e.g. /comments?ids[]=247

This is redundant and also we don't support that route in our API.

As a temporary workaround, we will just add the route.

This sounds like what I posted about in: #1312

Contributor

gerry3 commented Sep 16, 2013

Thanks @abobwhite, but the serializeHasMany workaround did not work for us.

@gerry3 The workaround seems prevent the GET for the hasMany models but it's only for those set with:

comments: DS.hasMany('comment', {async:true})

You may want to look into the same area to see what is causing that GET to fire for your situation.

Contributor

gerry3 commented Sep 16, 2013

Thanks @abobwhite. Just setting async: true worked for us. We did not need the serializeHasMany workaround (serializeHasMany wasn't being called at all).

Owner

wycats commented Sep 18, 2013

Hm. You shouldn't need { async: true } if you're sideloading. I bet this is some ordering quirk. Can you try to track down what is triggering the second request (or make a JSBin that demonstrates it)?

@wycats wycats closed this in d445d77 Sep 21, 2013

Owner

wycats commented Sep 21, 2013

I tested the JSBin and it seems to work after the fix.

Contributor

gerry3 commented Sep 22, 2013

@wycats thanks. great talk yesterday (Friday) btw.

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