Skip to content

Commit

Permalink
[BUGFIX] fix and tests for belongs-to proxy not properly updating (em…
Browse files Browse the repository at this point in the history
…berjs#5533)

* adds failing tests for emberjs#5511 and emberjs#5517

* add a test for emberjs#5525 that suspiciously passes

* adds test for emberjs#5522 isEmpty issue

* run prettier

* fix issue with proxy

* upgrade test for potential issue with create, still passes

* fix emberobserver issue for missing data member in payloads
  • Loading branch information
runspired authored and NullVoxPopuli committed Sep 8, 2018
1 parent 5f0bb9e commit bbda146
Show file tree
Hide file tree
Showing 13 changed files with 734 additions and 35 deletions.
1 change: 1 addition & 0 deletions addon/-legacy-private/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ const RootState = {
loading: {
// FLAGS
isLoading: true,
isEmpty: true,

exit(internalModel) {
internalModel._promiseProxy = null;
Expand Down
17 changes: 14 additions & 3 deletions addon/-legacy-private/system/relationships/state/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default class BelongsToRelationship extends Relationship {
} else if (this.inverseInternalModel) {
this.removeInternalModel(this.inverseInternalModel);
}

this.setHasAnyRelationshipData(true);
this.setRelationshipIsStale(false);
this.setRelationshipIsEmpty(false);
Expand Down Expand Up @@ -163,6 +164,12 @@ export default class BelongsToRelationship extends Relationship {
}

notifyBelongsToChange() {
if (this._promiseProxy !== null) {
let iM = this.inverseInternalModel;

this._updateLoadingPromise(proxyRecord(iM), iM ? iM.getRecord() : null);
}

this.internalModel.notifyBelongsToChange(this.key);
}

Expand Down Expand Up @@ -242,9 +249,7 @@ export default class BelongsToRelationship extends Relationship {

if (this.isAsync) {
if (this._promiseProxy === null) {
let promise = resolve(this.inverseInternalModel).then(internalModel => {
return internalModel ? internalModel.getRecord() : null;
});
let promise = proxyRecord(this.inverseInternalModel);
this._updateLoadingPromise(promise, record);
}

Expand Down Expand Up @@ -282,6 +287,12 @@ export default class BelongsToRelationship extends Relationship {
}
}

function proxyRecord(internalModel) {
return resolve(internalModel).then(resolvedInternalModel => {
return resolvedInternalModel ? resolvedInternalModel.getRecord() : null;
});
}

function handleCompletedFind(relationship, error) {
let internalModel = relationship.inverseInternalModel;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export default class Relationship {
this.canonicalMembers = new OrderedSet();
this.store = store;
this.key = relationshipMeta.key;
this.kind = relationshipMeta.kind;
this.inverseKey = inverseKey;
this.internalModel = internalModel;
this.isAsync = typeof async === 'undefined' ? true : async;
Expand Down Expand Up @@ -526,7 +527,7 @@ export default class Relationship {
warn(
`You pushed a record of type '${this.internalModel.modelName}' with a relationship '${
this.key
}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`,
}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload. EmberData will treat this relationship as known-to-be-empty.`,
this.isAsync || this.hasAnyRelationshipData,
{
id: 'ds.store.push-link-for-sync-relationship',
Expand Down Expand Up @@ -696,6 +697,11 @@ export default class Relationship {
this.updateData(payload.data, initial);
} else if (payload._partialData !== undefined) {
this.updateData(payload._partialData, initial);
} else if (this.isAsync === false) {
hasRelationshipDataProperty = true;
let data = this.kind === 'hasMany' ? [] : null;

this.updateData(data, initial);
}

if (payload.links && payload.links.related) {
Expand Down
2 changes: 1 addition & 1 deletion addon/-legacy-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -3162,7 +3162,7 @@ function setupRelationships(store, internalModel, data, modelNameToInverseMap) {
warn(
`You pushed a record of type '${
internalModel.modelName
}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`,
}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload. EmberData will treat this relationship as known-to-be-empty.`,
isAsync || relationshipData.data,
{
id: 'ds.store.push-link-for-sync-relationship',
Expand Down
2 changes: 1 addition & 1 deletion addon/-record-data-private/system/model/model-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default class ModelData {
warn(
`You pushed a record of type '${
this.modelName
}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`,
}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload. EmberData will treat this relationship as known-to-be-empty.`,
isAsync || relationshipData.data,
{
id: 'ds.store.push-link-for-sync-relationship',
Expand Down
1 change: 1 addition & 0 deletions addon/-record-data-private/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ const RootState = {
// XHR to retrieve the data.
loading: {
// FLAGS
isEmpty: true,
isLoading: true,

exit(internalModel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default class Relationship {
constructor(store, inverseKey, relationshipMeta, modelData, inverseIsAsync) {
heimdall.increment(newRelationship);
this.inverseIsAsync = inverseIsAsync;
this.kind = relationshipMeta.kind;
let async = relationshipMeta.options.async;
let polymorphic = relationshipMeta.options.polymorphic;
this.modelData = modelData;
Expand Down Expand Up @@ -572,7 +573,7 @@ export default class Relationship {
warn(
`You pushed a record of type '${this.modelData.modelName}' with a relationship '${
this.key
}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`,
}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload. EmberData will treat this relationship as known-to-be-empty.`,
this.isAsync || this.hasAnyRelationshipData,
{
id: 'ds.store.push-link-for-sync-relationship',
Expand Down Expand Up @@ -647,6 +648,11 @@ export default class Relationship {
if (payload.data !== undefined) {
hasRelationshipDataProperty = true;
this.updateData(payload.data, initial);
} else if (this.isAsync === false) {
hasRelationshipDataProperty = true;
let data = this.kind === 'hasMany' ? [] : null;

this.updateData(data, initial);
}

if (payload.links && payload.links.related) {
Expand Down
5 changes: 3 additions & 2 deletions addon/-record-data-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { A } from '@ember/array';
import EmberError from '@ember/error';
import MapWithDefault from './map-with-default';
import { run as emberRun } from '@ember/runloop';
import { run as emberRunLoop } from '@ember/runloop';
import { set, get, computed } from '@ember/object';
import { assign } from '@ember/polyfills';
import { default as RSVP, Promise } from 'rsvp';
Expand Down Expand Up @@ -52,6 +52,7 @@ import ModelData from './model/model-data';
import edBackburner from './backburner';

const badIdFormatAssertion = '`id` passed to `findRecord()` has to be non-empty string or number';
const emberRun = emberRunLoop.backburner;

const { ENV } = Ember;
let globalClientIdCounter = 1;
Expand Down Expand Up @@ -2033,7 +2034,7 @@ Store = Service.extend({
snapshot: snapshot,
resolver: resolver,
});
emberRun.once(this, this.flushPendingSave);
emberRun.scheduleOnce('actions', this, this.flushPendingSave);
},

/**
Expand Down
84 changes: 84 additions & 0 deletions tests/integration/records/create-record-test.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import { module, test } from 'qunit';
import JSONAPIAdapter from 'ember-data/adapters/json-api';
import JSONAPISerializer from 'ember-data/serializers/json-api';
import { setupTest } from 'ember-qunit';
import Store from 'ember-data/store';
import Model from 'ember-data/model';
import { resolve } from 'rsvp';
import { attr, belongsTo, hasMany } from '@ember-decorators/data';

class Person extends Model {
@hasMany('pet', { inverse: 'owner', async: false })
pets;
@belongsTo('pet', { inverse: 'bestHuman', async: true })
bestDog;
@attr name;
}

class Pet extends Model {
@belongsTo('person', { inverse: 'pets', async: false })
owner;
@belongsTo('person', { inverse: 'bestDog', async: false })
bestHuman;
@attr name;
}

Expand Down Expand Up @@ -111,4 +118,81 @@ module('Store.createRecord() coverage', function(hooks) {
.map(pet => pet.get('name'));
assert.deepEqual(pets, [], 'Chris no longer has any pets');
});

test('creating and saving a record with relationships puts them into the correct state', async function(assert) {
this.owner.register(
'serializer:application',
JSONAPISerializer.extend({
normalizeResponse(_, __, data) {
return data;
},
})
);
this.owner.register(
'adapter:application',
JSONAPIAdapter.extend({
shouldBackgroundReload() {
return false;
},
findRecord() {
assert.ok(false, 'Adapter should not make any findRecord Requests');
},
findBelongsTo() {
assert.ok(false, 'Adapter should not make any findBelongsTo Requests');
},
createRecord() {
return resolve({
data: {
type: 'pet',
id: '2',
attributes: { name: 'Shen' },
relationships: {
bestHuman: {
data: { type: 'person', id: '1' },
links: { self: './person', related: './person' },
},
},
},
});
},
})
);

let chris = store.push({
data: {
id: '1',
type: 'person',
attributes: {
name: 'Chris',
},
relationships: {
bestDog: {
data: null,
links: { self: './dog', related: './dog' },
},
},
},
});

let shen = store.createRecord('pet', {
name: 'Shen',
bestHuman: chris,
});

let bestHuman = shen.get('bestHuman');
let bestDog = await chris.get('bestDog');

// check that we are properly configured
assert.ok(bestHuman === chris, 'Precondition: Shen has bestHuman as Chris');
assert.ok(bestDog === shen, 'Precondition: Chris has Shen as his bestDog');

await shen.save();

bestHuman = shen.get('bestHuman');
bestDog = await chris.get('bestDog');

// check that the relationship has remained established
assert.ok(bestHuman === chris, 'Shen bestHuman is still Chris');
assert.ok(bestDog === shen, 'Chris still has Shen as bestDog');
});
});
Loading

0 comments on commit bbda146

Please sign in to comment.