From d2e55b19a1b2c4bbd70f335730fbe5f87c265065 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 3 Jun 2020 17:58:24 -0400 Subject: [PATCH 1/3] =?UTF-8?q?chore:=20=F0=9F=A4=96=20add=20error=20handl?= =?UTF-8?q?ing=20support=20to=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/adapters/application.js | 85 +++++++++++++++++++ .../tests/unit/adapters/application-test.js | 41 +++++++++ 2 files changed, 126 insertions(+) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 71c109cb0..389212374 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'; export default class ApplicationAdapter extends RESTAdapter { // =attributes @@ -37,4 +38,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' + ); + }); }); From 01b876a465dd1489830b711b7b79c9144961bf5b Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 3 Jun 2020 19:00:34 -0400 Subject: [PATCH 2/3] =?UTF-8?q?chore:=20=F0=9F=A4=96=20Ensure=20create/upd?= =?UTF-8?q?ate/delete=20error=20messages=20display?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui/core/app/components/form/project/index.hbs | 27 +++- ui/core/app/routes/orgs/org/projects/new.js | 4 +- .../app/routes/orgs/org/projects/project.js | 8 +- ui/core/app/styles/app.scss | 2 +- ui/core/config/environment.js | 2 +- ui/core/mirage/config.js | 29 +++- ui/core/tests/acceptance/projects-test.js | 138 ++++++++++++++++++ 7 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 ui/core/tests/acceptance/projects-test.js 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 contained invalid fields.", + // "details": { + // "request_id": "abc123", + // "TraceId": "0123-4567", + // "fields": [ + // { + // "name": "name", + // "message": "name is required" + // }, + // { + // "name": "description", + // "message": "The description must not be more than 1 trillion characters long." + // } + // ] + // } + // }); + + return new Response(409, {}, { + "status": 409, + "code": "invalid_argument", + "message": "The request was awful." + }); + }); this.del('/orgs/:org_id/projects/:id'); } diff --git a/ui/core/tests/acceptance/projects-test.js b/ui/core/tests/acceptance/projects-test.js new file mode 100644 index 000000000..a98601b19 --- /dev/null +++ b/ui/core/tests/acceptance/projects-test.js @@ -0,0 +1,138 @@ +import { module, test } from 'qunit'; +import { visit, currentURL, click, fillIn, find } from '@ember/test-helpers'; +import { setupApplicationTest } from 'ember-qunit'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; +import a11yAudit from 'ember-a11y-testing/test-support/audit'; +import { Response } from 'miragejs'; + +module('Acceptance | projects', function (hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + test('visiting projects', async function (assert) { + assert.expect(1); + await visit('/orgs/1/projects'); + await a11yAudit(); + assert.equal(currentURL(), '/orgs/1/projects'); + }); + + test('saving a new project with invalid fields displays error messages', async function (assert) { + assert.expect(2); + this.server.post('/orgs/:org_id/projects', () => { + 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.' + ); + }); +}); From dbeff7bff86d70bcf6bd881f942671176dd71acc Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 4 Jun 2020 13:12:53 -0400 Subject: [PATCH 3/3] =?UTF-8?q?chore:=20=F0=9F=A4=96=20remove=20dev=20erro?= =?UTF-8?q?r=20mocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui/core/mirage/config.js | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 32e0aac11..46f58635e 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -1,5 +1,5 @@ import config from '../config/environment'; -import { Response } from 'miragejs'; +//import { Response } from 'miragejs'; export default function() { @@ -28,32 +28,6 @@ export default function() { this.get('/orgs/:org_id/projects'); this.post('/orgs/:org_id/projects'); this.get('/orgs/:org_id/projects/:id'); - this.patch('/orgs/:org_id/projects/:id', () => { - // return new Response(400, {}, { - // "status": 400, - // "code": "invalid_argument", - // "message": "The request contained invalid fields.", - // "details": { - // "request_id": "abc123", - // "TraceId": "0123-4567", - // "fields": [ - // { - // "name": "name", - // "message": "name is required" - // }, - // { - // "name": "description", - // "message": "The description must not be more than 1 trillion characters long." - // } - // ] - // } - // }); - - return new Response(409, {}, { - "status": 409, - "code": "invalid_argument", - "message": "The request was awful." - }); - }); + this.patch('/orgs/:org_id/projects/:id'); this.del('/orgs/:org_id/projects/:id'); }