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.

Sign up for GitHub

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

Jump to bottom

Properly warn when an updated record has a null id #4544

Closed
wants to merge 2 commits into from
Closed

Properly warn when an updated record has a null id #4544

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95605da
[BUGFIX RELEASE] fix regression in store beahvior when an updated rec…
runspired Sep 22, 2016
29c17c9
improve assertion flow
runspired Sep 22, 2016
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
16 changes: 14 additions & 2 deletions addon/-private/system/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1696,8 +1696,10 @@ 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);
}
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);
Expand Down Expand Up @@ -1745,7 +1747,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 (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;
}

this.typeMapFor(internalModel.type).idToRecord[id] = internalModel;

Expand Down