Skip to content

Commit

Permalink
[BUGFIX] Reference.reload should not cause sync-relationship assertion (
Browse files Browse the repository at this point in the history
#5582)

* refactor reload tests and add tests for reference reloading
* dont assert loaded state if reloading
* only disable data integrity checks for the forced reload itself
  • Loading branch information
runspired authored Aug 27, 2018
1 parent 52f30be commit f5618a4
Show file tree
Hide file tree
Showing 7 changed files with 675 additions and 214 deletions.
10 changes: 8 additions & 2 deletions addon/-legacy-private/system/relationships/state/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,13 @@ export default class BelongsToRelationship extends Relationship {
});
}

getData() {
/*
While the `shouldForceReload` flag will also be true when `isForcedReload` is true,
`isForcedReload` is only `true` for an initial `getData` call during a forced reload.
Other calls must conform to the typical expectations, for instance, sync relationships
expect that their data is already loaded.
*/
getData(isForcedReload = false) {
//TODO(Igor) flushCanonical here once our syncing is not stupid
let record = this.inverseInternalModel ? this.inverseInternalModel.getRecord() : null;

Expand Down Expand Up @@ -264,7 +270,7 @@ export default class BelongsToRelationship extends Relationship {
"' with id " +
this.internalModel.id +
' but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (`DS.belongsTo({ async: true })`)',
record === null || !record.get('isEmpty')
record === null || !record.get('isEmpty') || isForcedReload
);
return record;
}
Expand Down
4 changes: 2 additions & 2 deletions addon/-legacy-private/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export default class ManyRelationship extends Relationship {
});
}

getData() {
getData(isForcedReload = false) {
//TODO(Igor) sync server here, once our syncing is not stupid
let manyArray = this.manyArray;

Expand Down Expand Up @@ -379,7 +379,7 @@ export default class ManyRelationship extends Relationship {
}' with id ${
this.internalModel.id
} but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (\`DS.hasMany({ async: true })\`)`,
this.allInverseRecordsAreLoaded
this.allInverseRecordsAreLoaded || isForcedReload
);

return manyArray;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ export default class Relationship {

this.setHasFailedLoadAttempt(false);
this.setShouldForceReload(true);
this.getData();
this.getData(true);

return this._promiseProxy;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/relationships/belongs-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class TestAdapter extends JSONAPIAdapter {
this._payloads = arr;
}

shouldBackgroundReload() {
shouldBackgroundReloadRecord() {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class TestAdapter extends JSONAPIAdapter {
this._payloads = arr;
}

shouldBackgroundReload() {
shouldBackgroundReloadRecord() {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/records/create-record-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ module('Store.createRecord() coverage', function(hooks) {
this.owner.register(
'adapter:application',
JSONAPIAdapter.extend({
shouldBackgroundReload() {
shouldBackgroundReloadRecord() {
return false;
},
findRecord() {
Expand Down
Loading

0 comments on commit f5618a4

Please sign in to comment.