diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 3bd3342a1..ace0b9669 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -1,6 +1,7 @@ import RESTAdapter from '@ember-data/adapter/rest'; import config from 'ember-get-config'; import { get } from '@ember/object'; +import { InvalidError } from '@ember-data/adapter/error'; import { dasherize } from '@ember/string'; import { pluralize } from 'ember-inflector'; @@ -50,4 +51,88 @@ export default class ApplicationAdapter extends RESTAdapter { const method = type === 'PUT' ? 'PATCH' : type; return super.ajax(url, method, options); } + + /** + * Our API represents invalid requests via the 400 error. But Ember Data + * expects 422 by default. We change it here. + * + * @override + * @method isInvalid + * @param {Number} status + * @param {Object} headers + * @param {Object} payload + * @return {Boolean} + */ + isInvalid(status /*, headers, payload*/) { + return status === 400; + } + + /** + * Ember Data blatantly assumes errors always present under an `errors` key + * in the payload, even _before normalizing errors_. Bizarre. Instead, + * we pass the payload through an adapter-level transformation step if the + * request was invalid. Then we can rely on the serializer's `extractErrors` + * to handle the rest. + * + * @override + * @method handleResponse + * @param {Number} status + * @param {Object} headers + * @param {Object} payload + * @param {Object} requestData - the original request information + * @return {Object | AdapterError} response + */ + handleResponse(status, headers, payload, requestData) { + if (this.isInvalid(status, headers, payload)) { + const detailedMessage = this.generatedDetailedMessage(...arguments); + const transformedPayload = this.transformValidationErrors(payload); + return new InvalidError(transformedPayload.errors, detailedMessage); + } + return super.handleResponse(status, headers, payload, requestData); + } + + /** + * Normalizes the errors found in a payload from our API into JSON API + * error format. + * + * @param {object} payload + * @return {object} + */ + transformValidationErrors(payload) { + const fieldErrors = get(payload, 'details.fields') || []; + // Normalize the primary error message into "base", which is an error + // that applies to the whole model instance + const baseError = { + status: payload.source, + code: payload.code, + detail: payload.message, + source: { pointer: '/data' }, + }; + // Normalize field-specific errors, if any. + const errors = fieldErrors.map((error) => ({ + detail: error.message, + source: { pointer: `/data/attributes/${error.name}` }, + })); + // Return a list of JSON API errors rooted under the `errors` key. + return { + errors: [baseError, ...errors], + }; + } + + /** + * Returns `payload.message`, which our API provides as a top-level + * description of the error that occurred. + * + * @override + * @method generatedDetailedMessage + * @private + * @param {Number} status + * @param {Object} headers + * @param {Object} payload + * @param {Object} requestData + * @return {String} detailed error message + */ + generatedDetailedMessage(status, headers, payload /*, requestData*/) { + return payload.message; + } } diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js index 5d07e1ccf..8464a7513 100644 --- a/addons/api/tests/unit/adapters/application-test.js +++ b/addons/api/tests/unit/adapters/application-test.js @@ -2,6 +2,7 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; import config from 'ember-get-config'; import RESTAdapter from '@ember-data/adapter/rest'; +import { InvalidError } from '@ember-data/adapter/error'; module('Unit | Adapter | application', function (hooks) { setupTest(hooks); @@ -28,4 +29,44 @@ module('Unit | Adapter | application', function (hooks) { }; adapter.ajax('/', 'PUT'); }); + + test('it correctly identifies 400 responses as invalid', function (assert) { + assert.expect(2); + const adapter = this.owner.lookup('adapter:application'); + assert.ok(adapter.isInvalid(400)); + assert.notOk(adapter.isInvalid(401)); + }); + + test('it returns an proper InvalidError from handleResponse', function (assert) { + assert.expect(3); + const adapter = this.owner.lookup('adapter:application'); + let payload = { + status: 400, + code: 'invalid_argument', + message: 'The request was invalid.', + }; + let handledResponse = adapter.handleResponse(400, {}, payload); + assert.ok(handledResponse instanceof InvalidError); + assert.equal(handledResponse.errors.length, 1); + assert.equal(handledResponse.message, 'The request was invalid.'); + }); + + test('it returns field-level errors in InvalidError from handleResponse', function (assert) { + assert.expect(1); + const adapter = this.owner.lookup('adapter:application'); + let payload = { + status: 400, + code: 'invalid_argument', + message: 'The request was invalid.', + details: { + fields: [{ name: 'name', message: 'Name is wrong.' }], + }, + }; + let handledResponse = adapter.handleResponse(400, {}, payload); + assert.equal( + handledResponse.errors.length, + 2, + 'A base error plus one field error' + ); + }); }); diff --git a/ui/core/app/components/form/project/index.hbs b/ui/core/app/components/form/project/index.hbs index 7541f8ed5..9db695baa 100644 --- a/ui/core/app/components/form/project/index.hbs +++ b/ui/core/app/components/form/project/index.hbs @@ -2,6 +2,7 @@ @onSubmit={{@submit}} @cancel={{@cancel}} @disabled={{@model.isSaving}} + @error={{true}} as |form| > @@ -10,17 +11,35 @@ @type="name" @value={{@model.name}} @label={{t "form.label.name"}} - @error={{false}} - readonly={{false}} /> + @error={{@model.errors.name}} + readonly={{false}} + as |field|> + {{#if @model.errors.name}} + + {{#each @model.errors.name as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + readonly={{false}} + as |field|> + {{#if @model.errors.description}} + + {{#each @model.errors.description as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + { + return new Response( + 400, + {}, + { + status: 400, + code: 'invalid_argument', + message: 'The request was invalid.', + details: { + fields: [ + { + name: 'name', + message: 'Name is required.', + }, + ], + }, + } + ); + }); + await visit('/orgs/1/projects/new'); + await fillIn('[name="name"]', 'random string'); + await click('[type="submit"]'); + assert.ok( + find('[role="alert"]').textContent.trim(), + 'The request was invalid.', + 'Displays primary error message.' + ); + assert.ok( + find('.rose-form-error-message').textContent.trim(), + 'Name is required.', + 'Displays field-level errors.' + ); + }); + + test('saving an existing project with invalid fields displays error messages', async function (assert) { + assert.expect(2); + this.server.createList('project', 1); + this.server.patch('/orgs/:org_id/projects/:id', () => { + return new Response( + 400, + {}, + { + status: 400, + code: 'invalid_argument', + message: 'The request was invalid.', + details: { + fields: [ + { + name: 'name', + message: 'Name is required.', + }, + ], + }, + } + ); + }); + await visit('/orgs/1/projects/1'); + await fillIn('[name="name"]', 'random string'); + await click('[type="submit"]'); + assert.ok( + find('[role="alert"]').textContent.trim(), + 'The request was invalid.', + 'Displays primary error message.' + ); + assert.ok( + find('.rose-form-error-message').textContent.trim(), + 'Name is required.', + 'Displays field-level errors.' + ); + }); + + test('errors are displayed when save project fails', async function (assert) { + assert.expect(1); + this.server.createList('project', 1); + this.server.patch('/orgs/:org_id/projects/:id', () => { + return new Response( + 490, + {}, + { + status: 490, + code: 'error', + message: 'Oops.', + } + ); + }); + await visit('/orgs/1/projects/1'); + await fillIn('[name="name"]', 'random string'); + await click('[type="submit"]'); + assert.ok( + find('[role="alert"]').textContent.trim(), + 'Oops.', + 'Displays primary error message.' + ); + }); + + test('errors are displayed when delete project fails', async function (assert) { + assert.expect(1); + this.server.createList('project', 1); + this.server.del('/orgs/:org_id/projects/:id', () => { + return new Response( + 490, + {}, + { + status: 490, + code: 'error', + message: 'Oops.', + } + ); + }); + await visit('/orgs/1/projects/1'); + await click('.rose-button-warning'); + assert.ok( + find('[role="alert"]').textContent.trim(), + 'Oops.', + 'Displays primary error message.' + ); + }); +});