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

Warn when findRecord returns a different id than the one requested #4604

Merged
merged 1 commit into from
Oct 20, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion addon/-private/system/store/finders.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Ember from 'ember';
import { assert } from "ember-data/-private/debug";
import { assert, warn } from "ember-data/-private/debug";
import {
_bind,
_guard,
Expand Down Expand Up @@ -38,6 +38,11 @@ export function _find(adapter, store, typeClass, id, internalModel, options) {
return store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, id, 'findRecord');
assert('Ember Data expected the primary data returned from a `findRecord` response to be an object but instead it found an array.', !Array.isArray(payload.data));

warn(`You requested a record of type '${typeClass.modelName}' with id '${id}' but the adapter returned a payload with primary data having an id of '${payload.data.id}'. Looks like you want to use store.queryRecord instead http://emberjs.com/api/data/classes/DS.Store.html#method_queryRecord`, payload.data.id === id, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried that out of the box store.queryRecord doesn't solve this problem out of the box and the documentation linked isn't doesn't point the user toward an actionable fix for the problem this is warning about.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see your concern 😔 . But I don't see how this issue could be solved other than queryRecord. I will try to update the docs and this warning...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #4609

id: 'ds.store.findRecord.id-mismatch'
});

//TODO Optimize
var record = store.push(payload);
return record._internalModel;
Expand Down
14 changes: 14 additions & 0 deletions tests/integration/adapter/find-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,17 @@ testInDebug('When multiple records are requested, and the payload is blank', (as
});
}, /You made a `findMany` request for person records with ids 1,2, but the adapter's response did not have any data/);
});

testInDebug("warns when returned record has different id", function(assert) {
env.registry.register('adapter:person', DS.Adapter.extend({
findRecord() {
return { id: 1, name: "Braaaahm Dale" };
}
}));

assert.expectWarning(/You requested a record of type 'person' with id 'me' but the adapter returned a payload with primary data having an id of '1'/);

run(function() {
env.store.findRecord('person', 'me');
});
});
2 changes: 1 addition & 1 deletion tests/integration/relationships/belongs-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ test("The store can materialize a non loaded monomorphic belongsTo association",
env.adapter.findRecord = function(store, type, id, snapshot) {
assert.ok(true, "The adapter's find method should be called");
return Ember.RSVP.resolve({
id: 1
id
});
};

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/store/adapter-interop-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ test("IDs provided as numbers are coerced to strings", function(assert) {
var adapter = TestAdapter.extend({
findRecord(store, type, id, snapshot) {
assert.equal(typeof id, 'string', "id has been normalized to a string");
return resolve({ id: 1, name: "Scumbag Sylvain" });
return resolve({ id, name: "Scumbag Sylvain" });
}
});

Expand Down