ember-model icon indicating copy to clipboard operation
ember-model copied to clipboard

_findFetchById doesn't work when isFetch and isLoaded are false.

Open mikejholly opened this issue 11 years ago • 6 comments

If you look at the code below you can see that adapter is never used. If we call _findFetchById(1, false) we'll only ever see an empty, unloaded instance of the model.

I think the fix would be changing return isFetch ? deferredOrPromise : record; to return isFetch ? deferredOrPromise : adapter.find(record, id);.

Please confirm and I'll create a pull request with the fix.

Thanks!

_findFetchById: function(id, isFetch) {
    var record = this.cachedRecordForId(id),
        isLoaded = get(record, 'isLoaded'),
        adapter = get(this, 'adapter'),
        deferredOrPromise;

    if (isLoaded) {
      if (isFetch) {
        return new Ember.RSVP.Promise(function(resolve, reject) {
          resolve(record);
        });
      } else {
        return record;
      }
    }

    deferredOrPromise = this._fetchById(record, id);

    return isFetch ? deferredOrPromise : record;
  },

mikejholly avatar Jan 14 '14 17:01 mikejholly

That looks right to me. A PR (with a failing test) would be great, thanks!

ebryn avatar Jan 14 '14 18:01 ebryn

Excellent. Will do.

mikejholly avatar Jan 14 '14 18:01 mikejholly

@mikejholly @ebryn is this still open issue? I can do it if you want.

arturmalecki avatar Jan 31 '14 07:01 arturmalecki

@seeweer Looks like this is still an open issue. Would love some help.

ebryn avatar Feb 21 '14 14:02 ebryn

Hi @ebryn

I need a little help with that. First of all how to reproduce this bug:

User.clearCache();
User.find(1).get('name');
# => undefined

But when you try this

User.clearCache();
user = User.find(1);
user.get('name');
# => Foo

And this works. After that I have tried to write test for this:

test("findById should return an object when caches was cleared", function() {
  expect(1);

  var record;

  Model.clearCache();
  record = Ember.run(Model, Model.findById, 'a');
  stop();
  record.on('didLoad', function() {
    start();
    console.log(record.get("name"));
    equal(record.get('name'), 'Erik');
  });
});

and this test pass. This is caused because I assign record to variable and invoke on on it. Side effect is that record load once again. I don't know how to write test for this. Do you have any idea?

And second thing. I have checked also code for _findFechById and I have noticed that there is no handling for case when isLoaded and isFetch is set to false and when cache is empty. I don't know if it is possible to get record from adapter without promise. Method adapter.find returns promise for RESTAdapater and for FixtureAdapter. Is there a way to get(find) a record through adapter that isn't wrapped with promise? Or maybe we should consider to wrap all finder in promises?

arturmalecki avatar Feb 28 '14 13:02 arturmalecki

paging @jnovatnack, could you take a look at this? :)

ebryn avatar Apr 24 '14 04:04 ebryn