diff --git a/package.json b/package.json index 0d5e038e7eb..6ff3d3f59a7 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "test:node": "lerna run test:node", "test:production": "yarn workspace ember-data test:production", "test:try-one": "yarn workspace ember-data test:try-one", - "test:enabled-in-progress-features": "yarn workspace ember-data test --enable-in-progress", + "test:enabled-in-progress-features": "yarn workspace ember-data test:optional-features", "test-external:ember-m3": "./bin/test-external-partner-project.js ember-m3 https://github.com/hjdivad/ember-m3.git", "test-external:ember-data-change-tracker": "./bin/test-external-partner-project.js ember-data-change-tracker https://github.com/danielspaniel/ember-data-change-tracker.git", "test-external:model-fragments": "./bin/test-external-partner-project.js ember-data-model-fragments https://github.com/lytics/ember-data-model-fragments.git", diff --git a/packages/-build-infra/src/debug-macros.js b/packages/-build-infra/src/debug-macros.js index 6f553ae36fa..16abd9852cb 100644 --- a/packages/-build-infra/src/debug-macros.js +++ b/packages/-build-infra/src/debug-macros.js @@ -10,17 +10,7 @@ module.exports = function debugMacros(environment) { flags: [ { source: '@ember-data/canary-features', - flags: Object.assign( - // explicit list of additional exports within @ember-data/canary-features - // without adding this (with a null value) an error is thrown during - // the feature replacement process (e.g. XYZ is not a supported flag) - { - FEATURES: null, - DEFAULT_FEATURES: null, - isEnabled: null, - }, - FEATURES - ), + flags: FEATURES, }, ], }, diff --git a/packages/-build-infra/src/features.js b/packages/-build-infra/src/features.js index 8c63e351ab3..9b09713a2fd 100644 --- a/packages/-build-infra/src/features.js +++ b/packages/-build-infra/src/features.js @@ -1,12 +1,29 @@ 'use strict'; function getFeatures() { - const features = { SAMPLE_FEATURE_FLAG: null }; - let enableFeatures = process.env.EMBER_DATA_FEATURES; - // turn on all features when given the above environment variable - if (enableFeatures) { - for (let key in features) { - features[key] = true; + const features = { + SAMPLE_FEATURE_FLAG: null, + RECORD_DATA_ERRORS: null, + }; + + const FEATURE_OVERRIDES = process.env.EMBER_DATA_FEATURE_OVERRIDE; + if (FEATURE_OVERRIDES === 'ENABLE_ALL_OPTIONAL') { + // enable all features with a current value of `null` + for (let feature in features) { + let featureValue = features[feature]; + + if (featureValue === null) { + features[feature] = true; + } + } + } else if (FEATURE_OVERRIDES) { + // enable only the specific features listed in the environment + // variable (comma separated) + const forcedFeatures = FEATURE_OVERRIDES.split(','); + for (var i = 0; i < forcedFeatures.length; i++) { + let featureName = forcedFeatures[i]; + + features[featureName] = true; } } diff --git a/packages/-ember-data/ember-cli-build.js b/packages/-ember-data/ember-cli-build.js index aa2951398b8..a47da076b2a 100644 --- a/packages/-ember-data/ember-cli-build.js +++ b/packages/-ember-data/ember-cli-build.js @@ -4,6 +4,11 @@ const EmberAddon = require('ember-cli/lib/broccoli/ember-addon'); module.exports = function(defaults) { let app = new EmberAddon(defaults, { + babel: { + // this ensures that the same `@ember-data/canary-features` processing that the various + // ember-data addons do is done in the dummy app + plugins: [...require('@ember-data/-build-infra/src/debug-macros')()], + }, 'ember-cli-babel': { throwUnlessParallelizable: true, }, diff --git a/packages/-ember-data/package.json b/packages/-ember-data/package.json index 7bc7622e707..29a7de59812 100644 --- a/packages/-ember-data/package.json +++ b/packages/-ember-data/package.json @@ -14,7 +14,7 @@ "test": "ember test", "test:all": "ember try:each", "test:production": "ember test -e production", - "test:optional-features": "ember test -e test-optional-features", + "test:optional-features": "EMBER_DATA_FEATURE_OVERRIDE=ENABLE_ALL_OPTIONAL ember test", "test:try-one": "ember try:one", "prepublishOnly": "ember build --environment=production && ember ts:precompile", "postpublish": "ember ts:clean" diff --git a/packages/-ember-data/tests/index.html b/packages/-ember-data/tests/index.html index 5209b852321..fd15b0f6ab8 100644 --- a/packages/-ember-data/tests/index.html +++ b/packages/-ember-data/tests/index.html @@ -24,6 +24,14 @@ + + + diff --git a/packages/-ember-data/tests/integration/record-data/record-data-errors-test.ts b/packages/-ember-data/tests/integration/record-data/record-data-errors-test.ts new file mode 100644 index 00000000000..ae523fa460f --- /dev/null +++ b/packages/-ember-data/tests/integration/record-data/record-data-errors-test.ts @@ -0,0 +1,352 @@ +import { get } from '@ember/object'; +import { setupTest } from 'ember-qunit'; +import Model from 'ember-data/model'; +import Store from 'ember-data/store'; +import { module, test } from 'qunit'; +import { settled } from '@ember/test-helpers'; +import EmberObject from '@ember/object'; +import { attr, hasMany, belongsTo } from '@ember-data/model'; +import { InvalidError, ServerError } from '@ember-data/adapter/error'; +import { JsonApiValidationError } from '@ember-data/store/-private/ts-interfaces/record-data-json-api'; +import RecordData, { RecordIdentifier } from '@ember-data/store/-private/ts-interfaces/record-data'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; + +class Person extends Model { + // TODO fix the typing for naked attrs + @attr('string', {}) + name; + + @attr('string', {}) + lastName; +} + +class TestRecordData implements RecordData { + commitWasRejected(recordIdentifier: RecordIdentifier, errors?: JsonApiValidationError[]): void {} + + // Use correct interface once imports have been fix + _storeWrapper: any; + + pushData(data, calculateChange?: boolean) {} + clientDidCreate() {} + + willCommit() {} + + unloadRecord() {} + rollbackAttributes() { + return []; + } + changedAttributes(): any {} + + hasChangedAttributes(): boolean { + return false; + } + + setDirtyAttribute(key: string, value: any) {} + + getAttr(key: string): string { + return 'test'; + } + + hasAttr(key: string): boolean { + return false; + } + + getHasMany(key: string) { + return {}; + } + + isRecordInUse(): boolean { + return true; + } + + isNew() { + return false; + } + + isDeleted() { + return false; + } + + addToHasMany(key: string, recordDatas: RecordData[], idx?: number) {} + removeFromHasMany(key: string, recordDatas: RecordData[]) {} + setDirtyHasMany(key: string, recordDatas: RecordData[]) {} + + getBelongsTo(key: string) { + return {}; + } + + setDirtyBelongsTo(name: string, recordData: RecordData | null) {} + + didCommit(data) {} + + isAttrDirty(key: string) { + return false; + } + removeFromInverseRelationships(isNew: boolean) {} + + _initRecordCreateOptions(options) { + return {}; + } +} + +let CustomStore = Store.extend({ + createRecordDataFor(modelName, id, clientId, storeWrapper) { + return new TestRecordData(); + }, +}); + +module('integration/record-data - Custom RecordData Errors', function(hooks) { + if (!RECORD_DATA_ERRORS) { + return; + } + + setupTest(hooks); + + let store; + + hooks.beforeEach(function() { + let { owner } = this; + + owner.register('model:person', Person); + owner.register('service:store', CustomStore); + }); + + test('Record Data invalid errors', async function(assert) { + assert.expect(2); + let called = 0; + let createCalled = 0; + const personHash = { + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Dale', + }, + }; + let { owner } = this; + + class LifecycleRecordData extends TestRecordData { + commitWasRejected(recordIdentifier, errors) { + assert.equal(errors[0].detail, 'is a generally unsavoury character', 'received the error'); + assert.equal(errors[0].source.pointer, '/data/attributes/name', 'pointer is correct'); + } + } + + let TestStore = Store.extend({ + createRecordDataFor(modelName, id, clientId, storeWrapper) { + return new LifecycleRecordData(); + }, + }); + + let TestAdapter = EmberObject.extend({ + updateRecord() { + return Promise.reject( + new InvalidError([ + { + title: 'Invalid Attribute', + detail: 'is a generally unsavoury character', + source: { + pointer: '/data/attributes/name', + }, + }, + ]) + ); + }, + + createRecord() { + return Promise.resolve(); + }, + }); + + owner.register('service:store', TestStore); + owner.register('adapter:application', TestAdapter, { singleton: false }); + + store = owner.lookup('service:store'); + + store.push({ + data: [personHash], + }); + let person = store.peekRecord('person', '1'); + person.save().then(() => {}, err => {}); + }); + + test('Record Data adapter errors', async function(assert) { + assert.expect(1); + const personHash = { + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Dale', + }, + }; + let { owner } = this; + + class LifecycleRecordData extends TestRecordData { + commitWasRejected(recordIdentifier, errors) { + assert.equal(errors, undefined, 'Did not pass adapter errors'); + } + } + + let TestStore = Store.extend({ + createRecordDataFor(modelName, id, clientId, storeWrapper) { + return new LifecycleRecordData(); + }, + }); + + let TestAdapter = EmberObject.extend({ + updateRecord() { + return Promise.reject(); + }, + }); + + owner.register('service:store', TestStore); + owner.register('adapter:application', TestAdapter, { singleton: false }); + + store = owner.lookup('service:store'); + + store.push({ + data: [personHash], + }); + let person = store.peekRecord('person', '1'); + await person.save().then(() => {}, err => {}); + }); + + test('Getting errors from Record Data shows up on the record', async function(assert) { + assert.expect(7); + let storeWrapper; + const personHash = { + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Dale', + lastName: 'something', + }, + }; + let { owner } = this; + let errorsToReturn = [ + { + title: 'Invalid Attribute', + detail: '', + source: { + pointer: '/data/attributes/name', + }, + }, + ]; + + class LifecycleRecordData extends TestRecordData { + constructor(sw) { + super(); + storeWrapper = sw; + } + + getErrors(recordIdentifier: RecordIdentifier): JsonApiValidationError[] { + return errorsToReturn; + } + } + + let TestStore = Store.extend({ + createRecordDataFor(modelName, id, clientId, storeWrapper) { + return new LifecycleRecordData(storeWrapper); + }, + }); + + owner.register('service:store', TestStore); + store = owner.lookup('service:store'); + + store.push({ + data: [personHash], + }); + let person = store.peekRecord('person', '1'); + let nameError = person + .get('errors') + .errorsFor('name') + .get('firstObject'); + assert.equal(nameError.attribute, 'name', 'error shows up on name'); + assert.equal(person.get('isValid'), false, 'person is not valid'); + errorsToReturn = []; + storeWrapper.notifyErrorsChange('person', '1'); + assert.equal(person.get('isValid'), true, 'person is valid'); + assert.equal(person.get('errors').errorsFor('name').length, 0, 'no errors on name'); + errorsToReturn = [ + { + title: 'Invalid Attribute', + detail: '', + source: { + pointer: '/data/attributes/lastName', + }, + }, + ]; + storeWrapper.notifyErrorsChange('person', '1'); + assert.equal(person.get('isValid'), false, 'person is valid'); + assert.equal(person.get('errors').errorsFor('name').length, 0, 'no errors on name'); + let lastNameError = person + .get('errors') + .errorsFor('lastName') + .get('firstObject'); + assert.equal(lastNameError.attribute, 'lastName', 'error shows up on lastName'); + }); + + test('Record data which does not implement getErrors still works correctly with the default DS.Model', async function(assert) { + assert.expect(4); + let called = 0; + let createCalled = 0; + const personHash = { + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Dale', + }, + }; + let { owner } = this; + + class LifecycleRecordData extends TestRecordData { + commitWasRejected(recordIdentifier, errors) { + assert.equal(errors[0].detail, 'is a generally unsavoury character', 'received the error'); + assert.equal(errors[0].source.pointer, '/data/attributes/name', 'pointer is correct'); + } + } + + let TestStore = Store.extend({ + createRecordDataFor(modelName, id, clientId, storeWrapper) { + return new LifecycleRecordData(); + }, + }); + + let TestAdapter = EmberObject.extend({ + updateRecord() { + return Promise.reject( + new InvalidError([ + { + title: 'Invalid Attribute', + detail: 'is a generally unsavoury character', + source: { + pointer: '/data/attributes/name', + }, + }, + ]) + ); + }, + + createRecord() { + return Promise.resolve(); + }, + }); + + owner.register('service:store', TestStore); + owner.register('adapter:application', TestAdapter, { singleton: false }); + + store = owner.lookup('service:store'); + + store.push({ + data: [personHash], + }); + let person = store.peekRecord('person', '1'); + await person.save().then(() => {}, err => {}); + + assert.equal(person.get('isValid'), false, 'rejecting the save invalidates the person'); + let nameError = person + .get('errors') + .errorsFor('name') + .get('firstObject'); + assert.equal(nameError.attribute, 'name', 'error shows up on name'); + }); +}); diff --git a/packages/-ember-data/tests/integration/record-data/record-data-test.ts b/packages/-ember-data/tests/integration/record-data/record-data-test.ts index 2ee45bbe34e..65617baaee1 100644 --- a/packages/-ember-data/tests/integration/record-data/record-data-test.ts +++ b/packages/-ember-data/tests/integration/record-data/record-data-test.ts @@ -6,6 +6,7 @@ import { module, test } from 'qunit'; import { settled } from '@ember/test-helpers'; import EmberObject from '@ember/object'; import { attr, hasMany, belongsTo } from '@ember-data/model'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; class Person extends Model { // TODO fix the typing for naked attrs @@ -25,32 +26,31 @@ class House extends Model { tenants; } - // TODO: this should work // class TestRecordData implements RecordData class TestRecordData { // Use correct interface once imports have been fix _storeWrapper: any; - pushData(data, calculateChange?: boolean) { } - clientDidCreate() { } + pushData(data, calculateChange?: boolean) {} + clientDidCreate() {} - willCommit() { } + willCommit() {} - commitWasRejected() { } + commitWasRejected() {} - unloadRecord() { } - rollbackAttributes() { } - changedAttributes(): any { } + unloadRecord() {} + rollbackAttributes() {} + changedAttributes(): any {} hasChangedAttributes(): boolean { return false; } - setDirtyAttribute(key: string, value: any) { } + setDirtyAttribute(key: string, value: any) {} getAttr(key: string): string { - return "test"; + return 'test'; } hasAttr(key: string): boolean { @@ -61,68 +61,70 @@ class TestRecordData { return {}; } - addToHasMany(key: string, recordDatas: this[], idx?: number) { } - removeFromHasMany(key: string, recordDatas: this[]) { } - setDirtyHasMany(key: string, recordDatas: this[]) { } + addToHasMany(key: string, recordDatas: this[], idx?: number) {} + removeFromHasMany(key: string, recordDatas: this[]) {} + setDirtyHasMany(key: string, recordDatas: this[]) {} - getBelongsTo(key: string) { } + getBelongsTo(key: string) {} - setDirtyBelongsTo(name: string, recordData: this | null) { } + setDirtyBelongsTo(name: string, recordData: this | null) {} - didCommit(data) { } + didCommit(data) {} - isAttrDirty(key: string) { return false; } - removeFromInverseRelationships(isNew: boolean) { } + isAttrDirty(key: string) { + return false; + } + removeFromInverseRelationships(isNew: boolean) {} - _initRecordCreateOptions(options) { } + _initRecordCreateOptions(options) {} } let CustomStore = Store.extend({ createRecordDataFor(modelName, id, clientId, storeWrapper) { return new TestRecordData(); - } + }, }); let houseHash, davidHash, runspiredHash, igorHash; -module('integration/record-data - Custom RecordData Implementations', function (hooks) { +module('integration/record-data - Custom RecordData Implementations', function(hooks) { setupTest(hooks); let store; - hooks.beforeEach(function () { + hooks.beforeEach(function() { let { owner } = this; houseHash = { type: 'house', id: '1', attributes: { - name: 'Moomin' - } + name: 'Moomin', + }, }; davidHash = { type: 'person', id: '1', attributes: { - name: 'David' - } + name: 'David', + }, }; runspiredHash = { type: 'person', id: '2', attributes: { - name: 'Runspired' - } + name: 'Runspired', + }, }; igorHash = { type: 'person', id: '3', attributes: { - name: 'Igor' - } + name: 'Igor', + }, }; owner.register('model:person', Person); @@ -130,7 +132,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( owner.register('service:store', CustomStore); }); - test("A RecordData implementation that has the required spec methods should not error out", async function (assert) { + test('A RecordData implementation that has the required spec methods should not error out', async function(assert) { let { owner } = this; store = owner.lookup('service:store'); @@ -173,7 +175,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(get(all, 'length'), 3); }); - test("Record Data push, create and save lifecycle", async function (assert) { + test('Record Data push, create and save lifecycle', async function(assert) { assert.expect(17); let called = 0; let createCalled = 0; @@ -182,10 +184,10 @@ module('integration/record-data - Custom RecordData Implementations', function ( id: '1', attributes: { name: 'Scumbag Dale', - } - } + }, + }; let { owner } = this; - let calledPush = 0 + let calledPush = 0; let calledClientDidCreate = 0; let calledWillCommit = 0; let calledWasRejected = 0; @@ -223,11 +225,10 @@ module('integration/record-data - Custom RecordData Implementations', function ( } } - let TestStore = Store.extend({ createRecordDataFor(modelName, id, clientId, storeWrapper) { return new LifecycleRecordData(); - } + }, }); let TestAdapter = EmberObject.extend({ @@ -242,7 +243,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( createRecord() { return Promise.resolve(); - } + }, }); owner.register('service:store', TestStore); @@ -251,7 +252,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [personHash] + data: [personHash], }); assert.equal(calledPush, 1, 'Called pushData'); @@ -309,18 +310,22 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(calledPush, 0, 'Did not call pushData'); }); - test("Record Data attribute settting", async function (assert) { - assert.expect(11); + test('Record Data attribute settting', async function(assert) { + let expectedCount = 11; + if (RECORD_DATA_ERRORS) { + expectedCount = 12; + } + assert.expect(expectedCount); const personHash = { type: 'person', id: '1', attributes: { name: 'Scumbag Dale', - } - } + }, + }; let { owner } = this; - let calledGet = 0 + let calledGet = 0; class AttributeRecordData extends TestRecordData { changedAttributes(): any { @@ -339,7 +344,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( getAttr(key: string): string { calledGet++; assert.equal(key, 'name', 'key passed to getAttr'); - return "new attribute"; + return 'new attribute'; } hasAttr(key: string): boolean { @@ -355,7 +360,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( let TestStore = Store.extend({ createRecordDataFor(modelName, id, clientId, storeWrapper) { return new AttributeRecordData(); - } + }, }); owner.register('service:store', TestStore); @@ -363,7 +368,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [personHash] + data: [personHash], }); let person = store.peekRecord('person', '1'); @@ -371,11 +376,19 @@ module('integration/record-data - Custom RecordData Implementations', function ( person.set('name', 'new value'); person.notifyPropertyChange('name'); assert.equal(person.get('name'), 'new attribute'); - assert.equal(calledGet, 3, 'called getAttr after notifyPropertyChange'); - assert.deepEqual(person.changedAttributes(), { name: ['old', 'new'] }, 'changed attributes passes through RD value'); + let expectedTimesToCallGet = 3; + if (RECORD_DATA_ERRORS) { + expectedTimesToCallGet = 4; + } + assert.equal(calledGet, expectedTimesToCallGet, 'called getAttr after notifyPropertyChange'); + assert.deepEqual( + person.changedAttributes(), + { name: ['old', 'new'] }, + 'changed attributes passes through RD value' + ); }); - test("Record Data controls belongsTo notifications", async function (assert) { + test('Record Data controls belongsTo notifications', async function(assert) { assert.expect(6); let called = 0; let createCalled = 0; @@ -384,7 +397,6 @@ module('integration/record-data - Custom RecordData Implementations', function ( let belongsToReturnValue = { data: { id: '1', type: 'person' } }; class RelationshipRecordData extends TestRecordData { - constructor(storeWrapper) { super(); this._storeWrapper = storeWrapper; @@ -409,7 +421,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( } else { return this._super(modelName, id, clientId, storeWrapper); } - } + }, }); owner.register('service:store', TestStore); @@ -417,11 +429,11 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [davidHash, runspiredHash] + data: [davidHash, runspiredHash], }); store.push({ - data: [houseHash] + data: [houseHash], }); let house = store.peekRecord('house', '1'); @@ -429,10 +441,14 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(house.get('landlord.name'), 'David', 'belongsTo get correctly looked up'); house.set('landlord', runspired); - assert.equal(house.get('landlord.name'), 'David', 'belongsTo does not change if RD did not notify'); + assert.equal( + house.get('landlord.name'), + 'David', + 'belongsTo does not change if RD did not notify' + ); }); - test("Record Data custom belongsTo", async function (assert) { + test('Record Data custom belongsTo', async function(assert) { assert.expect(4); let { owner } = this; @@ -462,7 +478,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( } else { return this._super(modelName, id, clientId, storeWrapper); } - } + }, }); owner.register('service:store', TestStore); @@ -470,14 +486,13 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [davidHash, runspiredHash, igorHash] + data: [davidHash, runspiredHash, igorHash], }); store.push({ - data: [houseHash] + data: [houseHash], }); - let house = store.peekRecord('house', '1'); assert.equal(house.get('landlord.name'), 'David', 'belongsTo get correctly looked up'); @@ -488,7 +503,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(house.get('landlord.name'), 'Igor', 'RecordData sets the custom belongsTo value'); }); - test("Record Data controls hasMany notifications", async function (assert) { + test('Record Data controls hasMany notifications', async function(assert) { assert.expect(10); let called = 0; let createCalled = 0; @@ -544,7 +559,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( } else { return this._super(modelName, id, clientId, storeWrapper); } - } + }, }); owner.register('service:store', TestStore); @@ -552,11 +567,11 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [davidHash, runspiredHash, igorHash] + data: [davidHash, runspiredHash, igorHash], }); store.push({ - data: [houseHash] + data: [houseHash], }); let house = store.peekRecord('house', '1'); @@ -571,13 +586,17 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.deepEqual(people.toArray(), [david], 'has many doesnt change if RD did not notify'); people.removeObject(david); - assert.deepEqual(people.toArray(), [david], 'hasMany removal doesnt apply the change unless notified'); + assert.deepEqual( + people.toArray(), + [david], + 'hasMany removal doesnt apply the change unless notified' + ); house.set('tenants', [igor]); assert.deepEqual(people.toArray(), [david], 'setDirtyHasMany doesnt apply unless notified'); }); - test("Record Data supports custom hasMany handling", async function (assert) { + test('Record Data supports custom hasMany handling', async function(assert) { assert.expect(10); let { owner } = this; @@ -606,7 +625,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(recordDatas[0].id, '2', 'Passed correct RD to addToHasMany'); calledAddToHasMany++; - hasManyReturnValue = { data: [{ id: '3', type: 'person'} , { id: '2', type: 'person' }] }; + hasManyReturnValue = { data: [{ id: '3', type: 'person' }, { id: '2', type: 'person' }] }; this._storeWrapper.notifyHasManyChange('house', '1', null, 'tenants'); } @@ -618,14 +637,14 @@ module('integration/record-data - Custom RecordData Implementations', function ( assert.equal(key, 'tenants', 'Passed correct key to removeFromHasMany'); assert.equal(recordDatas[0].id, '2', 'Passed correct RD to removeFromHasMany'); calledRemoveFromHasMany++; - hasManyReturnValue = { data: [{ id: '1', type: 'person'}] }; + hasManyReturnValue = { data: [{ id: '1', type: 'person' }] }; this._storeWrapper.notifyHasManyChange('house', '1', null, 'tenants'); } setDirtyHasMany(key: string, recordDatas: any[]) { assert.equal(key, 'tenants', 'Passed correct key to addToHasMany'); assert.equal(recordDatas[0].id, '3', 'Passed correct RD to addToHasMany'); - hasManyReturnValue = { data: [{ id: '1', type: 'person'} , { id: '2', type: 'person' }] }; + hasManyReturnValue = { data: [{ id: '1', type: 'person' }, { id: '2', type: 'person' }] }; this._storeWrapper.notifyHasManyChange('house', '1', null, 'tenants'); } } @@ -637,7 +656,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( } else { return this._super(modelName, id, clientId, storeWrapper); } - } + }, }); owner.register('service:store', TestStore); @@ -645,11 +664,11 @@ module('integration/record-data - Custom RecordData Implementations', function ( store = owner.lookup('service:store'); store.push({ - data: [davidHash, runspiredHash, igorHash] + data: [davidHash, runspiredHash, igorHash], }); store.push({ - data: [houseHash] + data: [houseHash], }); let house = store.peekRecord('house', '1'); @@ -672,5 +691,4 @@ module('integration/record-data - Custom RecordData Implementations', function ( // This is intentionally !== [igor] to test the custom RD implementation assert.deepEqual(people.toArray(), [david, runspired], 'setDirtyHasMany applies changes'); }); - }); diff --git a/packages/-ember-data/tests/unit/model-test.js b/packages/-ember-data/tests/unit/model-test.js index f842f47a25c..cd64c6ab9fe 100644 --- a/packages/-ember-data/tests/unit/model-test.js +++ b/packages/-ember-data/tests/unit/model-test.js @@ -1225,7 +1225,15 @@ module('unit/model - Model', function(hooks) { test('an invalid record becomes clean again if changed property is reset', async function(assert) { adapter.updateRecord = () => { - return reject(new InvalidError([{ name: 'not valid' }])); + return reject( + new InvalidError([ + { + source: { + pointer: '/data/attributes/name', + }, + }, + ]) + ); }; store.push({ @@ -1279,7 +1287,15 @@ module('unit/model - Model', function(hooks) { test('an invalid record stays dirty if only invalid property is reset', async function(assert) { adapter.updateRecord = () => { - return reject(new InvalidError([{ name: 'not valid' }])); + return reject( + new InvalidError([ + { + source: { + pointer: '/data/attributes/name', + }, + }, + ]) + ); }; store.push({ diff --git a/packages/-ember-data/tsconfig.json b/packages/-ember-data/tsconfig.json index aec3c076a4b..8cf7522af40 100644 --- a/packages/-ember-data/tsconfig.json +++ b/packages/-ember-data/tsconfig.json @@ -21,7 +21,10 @@ "ember-data/*": ["addon/*"], "ember-data/test-support": ["addon-test-support"], "ember-data/test-support/*": ["addon-test-support/*"], - "*": ["types/*"] + "@ember-data/store": ["../store/addon"], + "@ember-data/store/*": ["../store/addon/*"], + "@ember-data/adapter/error": ["../adapter/addon/error"], + "*": ["../store/types/*"] } }, "include": [ diff --git a/packages/adapter/addon/error.js b/packages/adapter/addon/error.js index 5908c1076c1..db3a86c81d3 100644 --- a/packages/adapter/addon/error.js +++ b/packages/adapter/addon/error.js @@ -1,12 +1,6 @@ -import { makeArray } from '@ember/array'; -import { isPresent } from '@ember/utils'; import EmberError from '@ember/error'; import { assert } from '@ember/debug'; -const SOURCE_POINTER_REGEXP = /^\/?data\/(attributes|relationships)\/(.*)/; -const SOURCE_POINTER_PRIMARY_REGEXP = /^\/?data/; -const PRIMARY_ATTRIBUTE_KEY = 'base'; - /** A `AdapterError` is used by an adapter to signal that an error occurred during a request to an external API. It indicates a generic error, and @@ -332,132 +326,4 @@ export const ServerError = extend( 'The adapter operation failed due to a server error' ); -/** - Convert an hash of errors into an array with errors in JSON-API format. - - ```javascript - import { errorsHashToArray } from '@ember-data/adapter/error'; - - let errors = { - base: 'Invalid attributes on saving this record', - name: 'Must be present', - age: ['Must be present', 'Must be a number'] - }; - - let errorsArray = errorsHashToArray(errors); - // [ - // { - // title: "Invalid Document", - // detail: "Invalid attributes on saving this record", - // source: { pointer: "/data" } - // }, - // { - // title: "Invalid Attribute", - // detail: "Must be present", - // source: { pointer: "/data/attributes/name" } - // }, - // { - // title: "Invalid Attribute", - // detail: "Must be present", - // source: { pointer: "/data/attributes/age" } - // }, - // { - // title: "Invalid Attribute", - // detail: "Must be a number", - // source: { pointer: "/data/attributes/age" } - // } - // ] - ``` - - @method errorsHashToArray - @public - @param {Object} errors hash with errors as properties - @return {Array} array of errors in JSON-API format -*/ -export function errorsHashToArray(errors) { - let out = []; - - if (isPresent(errors)) { - Object.keys(errors).forEach(key => { - let messages = makeArray(errors[key]); - for (let i = 0; i < messages.length; i++) { - let title = 'Invalid Attribute'; - let pointer = `/data/attributes/${key}`; - if (key === PRIMARY_ATTRIBUTE_KEY) { - title = 'Invalid Document'; - pointer = `/data`; - } - out.push({ - title: title, - detail: messages[i], - source: { - pointer: pointer, - }, - }); - } - }); - } - - return out; -} - -/** - Convert an array of errors in JSON-API format into an object. - - ```javascript - import { errorsArrayToHash } from '@ember-data/adapter/error'; - - let errorsArray = [ - { - title: 'Invalid Attribute', - detail: 'Must be present', - source: { pointer: '/data/attributes/name' } - }, - { - title: 'Invalid Attribute', - detail: 'Must be present', - source: { pointer: '/data/attributes/age' } - }, - { - title: 'Invalid Attribute', - detail: 'Must be a number', - source: { pointer: '/data/attributes/age' } - } - ]; - - let errors = errorsArrayToHash(errorsArray); - // { - // "name": ["Must be present"], - // "age": ["Must be present", "must be a number"] - // } - ``` - - @method errorsArrayToHash - @public - @param {Array} errors array of errors in JSON-API format - @return {Object} -*/ -export function errorsArrayToHash(errors) { - let out = {}; - - if (isPresent(errors)) { - errors.forEach(error => { - if (error.source && error.source.pointer) { - let key = error.source.pointer.match(SOURCE_POINTER_REGEXP); - - if (key) { - key = key[2]; - } else if (error.source.pointer.search(SOURCE_POINTER_PRIMARY_REGEXP) !== -1) { - key = PRIMARY_ATTRIBUTE_KEY; - } - - if (key) { - out[key] = out[key] || []; - out[key].push(error.detail || error.title); - } - } - }); - } - - return out; -} +export { errorsHashToArray, errorsArrayToHash } from '@ember-data/store/-private'; diff --git a/packages/adapter/tsconfig.json b/packages/adapter/tsconfig.json index e72b9a85523..72e5477cf94 100644 --- a/packages/adapter/tsconfig.json +++ b/packages/adapter/tsconfig.json @@ -23,6 +23,8 @@ "@ember-data/adapter/test-support/*": ["addon-test-support/*"], "ember-data": ["../-ember-data/addon"], "ember-data/*": ["../-ember-data/addon/*"], + "@ember-data/store": ["../store/addon"], + "@ember-data/store/*": ["../store/addon/*"], "*": ["types/*"] } }, diff --git a/packages/canary-features/addon/environment/global.js b/packages/canary-features/addon/environment/global.js deleted file mode 100644 index 0d8c8b47f43..00000000000 --- a/packages/canary-features/addon/environment/global.js +++ /dev/null @@ -1,17 +0,0 @@ -/* global global:false mainContext:false */ -// from lodash to catch fake globals -function checkGlobal(value) { - return value && value.Object === Object ? value : undefined; -} - -// element ids can ruin global miss checks -function checkElementIdShadowing(value) { - return value && value.nodeType === undefined ? value : undefined; -} - -// export real global -export default checkGlobal(checkElementIdShadowing(typeof global === 'object' && global)) || -checkGlobal(typeof self === 'object' && self) || -checkGlobal(typeof window === 'object' && window) || -(typeof mainContext !== 'undefined' && mainContext) || // set before strict mode in Ember loader/wrapper - new Function('return this')(); // eval outside of strict mode diff --git a/packages/canary-features/addon/environment/index.js b/packages/canary-features/addon/environment/index.js deleted file mode 100644 index 7afcc369ba1..00000000000 --- a/packages/canary-features/addon/environment/index.js +++ /dev/null @@ -1 +0,0 @@ -export * from './global'; diff --git a/packages/canary-features/addon/index.js b/packages/canary-features/addon/index.js index bbaa1b676b2..6316194d601 100644 --- a/packages/canary-features/addon/index.js +++ b/packages/canary-features/addon/index.js @@ -1,30 +1,24 @@ +/* globals EmberDataENV */ + import { assign } from '@ember/polyfills'; -import global from '@ember-data/canary-features/environment'; -export const ENV = { - FEATURES: {}, -}; -(EmberDataENV => { - if (typeof EmberDataENV !== 'object' || EmberDataENV === null) return; - for (let flag in EmberDataENV) { - if ( - !EmberDataENV.hasOwnProperty(flag) || - flag === 'EXTEND_PROTOTYPES' || - flag === 'EMBER_LOAD_HOOKS' - ) - continue; - let defaultValue = ENV[flag]; - if (defaultValue === true) { - ENV[flag] = EmberDataENV[flag] !== false; - } else if (defaultValue === false) { - ENV[flag] = EmberDataENV[flag] === true; - } - } -})(global.EmberDataENV || global.ENV); +const ENV = typeof EmberDataENV === 'object' && EmberDataENV !== null ? EmberDataENV : {}; +// TODO: Make this file the source of truth, currently this must match +// the contents of `packages/-build-infra/src/features.js` export const DEFAULT_FEATURES = { SAMPLE_FEATURE_FLAG: null, + RECORD_DATA_ERRORS: null, }; +function featureValue(value) { + if (ENV.ENABLE_OPTIONAL_FEATURES && value === null) { + return true; + } + + return value; +} + export const FEATURES = assign({}, DEFAULT_FEATURES, ENV.FEATURES); -export const SAMPLE_FEATURE_FLAG = FEATURES.SAMPLE_FEATURE_FLAG; +export const SAMPLE_FEATURE_FLAG = featureValue(FEATURES.SAMPLE_FEATURE_FLAG); +export const RECORD_DATA_ERRORS = featureValue(FEATURES.RECORD_DATA_ERRORS); diff --git a/packages/model/addon/-private/attr.js b/packages/model/addon/-private/attr.js index 85666cd88c6..0aea2024d32 100644 --- a/packages/model/addon/-private/attr.js +++ b/packages/model/addon/-private/attr.js @@ -2,6 +2,7 @@ import { computed } from '@ember/object'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { recordDataFor } from '@ember-data/store/-private'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; /** @module ember-data @@ -147,6 +148,16 @@ export default function attr(type, options) { ); } } + if (RECORD_DATA_ERRORS) { + let oldValue = this._internalModel._recordData.getAttr(key); + if (oldValue !== value) { + let errors = this.get('errors'); + if (errors.get(key)) { + errors.remove(key); + } + this._markInvalidRequestAsClean(); + } + } return this._internalModel.setDirtyAttribute(key, value); }, }).meta(meta); diff --git a/packages/model/index.js b/packages/model/index.js index e6b2a672511..4350d808784 100644 --- a/packages/model/index.js +++ b/packages/model/index.js @@ -7,6 +7,6 @@ const addonBaseConfig = addonBuildConfigForDataPackage(name); module.exports = Object.assign(addonBaseConfig, { shouldRollupPrivate: true, externalDependenciesForPrivateModule() { - return ['@ember-data/store', '@ember-data/store/-private']; + return ['@ember-data/canary-features', '@ember-data/store', '@ember-data/store/-private']; }, }); diff --git a/packages/store/addon/-private/index.js b/packages/store/addon/-private/index.js index d769e5e96f6..4ff5e854fd9 100644 --- a/packages/store/addon/-private/index.js +++ b/packages/store/addon/-private/index.js @@ -14,6 +14,8 @@ export { export { default as normalizeModelName } from './system/normalize-model-name'; export { default as coerceId } from './system/coerce-id'; +export { errorsHashToArray, errorsArrayToHash } from './system/errors-utils'; + // `ember-data-model-fragments` relies on `RootState` and `InternalModel` export { default as RootState } from './system/model/states'; export { default as InternalModel } from './system/model/internal-model'; diff --git a/packages/store/addon/-private/system/errors-utils.js b/packages/store/addon/-private/system/errors-utils.js new file mode 100644 index 00000000000..c5271ac1984 --- /dev/null +++ b/packages/store/addon/-private/system/errors-utils.js @@ -0,0 +1,139 @@ +import { makeArray } from '@ember/array'; +import { isPresent } from '@ember/utils'; + +const SOURCE_POINTER_REGEXP = /^\/?data\/(attributes|relationships)\/(.*)/; +const SOURCE_POINTER_PRIMARY_REGEXP = /^\/?data/; +const PRIMARY_ATTRIBUTE_KEY = 'base'; + +/** + Convert an hash of errors into an array with errors in JSON-API format. + ```javascript + import DS from 'ember-data'; + const { errorsHashToArray } = DS; + let errors = { + base: 'Invalid attributes on saving this record', + name: 'Must be present', + age: ['Must be present', 'Must be a number'] + }; + let errorsArray = errorsHashToArray(errors); + // [ + // { + // title: "Invalid Document", + // detail: "Invalid attributes on saving this record", + // source: { pointer: "/data" } + // }, + // { + // title: "Invalid Attribute", + // detail: "Must be present", + // source: { pointer: "/data/attributes/name" } + // }, + // { + // title: "Invalid Attribute", + // detail: "Must be present", + // source: { pointer: "/data/attributes/age" } + // }, + // { + // title: "Invalid Attribute", + // detail: "Must be a number", + // source: { pointer: "/data/attributes/age" } + // } + // ] + ``` + @method errorsHashToArray + @public + @namespace + @for DS + @param {Object} errors hash with errors as properties + @return {Array} array of errors in JSON-API format +*/ +export function errorsHashToArray(errors) { + let out = []; + + if (isPresent(errors)) { + Object.keys(errors).forEach(key => { + let messages = makeArray(errors[key]); + for (let i = 0; i < messages.length; i++) { + let title = 'Invalid Attribute'; + let pointer = `/data/attributes/${key}`; + if (key === PRIMARY_ATTRIBUTE_KEY) { + title = 'Invalid Document'; + pointer = `/data`; + } + out.push({ + title: title, + detail: messages[i], + source: { + pointer: pointer, + }, + }); + } + }); + } + + return out; +} + +/** + Convert an array of errors in JSON-API format into an object. + + ```javascript + import DS from 'ember-data'; + + const { errorsArrayToHash } = DS; + + let errorsArray = [ + { + title: 'Invalid Attribute', + detail: 'Must be present', + source: { pointer: '/data/attributes/name' } + }, + { + title: 'Invalid Attribute', + detail: 'Must be present', + source: { pointer: '/data/attributes/age' } + }, + { + title: 'Invalid Attribute', + detail: 'Must be a number', + source: { pointer: '/data/attributes/age' } + } + ]; + + let errors = errorsArrayToHash(errorsArray); + // { + // "name": ["Must be present"], + // "age": ["Must be present", "must be a number"] + // } + ``` + + @method errorsArrayToHash + @public + @namespace + @for DS + @param {Array} errors array of errors in JSON-API format + @return {Object} +*/ +export function errorsArrayToHash(errors) { + let out = {}; + + if (isPresent(errors)) { + errors.forEach(error => { + if (error.source && error.source.pointer) { + let key = error.source.pointer.match(SOURCE_POINTER_REGEXP); + + if (key) { + key = key[2]; + } else if (error.source.pointer.search(SOURCE_POINTER_PRIMARY_REGEXP) !== -1) { + key = PRIMARY_ATTRIBUTE_KEY; + } + + if (key) { + out[key] = out[key] || []; + out[key].push(error.detail || error.title); + } + } + }); + } + + return out; +} diff --git a/packages/store/addon/-private/system/model/internal-model.ts b/packages/store/addon/-private/system/model/internal-model.ts index 678d398680b..0d7010ceab8 100644 --- a/packages/store/addon/-private/system/model/internal-model.ts +++ b/packages/store/addon/-private/system/model/internal-model.ts @@ -14,13 +14,15 @@ import OrderedSet from '../ordered-set'; import ManyArray from '../many-array'; import { PromiseBelongsTo, PromiseManyArray } from '../promise-proxies'; import Store from '../store'; +import { errorsHashToArray, errorsArrayToHash } from '../errors-utils'; import { RecordReference, BelongsToReference, HasManyReference } from '../references'; import { default as recordDataFor, relationshipStateFor } from '../record-data-for'; import RecordData from '../../ts-interfaces/record-data'; -import { JsonApiResource } from '../../ts-interfaces/record-data-json-api'; +import { JsonApiResource, JsonApiValidationError } from '../../ts-interfaces/record-data-json-api'; import { Record } from '../../ts-interfaces/record'; import { Dict } from '../../types'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; // move to TS hacks module that we can delete when this is no longer a necessary recast type ManyArray = InstanceType; @@ -105,13 +107,12 @@ export default class InternalModel { // we create a new ManyArray, but in the interim the retained version will be // updated if inverse internal models are unloaded. _retainedManyArrayCache: Dict = Object.create(null); - _relationshipPromisesCache: Dict> = Object.create(null); + _relationshipPromisesCache: Dict> = Object.create(null); _relationshipProxyCache: Dict = Object.create(null); currentState: any; error: any; constructor(modelName: string, id: string | null, store, data, clientId) { - this.id = id; this.store = store; this.modelName = modelName; @@ -246,7 +247,10 @@ export default class InternalModel { } isValid() { - return this.currentState.isValid; + if (RECORD_DATA_ERRORS) { + } else { + return this.currentState.isValid; + } } dirtyType() { @@ -630,7 +634,7 @@ export default class InternalModel { manyArray => handleCompletedRelationshipRequest(this, key, jsonApi._relationship, manyArray, null), e => handleCompletedRelationshipRequest(this, key, jsonApi._relationship, null, e) - ); + ) as RSVP.Promise; this._relationshipPromisesCache[key] = loadingPromise; return loadingPromise; } @@ -896,13 +900,25 @@ export default class InternalModel { @param {String} name @param {Object} context */ - send(name, context?) { + send(name, context?, fromModel?) { let currentState = this.currentState; if (!currentState[name]) { this._unhandledEvent(currentState, name, context); } + if (RECORD_DATA_ERRORS) { + if ( + fromModel && + name === 'becameInvalid' && + this._recordData.getErrors && + this._recordData.getErrors({}).length === 0 + ) { + // this is a very specific internal hack for backsupport for record.send('becameInvalid') + let jsonApiErrors = [{ title: 'Invalid Error', detail: '', source: { pointer: '/data' } }]; + this._recordData.commitWasRejected({}, jsonApiErrors); + } + } return currentState[name](this, context); } @@ -1230,9 +1246,17 @@ export default class InternalModel { } hasErrors() { - let errors = get(this.getRecord(), 'errors'); - - return errors.get('length') > 0; + if (RECORD_DATA_ERRORS) { + if (this._recordData.getErrors) { + return this._recordData.getErrors({}).length > 0; + } else { + let errors = get(this.getRecord(), 'errors'); + return errors.get('length') > 0; + } + } else { + let errors = get(this.getRecord(), 'errors'); + return errors.get('length') > 0; + } } // FOR USE DURING COMMIT PROCESS @@ -1241,18 +1265,55 @@ export default class InternalModel { @method adapterDidInvalidate @private */ - adapterDidInvalidate(errors) { - let attribute; + adapterDidInvalidate(parsedErrors, error) { + if (RECORD_DATA_ERRORS) { + let attribute; + if (error && parsedErrors) { + if (!this._recordData.getErrors) { + for (attribute in parsedErrors) { + if (parsedErrors.hasOwnProperty(attribute)) { + this.addErrorMessageToAttribute(attribute, parsedErrors[attribute]); + } + } + } - for (attribute in errors) { - if (errors.hasOwnProperty(attribute)) { - this.addErrorMessageToAttribute(attribute, errors[attribute]); + let jsonApiErrors: JsonApiValidationError[] = errorsHashToArray(parsedErrors); + this.send('becameInvalid'); + if (jsonApiErrors.length === 0) { + jsonApiErrors = [{ title: 'Invalid Error', detail: '', source: { pointer: '/data' } }]; + } + this._recordData.commitWasRejected({}, jsonApiErrors); + } else { + this.send('becameError'); + this._recordData.commitWasRejected({}); } + } else { + let attribute; + + for (attribute in parsedErrors) { + if (parsedErrors.hasOwnProperty(attribute)) { + this.addErrorMessageToAttribute(attribute, parsedErrors[attribute]); + } + } + + this.send('becameInvalid'); + + this._recordData.commitWasRejected(); } + } - this.send('becameInvalid'); + notifyErrorsChange() { + let invalidErrors; + if (this._recordData.getErrors) { + invalidErrors = this._recordData.getErrors({}) || []; + } else { + return; + } + this.notifyInvalidErrorsChange(invalidErrors); + } - this._recordData.commitWasRejected(); + notifyInvalidErrorsChange(jsonApiErrors: JsonApiValidationError[]) { + this.getRecord().invalidErrorsChanged(jsonApiErrors); } /* diff --git a/packages/store/addon/-private/system/model/model.js b/packages/store/addon/-private/system/model/model.js index f1c9eed50b8..4e084d79817 100644 --- a/packages/store/addon/-private/system/model/model.js +++ b/packages/store/addon/-private/system/model/model.js @@ -5,6 +5,7 @@ import EmberObject, { computed, get } from '@ember/object'; import { DEBUG } from '@glimmer/env'; import { assert, warn, deprecate } from '@ember/debug'; import { PromiseObject } from '../promise-proxies'; +import { errorsArrayToHash } from '../errors-utils'; import Errors from '../model/errors'; import { relationshipsByNameDescriptor, @@ -16,6 +17,8 @@ import recordDataFor from '../record-data-for'; import Ember from 'ember'; import InternalModel from './internal-model'; import RootState from './states'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; + const { changeProperties } = Ember; /** @@ -59,6 +62,11 @@ const retrieveFromCurrentState = computed('currentState', function(key) { return get(this._internalModel.currentState, key); }).readOnly(); +const isValidRecordData = computed('errors.length', function(key) { + return !(this.get('errors.length') > 0); +}).readOnly(); + +const isValid = RECORD_DATA_ERRORS ? isValidRecordData : retrieveFromCurrentState; /** The model class that all Ember Data records descend from. @@ -70,6 +78,17 @@ const retrieveFromCurrentState = computed('currentState', function(key) { @uses Ember.Evented */ const Model = EmberObject.extend(Evented, { + init() { + this._super(...arguments); + if (RECORD_DATA_ERRORS) { + this._invalidRequests = []; + } + }, + + _notifyNetworkChanges: function() { + this.notifyPropertyChange('isValid'); + }, + /** If this property is `true` the record is in the `empty` state. Empty is the first state all records enter after they have @@ -237,7 +256,15 @@ const Model = EmberObject.extend(Evented, { @type {Boolean} @readOnly */ - isValid: retrieveFromCurrentState, + isValid: isValid, + + _markInvalidRequestAsClean() { + if (RECORD_DATA_ERRORS) { + this._invalidRequests = []; + this._notifyNetworkChanges(); + } + }, + /** If the record is in the dirty state this property will report what kind of change has caused it to move into the dirty @@ -407,9 +434,44 @@ const Model = EmberObject.extend(Evented, { this.send('becameValid'); } ); + if (RECORD_DATA_ERRORS) { + let recordData = recordDataFor(this); + let jsonApiErrors; + if (recordData.getErrors) { + jsonApiErrors = recordData.getErrors(); + if (jsonApiErrors) { + let errorsHash = errorsArrayToHash(jsonApiErrors); + let errorKeys = Object.keys(errorsHash); + + for (let i = 0; i < errorKeys.length; i++) { + errors._add(errorKeys[i], errorsHash[errorKeys[i]]); + } + } + } + } return errors; }).readOnly(), + invalidErrorsChanged(jsonApiErrors) { + if (RECORD_DATA_ERRORS) { + this._clearErrorMessages(); + let errors = errorsArrayToHash(jsonApiErrors); + let errorKeys = Object.keys(errors); + + for (let i = 0; i < errorKeys.length; i++) { + this._addErrorMessageToAttribute(errorKeys[i], errors[errorKeys[i]]); + } + } + }, + + _addErrorMessageToAttribute(attribute, message) { + this.get('errors')._add(attribute, message); + }, + + _clearErrorMessages() { + this.get('errors')._clear(); + }, + /** This property holds the `AdapterError` object with which last adapter operation was rejected. @@ -524,7 +586,7 @@ const Model = EmberObject.extend(Evented, { @param {Object} context */ send(name, context) { - return this._internalModel.send(name, context); + return this._internalModel.send(name, context, true); }, /** @@ -714,6 +776,9 @@ const Model = EmberObject.extend(Evented, { */ rollbackAttributes() { this._internalModel.rollbackAttributes(); + if (RECORD_DATA_ERRORS) { + this._markInvalidRequestAsClean(); + } }, /* diff --git a/packages/store/addon/-private/system/model/record-data.ts b/packages/store/addon/-private/system/model/record-data.ts index a1322565837..1c73133f96c 100644 --- a/packages/store/addon/-private/system/model/record-data.ts +++ b/packages/store/addon/-private/system/model/record-data.ts @@ -8,21 +8,25 @@ import coerceId from '../coerce-id'; import BelongsToRelationship from '../relationships/state/belongs-to'; import ManyRelationship from '../relationships/state/has-many'; import Relationship from '../relationships/state/relationship'; -import RecordData, { ChangedAttributesHash } from '../../ts-interfaces/record-data' -import { JsonApiResource, JsonApiResourceIdentity, JsonApiBelongsToRelationship, JsonApiHasManyRelationship, AttributesHash } from "../../ts-interfaces/record-data-json-api"; +import RecordData, { ChangedAttributesHash } from '../../ts-interfaces/record-data'; +import { + JsonApiResource, + JsonApiResourceIdentity, + JsonApiBelongsToRelationship, + JsonApiHasManyRelationship, + JsonApiValidationError, + AttributesHash, +} from '../../ts-interfaces/record-data-json-api'; import { RelationshipRecordData } from '../../ts-interfaces/relationship-record-data'; import { RecordDataStoreWrapper } from '../../ts-interfaces/record-data-store-wrapper'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; let nextBfsId = 1; export default class RecordDataDefault implements RelationshipRecordData { - store: any; - modelName: string; + _errors?: JsonApiValidationError[]; __relationships: Relationships | null; - __implicitRelationships:{ [key: string]: Relationship } | null; - clientId: string; - id: string | null; - storeWrapper: RecordDataStoreWrapper; + __implicitRelationships: { [key: string]: Relationship } | null; isDestroyed: boolean; _isNew: boolean; _bfsId: number; @@ -31,13 +35,15 @@ export default class RecordDataDefault implements RelationshipRecordData { __data: any; _scheduledDestroy: any; - constructor(modelName: string, id: string | null, clientId: string, storeWrapper: RecordDataStoreWrapper, store:any) { - this.store = store; - this.modelName = modelName; + constructor( + public modelName: string, + public id: string | null, + public clientId: string, + public storeWrapper: RecordDataStoreWrapper, + public store: any + ) { this.__relationships = null; this.__implicitRelationships = null; - this.clientId = clientId; - this.id = id; this.storeWrapper = storeWrapper; this.isDestroyed = false; this._isNew = false; @@ -90,6 +96,28 @@ export default class RecordDataDefault implements RelationshipRecordData { return this.__attributes !== null && Object.keys(this.__attributes).length > 0; } + _clearErrors() { + if (RECORD_DATA_ERRORS) { + if (this._errors) { + this._errors = undefined; + this.storeWrapper.notifyErrorsChange(this.modelName, this.id, this.clientId); + } + } + } + + getErrors(): JsonApiValidationError[] { + assert( + 'Can not call getErrors unless the RECORD_DATA_ERRORS feature flag is on', + RECORD_DATA_ERRORS + ); + if (RECORD_DATA_ERRORS) { + let errors: JsonApiValidationError[] = this._errors || []; + return errors; + } else { + return []; + } + } + // this is a hack bc we don't have access to the state machine // and relationships need this info and @runspired didn't see // how to get it just yet from storeWrapper. @@ -101,6 +129,7 @@ export default class RecordDataDefault implements RelationshipRecordData { this.__attributes = null; this.__inFlightAttributes = null; this.__data = null; + this._errors = undefined; } _setupRelationships(data) { @@ -179,10 +208,10 @@ export default class RecordDataDefault implements RelationshipRecordData { /* Checks if the attributes which are considered as changed are still different to the state which is acknowledged by the server. - + This method is needed when data for the internal model is pushed and the pushed data might acknowledge dirty attributes as confirmed. - + @method updateChangedAttributes @private */ @@ -206,7 +235,7 @@ export default class RecordDataDefault implements RelationshipRecordData { /* Returns an object, whose keys are changed properties, and value is an [oldProp, newProp] array. - + @method changedAttributes @private */ @@ -243,6 +272,8 @@ export default class RecordDataDefault implements RelationshipRecordData { this._inFlightAttributes = null; + this._clearErrors(); + return dirtyKeys; } @@ -268,6 +299,8 @@ export default class RecordDataDefault implements RelationshipRecordData { this._inFlightAttributes = null; this._updateChangedAttributes(); + this._clearErrors(); + return changedKeys; } @@ -293,7 +326,7 @@ export default class RecordDataDefault implements RelationshipRecordData { this._relationships.get(key).removeRecordDatas(recordDatas); } - commitWasRejected() { + commitWasRejected(identifier?, errors?: JsonApiValidationError[]) { let keys = Object.keys(this._inFlightAttributes); if (keys.length > 0) { let attrs = this._attributes; @@ -304,6 +337,12 @@ export default class RecordDataDefault implements RelationshipRecordData { } } this._inFlightAttributes = null; + if (RECORD_DATA_ERRORS) { + if (errors) { + this._errors = errors; + } + this.storeWrapper.notifyErrorsChange(this.modelName, this.id, this.clientId); + } } getBelongsTo(key: string): JsonApiBelongsToRelationship { @@ -385,10 +424,10 @@ export default class RecordDataDefault implements RelationshipRecordData { /** Computes the set of internal models reachable from `this` across exactly one relationship. - + @return {Array} An array containing the internal models that `this` belongs to or has many. - + */ _directlyRelatedRecordDatas(): RecordData[] { let array = []; @@ -403,11 +442,11 @@ export default class RecordDataDefault implements RelationshipRecordData { /** Computes the set of internal models reachable from this internal model. - + Reachability is determined over the relationship graph (ie a graph where nodes are internal models and edges are belongs to or has many relationships). - + @return {Array} An array including `this` and all internal models reachable from `this`. */ @@ -487,26 +526,26 @@ export default class RecordDataDefault implements RelationshipRecordData { implicit relationships are relationship which have not been declared but the inverse side exists on another record somewhere For example if there was - + ```app/models/comment.js import Model, { attr } from '@ember-data/model'; - + export default Model.extend({ name: attr() }); ``` - + but there is also - + ```app/models/post.js import Model, { attr, hasMany } from '@ember-data/model'; - + export default Model.extend({ name: attr(), comments: hasMany('comment') }); ``` - + would have a implicit post relationship in order to be do things like remove ourselves from the post when we are deleted */ @@ -589,17 +628,17 @@ export default class RecordDataDefault implements RelationshipRecordData { } /* - - + + TODO IGOR AND DAVID this shouldn't be public This method should only be called by records in the `isNew()` state OR once the record has been deleted and that deletion has been persisted. - + It will remove this record from any associated relationships. - + If `isNew` is true (default false), it will also completely reset all relationships to an empty state as well. - + @method removeFromInverseRelationships @param {Boolean} isNew whether to unload from the `isNew` perspective @private @@ -643,15 +682,15 @@ export default class RecordDataDefault implements RelationshipRecordData { /* Ember Data has 3 buckets for storing the value of an attribute on an internalModel. - + `_data` holds all of the attributes that have been acknowledged by a backend via the adapter. When rollbackAttributes is called on a model all attributes will revert to the record's state in `_data`. - + `_attributes` holds any change the user has made to an attribute that has not been acknowledged by the adapter. Any values in `_attributes` are have priority over values in `_data`. - + `_inFlightAttributes`. When a record is being synced with the backend the values in `_attributes` are copied to `_inFlightAttributes`. This way if the backend acknowledges the @@ -659,26 +698,26 @@ export default class RecordDataDefault implements RelationshipRecordData { values from `_inFlightAttributes` to `_data`. Without having to worry about changes made to `_attributes` while the save was happenign. - - + + Changed keys builds a list of all of the values that may have been changed by the backend after a successful save. - + It does this by iterating over each key, value pair in the payload returned from the server after a save. If the `key` is found in `_attributes` then the user has a local changed to the attribute that has not been synced with the server and the key is not included in the list of changed keys. - - - + + + If the value, for a key differs from the value in what Ember Data believes to be the truth about the backend state (A merger of the `_data` and `_inFlightAttributes` objects where `_inFlightAttributes` has priority) then that means the backend has updated the value and the key is added to the list of changed keys. - + @method _changedKeys @private */ diff --git a/packages/store/addon/-private/system/store.js b/packages/store/addon/-private/system/store.js index a98d4119306..8c2bfe7a416 100644 --- a/packages/store/addon/-private/system/store.js +++ b/packages/store/addon/-private/system/store.js @@ -45,6 +45,7 @@ import RecordArrayManager from './record-array-manager'; import InternalModel from './model/internal-model'; import RecordDataDefault from './model/record-data'; import edBackburner from './backburner'; +import { RECORD_DATA_ERRORS } from '@ember-data/canary-features'; const badIdFormatAssertion = '`id` passed to `findRecord()` has to be non-empty string or number'; const emberRun = emberRunLoop.backburner; @@ -2184,11 +2185,15 @@ const Store = Service.extend({ @param {InternalModel} internalModel @param {Object} errors */ - recordWasInvalid(internalModel, errors) { + recordWasInvalid(internalModel, parsedErrors, error) { if (DEBUG) { assertDestroyingStore(this, 'recordWasInvalid'); } - internalModel.adapterDidInvalidate(errors); + if (RECORD_DATA_ERRORS) { + internalModel.adapterDidInvalidate(parsedErrors, error); + } else { + internalModel.adapterDidInvalidate(parsedErrors); + } }, /** @@ -3272,9 +3277,8 @@ function _commit(adapter, store, operation, snapshot) { }, function(error) { if (error instanceof InvalidError) { - let errors = serializer.extractErrors(store, modelClass, error, snapshot.id); - - store.recordWasInvalid(internalModel, errors); + let parsedErrors = serializer.extractErrors(store, modelClass, error, snapshot.id); + store.recordWasInvalid(internalModel, parsedErrors, error); } else { store.recordWasError(internalModel, error); } diff --git a/packages/store/addon/-private/system/store/record-data-store-wrapper.ts b/packages/store/addon/-private/system/store/record-data-store-wrapper.ts index ae387fc5d2a..52afd4e13c6 100644 --- a/packages/store/addon/-private/system/store/record-data-store-wrapper.ts +++ b/packages/store/addon/-private/system/store/record-data-store-wrapper.ts @@ -27,6 +27,11 @@ export default class RecordDataStoreWrapper implements IRecordDataStoreWrapper { }); } + notifyErrorsChange(modelName: string, id: string | null, clientId: string | null) { + let internalModel = this.store._getInternalModelForId(modelName, id, clientId); + internalModel.notifyErrorsChange(); + } + _flushPendingManyArrayUpdates() { if (this._willUpdateManyArrays === false) { return; diff --git a/packages/store/addon/-private/ts-interfaces/record-data-json-api.ts b/packages/store/addon/-private/ts-interfaces/record-data-json-api.ts index 581afbc6c4a..77282c8d750 100644 --- a/packages/store/addon/-private/ts-interfaces/record-data-json-api.ts +++ b/packages/store/addon/-private/ts-interfaces/record-data-json-api.ts @@ -37,4 +37,13 @@ export interface JsonApiHasManyRelationship { // Private _relationship?: ManyRelationship; } + +export interface JsonApiValidationError { + title: string; + detail: string; + source: { + pointer: string; + } +} + export type JsonApiRelationship = JsonApiBelongsToRelationship | JsonApiHasManyRelationship; diff --git a/packages/store/addon/-private/ts-interfaces/record-data-store-wrapper.ts b/packages/store/addon/-private/ts-interfaces/record-data-store-wrapper.ts index e48337ddbba..266283ef33f 100644 --- a/packages/store/addon/-private/ts-interfaces/record-data-store-wrapper.ts +++ b/packages/store/addon/-private/ts-interfaces/record-data-store-wrapper.ts @@ -1,15 +1,31 @@ -import { RelationshipsSchema, AttributesSchema } from "./record-data-schemas"; +import { RelationshipsSchema, AttributesSchema } from './record-data-schemas'; export interface RecordDataStoreWrapper { relationshipsDefinitionFor(modelName: string): RelationshipsSchema; attributesDefinitionFor(modelName: string): AttributesSchema; setRecordId(modelName: string, id: string, clientId: string): void; disconnectRecord(modelName: string, id: string | null, clientId: string): void; isRecordInUse(modelName: string, id: string | null, clientId: string): boolean; - notifyPropertyChange(modelName: string, id: string | null, clientId: string | null, key: string): void; + notifyPropertyChange( + modelName: string, + id: string | null, + clientId: string | null, + key: string + ): void; // Needed For relationships - notifyHasManyChange(modelName: string, id: string | null, clientId: string | null, key: string): void; + notifyHasManyChange( + modelName: string, + id: string | null, + clientId: string | null, + key: string + ): void; recordDataFor(modelName: string, id: string, clientId?: string): unknown; - notifyBelongsToChange(modelName: string, id: string | null, clientId: string | null, key: string): void; + notifyBelongsToChange( + modelName: string, + id: string | null, + clientId: string | null, + key: string + ): void; inverseForRelationship(modelName: string, key: string): string; inverseIsAsyncForRelationship(modelName: string, key: string): boolean; -} \ No newline at end of file + notifyErrorsChange(modelName: string, id: string | null, clientId: string | null): void; +} diff --git a/packages/store/addon/-private/ts-interfaces/record-data.ts b/packages/store/addon/-private/ts-interfaces/record-data.ts index a6d8614a34c..d3b5203478c 100644 --- a/packages/store/addon/-private/ts-interfaces/record-data.ts +++ b/packages/store/addon/-private/ts-interfaces/record-data.ts @@ -2,17 +2,24 @@ import { JsonApiResource, JsonApiHasManyRelationship, JsonApiBelongsToRelationship, + JsonApiValidationError, } from './record-data-json-api'; export interface ChangedAttributesHash { [key: string]: [string, string]; } +export interface RecordIdentifier { + id?: string | null; + type?: string; + lid?: string; +} + export default interface RecordData { pushData(data: JsonApiResource, calculateChange?: boolean): void; clientDidCreate(): void; willCommit(): void; - commitWasRejected(): void; + commitWasRejected(recordIdentifier?: RecordIdentifier, errors?: JsonApiValidationError[]): void; unloadRecord(): void; rollbackAttributes(): string[]; changedAttributes(): ChangedAttributesHash; @@ -38,4 +45,10 @@ export default interface RecordData { isRecordInUse(): boolean; _initRecordCreateOptions(options: any): object; + + // new + + isNew(): boolean; + + getErrors?(recordIdentifier: RecordIdentifier): JsonApiValidationError[]; } diff --git a/packages/store/index.js b/packages/store/index.js index 66a1366bcbe..f61aa24a496 100644 --- a/packages/store/index.js +++ b/packages/store/index.js @@ -9,6 +9,7 @@ module.exports = Object.assign(addonBaseConfig, { externalDependenciesForPrivateModule() { return [ '@ember-data/adapter/error', + '@ember-data/canary-features', 'ember-inflector', '@ember/ordered-set', 'ember-data/-debug',