Merge pull request #37 from hashicorp/chore-errors

Errors UI

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;
+  }
 }

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'
+    );
+  });
 });

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}}
+      <field.errors as |errors|>
+        {{#each @model.errors.name as |error|}}
+          <errors.message>{{error.message}}</errors.message>
+        {{/each}}
+      </field.errors>
+    {{/if}}
+  </form.input>
 
   <form.textarea
     name="description"
     @type="description"
     @value={{@model.description}}
     @label={{t "form.label.description"}}
-    @error={{false}}
+    @error={{@model.errors.description}}
     disabled={{@model.isSaving}}
-    readonly={{false}} />
+    readonly={{false}}
+  as |field|>
+    {{#if @model.errors.description}}
+      <field.errors as |errors|>
+        {{#each @model.errors.description as |error|}}
+          <errors.message>{{error.message}}</errors.message>
+        {{/each}}
+      </field.errors>
+    {{/if}}
+  </form.textarea>
 
   <form.actions
     @disabled={{if @model.cannotSave @model.cannotSave}}

ui/core/app/routes/orgs/org/projects/new.js

@@ -42,9 +42,9 @@ export default class OrgsOrgProjectsNewRoute extends Route {
       this.transitionTo('orgs.org.projects.project', project);
       // TODO: replace with translated strings
       this.notify.success('Project created.');
-    } catch (e) {
+    } catch (error) {
       // TODO: replace with translated strings
-      this.notify.error('Project could not be created.', {closeAfter: null});
+      this.notify.error(error.message, { closeAfter: null });
     }
   }
 }

ui/core/app/routes/orgs/org/projects/project.js

@@ -50,9 +50,9 @@ export default class OrgsOrgProjectsProjectRoute extends Route {
       await project.save();
       // TODO: replace with translated strings
       this.notify.success('Save succeeded.');
-    } catch (e) {
+    } catch (error) {
       // TODO: replace with translated strings
-      this.notify.error('An error occurred.', {closeAfter: null});
+      this.notify.error(error.message, { closeAfter: null });
     }
   }
 
@@ -67,9 +67,9 @@ export default class OrgsOrgProjectsProjectRoute extends Route {
       this.transitionTo('orgs.org.projects');
       // TODO: replace with translated strings
       this.notify.success('Deleted project succesfully.');
-    } catch (e) {
+    } catch (error) {
       // TODO: replace with translated strings
-      this.notify.error('An error occurred.', {closeAfter: null});
+      this.notify.error(error.message, { closeAfter: null });
     }
   }
 }

ui/core/app/styles/app.scss

@@ -9,7 +9,7 @@
 
 .rose-form {
   width: 66%;
-  
+
   .rose-form-input {
     width: 100%;
 

ui/core/config/environment.js

@@ -41,7 +41,7 @@ module.exports = function (environment) {
       'frame-src': ["'self'"],
       'font-src': ["'self'"],
       'connect-src': ["'self'"],
-      'img-src': ["'self'", "data:"],
+      'img-src': ["'self'", 'data:'],
       'style-src': ["'self'"],
       'media-src': ["'self'"],
       'manifest-src': ["'self'"],

ui/core/mirage/config.js

@@ -1,4 +1,5 @@
 import config from '../config/environment';
+//import { Response } from 'miragejs';
 
 export default function() {
 

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.'
+    );
+  });
+});