From 95605da28ed9a490ec8f92a4ee27a7b7b8060958 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 21 Sep 2016 18:52:52 -0700 Subject: [PATCH 1/2] [BUGFIX RELEASE] fix regression in store beahvior when an updated record has a null id --- addon/-private/system/store.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index be8b7ea3e24..9222ec83c32 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -1697,7 +1697,7 @@ Store = Service.extend({ this._backburner.schedule('normalizeRelationships', this, '_setupRelationships', internalModel, data); this.updateId(internalModel, data); } - assert(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the on the client side before saving the record.`, internalModel.id !== null); + //We first make sure the primary data has been updated //TODO try to move notification to the user to the end of the runloop internalModel.adapterDidCommit(data); @@ -1745,7 +1745,17 @@ Store = Service.extend({ var oldId = internalModel.id; var id = coerceId(data.id); - assert("An adapter cannot assign a new id to a record that already has an id. " + internalModel + " had id: " + oldId + " and you tried to update it with " + id + ". This likely happened because your server returned data in response to a find or update that had a different id than the one you sent.", oldId === null || id === oldId); + // ID absolutely can't be missing if the oldID is empty (missing Id in response for a new record) + assert(`The store cannot assign an empty id to record '${internalModel.type.modelName}:${internalModel._id}', because it already has an empty ID.`, id !== null && oldId !== null); + + // ID absolutely can't be different than oldID if oldID is not null + assert("The store cannot assign a new id to a record that already has an id. " + internalModel + " had id: " + oldId + " and you tried to update it with " + id + ". This likely happened because your server returned data in response to a find or update that had a different id than the one you sent.", oldId === null || id === oldId); + + // ID can be null if oldID is not null (altered ID in response for a record) + if (id === null) { + warn(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the id on the client side before saving the record.`, id !== null); + return; + } this.typeMapFor(internalModel.type).idToRecord[id] = internalModel; From 29c17c987a0387fd8bb45f5f18385c5b9b2d3671 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 22 Sep 2016 12:21:27 -0700 Subject: [PATCH 2/2] improve assertion flow --- addon/-private/system/store.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 9222ec83c32..9b7366a3971 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -1696,6 +1696,8 @@ Store = Service.extend({ // normalize relationship IDs into records this._backburner.schedule('normalizeRelationships', this, '_setupRelationships', internalModel, data); this.updateId(internalModel, data); + } else { + assert(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the id on the client side before saving the record.`, internalModel.id); } //We first make sure the primary data has been updated @@ -1749,11 +1751,11 @@ Store = Service.extend({ assert(`The store cannot assign an empty id to record '${internalModel.type.modelName}:${internalModel._id}', because it already has an empty ID.`, id !== null && oldId !== null); // ID absolutely can't be different than oldID if oldID is not null - assert("The store cannot assign a new id to a record that already has an id. " + internalModel + " had id: " + oldId + " and you tried to update it with " + id + ". This likely happened because your server returned data in response to a find or update that had a different id than the one you sent.", oldId === null || id === oldId); + assert("The store cannot assign a new id to a record that already has an id. " + internalModel + " had id: " + oldId + " and you tried to update it with " + id + ". This likely happened because your server returned data in response to a find or update that had a different id than the one you sent.", oldId !== null && id === oldId); // ID can be null if oldID is not null (altered ID in response for a record) - if (id === null) { - warn(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the id on the client side before saving the record.`, id !== null); + if (oldId !== null && id === null) { + warn(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the id on the client side before saving the record.`, !(oldId !== null && id === null)); return; }