Skip to content

Commit

Permalink
Trigger an assertion when calling findRecord with falsy (except 0) …
Browse files Browse the repository at this point in the history
…id value.
  • Loading branch information
e00dan committed Sep 29, 2015
1 parent 69b63f3 commit 010e846
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 3 deletions.
4 changes: 4 additions & 0 deletions packages/ember-data/lib/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ import InternalModel from "ember-data/system/model/internal-model";

import EmptyObject from "ember-data/system/empty-object";

export let badIdFormatAssertion = '`id` has to be non-empty string or number';

var Backburner = Ember._Backburner || Ember.Backburner || Ember.__loader.require('backburner')['default'] || Ember.__loader.require('backburner')['Backburner'];
var Map = Ember.Map;

Expand Down Expand Up @@ -495,6 +497,8 @@ Store = Service.extend({
*/
findRecord: function(modelName, id, options) {
Ember.assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
Ember.assert(badIdFormatAssertion, (typeof id === 'string' && id.length > 0) || (typeof id === 'number' && !isNaN(id)));

var internalModel = this._internalModelForId(modelName, id);
options = options || {};

Expand Down
18 changes: 16 additions & 2 deletions packages/ember-data/tests/integration/store-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ function ajaxResponse(value) {



module("integration/store - findRecord", {
});
module("integration/store - findRecord");

test("store#findRecord fetches record from server when cached record is not present", function() {
expect(2);
Expand Down Expand Up @@ -316,6 +315,21 @@ test("store#findRecord { reload: true } ignores cached record and reloads record
});
});

test('store#findRecord call with `id` of type different than non-empty string or number should trigger an assertion', assert => {
const badValues = ['', undefined, null, NaN, false];
assert.expect(badValues.length);

initializeStore(DS.RESTAdapter.extend());

run(function() {
badValues.map(item => {
expectAssertion(function() {
store.findRecord('car', item);
}, '`id` has to be non-empty string or number');
});
});
});

module("integration/store - findAll", {
setup: function() {
initializeStore(DS.RESTAdapter.extend());
Expand Down
1 change: 0 additions & 1 deletion packages/ember-data/tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,6 @@ test("currentState is accessible when the record is created", function() {
});
});


module("unit/model - DS.Model updating", {
setup: function() {
Person = DS.Model.extend({ name: DS.attr('string') });
Expand Down

0 comments on commit 010e846

Please sign in to comment.