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

BackroundReload breaks findAll().then() for sorting in route #4148

Closed
adam-knights opened this issue Feb 10, 2016 · 3 comments
Closed

BackroundReload breaks findAll().then() for sorting in route #4148

adam-knights opened this issue Feb 10, 2016 · 3 comments

Comments

@adam-knights
Copy link
Contributor

When upgrading to Ember data 2.1, 2.2 or 2.3.3 I hit an issue in that if I goto a specific models route - say /member/1234 - and then browse to the all of that models route, say /members/ - then rather than showing all 6 members, the page only shows the member already downloaded (1234) and none of the others.

The issue also shows up in Ember data 1.13.15 if I set

  shouldReloadAll(store, snapshotRecordArray) {
    return !snapshotRecordArray.length;
  },
  shouldBackgroundReloadAll() {
    return true;
  },
  shouldReloadRecord() {
    return false;
  },
  shouldBackgroundReloadRecord() {
    return true;
  }

Apart from the standard reloadAll deprecation the below code in the members route shows no other deprecation warnings.

  model: function() {
    return this.store.findAll('member').then(members => {
      let accountPromises = [];
      members.forEach(member => accountPromises.push(this.store.findRecord('account', member.get('id'))));
      return Ember.RSVP.all(accountPromises).then(() => members.sortBy('account.tradingName'));
    });
  },

If I take out the sorting and just return this.store.findAll('member') then everything behaves as expected.

It seems we can no longer rely on a findAll().then() in the way we could before. Presumably because the promise resolves with the single record. Should there have been a deprecation warning? The change in behavior causing this took quite a while to work out.

If I change the code to return this.store.findAll('member', { reload: true }).then then it also works as expected.

Is there a way to utilise the background refresh behavior AND do something like the above when the background refresh completes? i.e. download the next level of the relationship with findRecord and then sort on it?

Last thing - the 2.0 release blog post shows the {reload: true} type very clearly, but the API docs just show type DS.Model for the second param to findAll, which seems unintuitive, should it be DS.Model?

Summary - should there be a deprecation message, and are the api docs incorrect for findAll and findRecord?

@kernel-io
Copy link

I had this bugging me all day! Would be nice to have some more guidance / documentation regarding this :). Although I must say, I wasn't using .then, I was having the issue while using a standard .findAll (in a RSVP.hash) adding reload: true fixed this.

@pangratz
Copy link
Member

pangratz commented May 4, 2016

I am closing this issue since the documentation should have improved now that #4338 has been merged. Please feel free to reopen if you think this is still not documented sufficiently. Thanks!

@adam-knights as to your sorting: I think you should do the sorting in a component / controller using a computed property. By this you can do a background reload and the sorted list is updated once the data is updated.

@pangratz pangratz closed this as completed May 4, 2016
@adam-knights
Copy link
Contributor Author

Agreed - docs look alot better and the sorting on a controller/component makes sense

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

No branches or pull requests

3 participants