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.

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

[Bug]: errors remove to accurately update in template #7335

Merged
merged 2 commits into from
Sep 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ jobs:
strategy:
fail-fast: false
matrix:
scenario: [ember-lts-3.12, ember-lts-3.16]
scenario: [ember-lts-3.16, ember-lts-3.20]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
Expand Down
8 changes: 4 additions & 4 deletions packages/-ember-data/config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,18 @@ module.exports = function() {
},
},
{
name: 'ember-lts-3.12',
name: 'ember-lts-3.16',
npm: {
devDependencies: {
'ember-source': '~3.12.0',
'ember-source': '~3.16.0',
},
},
},
{
name: 'ember-lts-3.16',
name: 'ember-lts-3.20',
npm: {
devDependencies: {
'ember-source': '~3.16.0',
'ember-source': '~3.20.0',
},
},
},
Expand Down
70 changes: 70 additions & 0 deletions packages/-ember-data/tests/integration/model-errors-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import 'qunit-dom'; // tell TS consider *.dom extension for assert

// @ts-ignore
import { setComponentTemplate } from '@ember/component';
import { get } from '@ember/object';
import { render, settled } from '@ember/test-helpers';
import Component from '@glimmer/component';

import { module, test } from 'qunit';

import { hbs } from 'ember-cli-htmlbars';
import { setupRenderingTest } from 'ember-qunit';
import { TestContext } from 'ember-test-helpers';

import Model, { attr } from '@ember-data/model';

class Tag extends Model {
@attr('string', {})
name;
}

class ErrorList extends Component<{ model: Model; field: string }> {
get errors() {
const { model, field } = this.args;
return model.errors.errorsFor(field).map(error => error.message);
}
}

const template = hbs`
<ul class="error-list">
{{#each this.errors as |error|}}
<li class="error-list__error">{{error}}</li>
{{/each}}
</ul>
`;

interface CurrentTestContext extends TestContext {
tag: Tag;
}

module('integration/model.errors', function(hooks) {
setupRenderingTest(hooks);

hooks.beforeEach(function() {
let { owner } = this;

owner.register('model:tag', Tag);
owner.register('component:error-list', setComponentTemplate(template, ErrorList));
});

test('Model errors are autotracked', async function(this: CurrentTestContext, assert) {
this.tag = this.owner.lookup('service:store').createRecord('tag');
// @ts-ignore
const errors = get(this.tag, 'errors');

await render(hbs`<ErrorList @model={{this.tag}} @field="name"/>`);

assert.dom('.error-list__error').doesNotExist();

errors.add('name', 'the-error');
await settled();

assert.dom('.error-list__error').hasText('the-error');

errors.remove('name');
await settled();

assert.dom('.error-list__error').doesNotExist();
});
});
10 changes: 10 additions & 0 deletions packages/model/addon/-private/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,16 @@ export default ArrayProxy.extend(DeprecatedEvented, {

let content = this.rejectBy('attribute', attribute);
get(this, 'content').setObjects(content);

// Although errorsByAttributeName.delete is technically enough to sync errors state, we also
// must mutate the array as well for autotracking
let errors = this.errorsFor(attribute);
for (let i = 0; i < errors.length; i++) {
if (errors[i].attribute === attribute) {
// .replace from Ember.NativeArray is necessary. JS splice will not work.
errors.replace(i, 1);
}
}
get(this, 'errorsByAttributeName').delete(attribute);

this.notifyPropertyChange(attribute);
Expand Down