From aa9eb4f8d4892c8225b6edc41081eeab99fcf040 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Mon, 14 Sep 2020 10:31:43 -0400 Subject: [PATCH 1/5] feat: Introduce error component --- ui/core/app/components/error/message.hbs | 4 + ui/core/app/components/error/message.js | 104 ++++++++++++++++++ ui/core/app/routes/application.js | 2 - ui/core/app/routes/scopes/scope.js | 1 + ui/core/app/templates/scopes/scope/error.hbs | 6 + ui/core/mirage/config.js | 16 ++- .../components/error/message-test.js | 26 +++++ ui/core/translations/en-us.yaml | 6 + 8 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 ui/core/app/components/error/message.hbs create mode 100644 ui/core/app/components/error/message.js create mode 100644 ui/core/app/templates/scopes/scope/error.hbs create mode 100644 ui/core/tests/integration/components/error/message-test.js diff --git a/ui/core/app/components/error/message.hbs b/ui/core/app/components/error/message.hbs new file mode 100644 index 000000000..7cc59f8c9 --- /dev/null +++ b/ui/core/app/components/error/message.hbs @@ -0,0 +1,4 @@ + + {{t this.description}} + {{t "actions.help"}} + \ No newline at end of file diff --git a/ui/core/app/components/error/message.js b/ui/core/app/components/error/message.js new file mode 100644 index 000000000..c949ba822 --- /dev/null +++ b/ui/core/app/components/error/message.js @@ -0,0 +1,104 @@ +import Message from 'rose/components/rose/message'; + +/* + * Helpful error booleans are attached based on the error status code: + * + * - 401 - `error.isUnauthenticated`: the session isn't authenticated + * - 403 - `error.isForbidden`: the session is authenticated but does not have + * permission to perform the requested action + * - 404 - `error.isNotFound`: the requested resource could not be found + * - 500 - `error.isServer`: an internal server error occurred + * + * For an unknown error state, i.e error state not matching to the above defined list: + * - unknown - `error.isUnknown`: an error occurred, but we don't know which or + * we don't distinguish it yet +*/ + +const status_messages = { + '401' : { + icon: 'disabled', + title: 'errors.titles.unauthenticated-error', + subtitle: 'errors.subtitles.unauthenticated-error', + description: 'errors.descriptions.unauthenticated-error', + }, + '403' : { + icon: 'disabled', + title: 'errors.titles.forbidden-error', + subtitle: 'errors.subtitles.forbidden-error', + description: 'errors.descriptions.forbidden-error', + }, + '404' : { + icon: 'help-circle-outline', + title: 'errors.titles.notfound-error', + subtitle: 'errors.subtitles.notfound-error', + description: 'errors.descriptions.notfound-error', + }, + '500' : { + icon: 'alert-circle-outline', + title: 'errors.titles.server-error', + subtitle: 'errors.subtitles.server-error', + description: 'errors.descriptions.server-error', + }, + 'unknown' : { + icon: 'alert-circle-outline', + title: 'errors.titles.unknown-error', + subtitle: 'errors.subtitles.unknown-error', + description: 'errors.descriptions.unknown-error', + } +}; + +export default class ErrorMessageComponent extends Message { + // =methods + + /** + * Returns an icon name based on error status. + * @return {string} + */ + get icon() { + return this.statusMessage.icon; + } + + /** + * Returns a message title based on error status. + * @return {string} + */ + get title() { + return this.statusMessage.title; + } + + /** + * Returns a message subtitle of error status prefixed with 'Error'. + * @return {string} + */ + get subtitle() { + return this.statusMessage.subtitle; + } + + /** + * Returns an error description based on error status. + * @return {string} + */ + get description() { + return this.statusMessage.description; + } + + /** + * Returns an route string for help text in message. + * @return {string} + */ + get helpRoute() { + return 'index' + } + + /** + * Returns an object with message details based on error status. + * Return message for `unkown` error when error status isn't part of + * predefined message set. + * @return {object} + */ + get statusMessage() { + let status = this.status; + if(!status_messages[status]) { status = 'unknown'; } + return status_messages[status]; + } +} \ No newline at end of file diff --git a/ui/core/app/routes/application.js b/ui/core/app/routes/application.js index 0a6c58271..1ee2b2ce2 100644 --- a/ui/core/app/routes/application.js +++ b/ui/core/app/routes/application.js @@ -50,8 +50,6 @@ export default class ApplicationRoute extends Route.extend( if (!Ember.testing) later(location, location.reload, 150); } - // =actions - /** * Delegates invalidation to the session service. */ diff --git a/ui/core/app/routes/scopes/scope.js b/ui/core/app/routes/scopes/scope.js index 820f7caf2..def116b51 100644 --- a/ui/core/app/routes/scopes/scope.js +++ b/ui/core/app/routes/scopes/scope.js @@ -33,4 +33,5 @@ export default class ScopesScopeRoute extends Route { ); }); } + } diff --git a/ui/core/app/templates/scopes/scope/error.hbs b/ui/core/app/templates/scopes/scope/error.hbs new file mode 100644 index 000000000..417feb0ea --- /dev/null +++ b/ui/core/app/templates/scopes/scope/error.hbs @@ -0,0 +1,6 @@ + + {{#each @model.errors as |error|}} + {{title error.status}} + + {{/each}} + \ No newline at end of file diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 7f2ea5ba0..846da6b99 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -1,6 +1,6 @@ import config from '../config/environment'; import { authHandler, deauthHandler } from './route-handlers/auth'; -//import { Response } from 'miragejs'; +import { Response } from 'miragejs'; export default function() { @@ -288,4 +288,18 @@ export default function() { } return target.update(updatedAttrs); }); + + /* Uncomment the following line and the Response import above + * Then change the response code to simulate error responses. + * this.get('/scopes', () => new Response(505)); + * + * Update error payload to simulate specific error responses. + */ + // this.get('/host-catalogs', () => new Response(404)); + // this.get('/scopes/:id', () => new Response(505, {}, { + // errors: [{ + // status: 505, + // message: 'HTTP version not supported.', + // }] + // })); } diff --git a/ui/core/tests/integration/components/error/message-test.js b/ui/core/tests/integration/components/error/message-test.js new file mode 100644 index 000000000..811a0a138 --- /dev/null +++ b/ui/core/tests/integration/components/error/message-test.js @@ -0,0 +1,26 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +module('Integration | Component | error/message', function(hooks) { + setupRenderingTest(hooks); + + test('it renders', async function(assert) { + // Set any properties with this.set('myProperty', 'value'); + // Handle any actions with this.set('myAction', function(val) { ... }); + + await render(hbs``); + + assert.equal(this.element.textContent.trim(), ''); + + // Template block usage: + await render(hbs` + + template block text + + `); + + assert.equal(this.element.textContent.trim(), 'template block text'); + }); +}); diff --git a/ui/core/translations/en-us.yaml b/ui/core/translations/en-us.yaml index 301ac40a2..756b98b98 100644 --- a/ui/core/translations/en-us.yaml +++ b/ui/core/translations/en-us.yaml @@ -162,6 +162,12 @@ errors: server-error: Server error unknown-error: Something went wrong authentication-failed: Authentication Failed + subtitles: + unauthenticated-error: Error 401 + forbidden-error: Error 403 + notfound-error: Error 404 + server-error: Error 500 + unknown-error: Error descriptions: unauthenticated-error: You are not authenticated. Please authenticate and try again later. forbidden-error: You must be granted permissions to view this data. Ask your administrator if you think you should have access. From cfd1ac81fbe7abbfbfd3e0d5bdc445f80ffd957f Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 16 Sep 2020 15:25:35 -0400 Subject: [PATCH 2/5] refactor: Update comments and tests for error message --- ui/core/app/components/error/message.hbs | 4 +- ui/core/app/components/error/message.js | 51 ++++++++++--------- ui/core/app/routes/scopes/scope.js | 1 - ui/core/mirage/config.js | 3 +- .../components/error/message-test.js | 45 ++++++++++------ 5 files changed, 62 insertions(+), 42 deletions(-) diff --git a/ui/core/app/components/error/message.hbs b/ui/core/app/components/error/message.hbs index 7cc59f8c9..200395481 100644 --- a/ui/core/app/components/error/message.hbs +++ b/ui/core/app/components/error/message.hbs @@ -1,4 +1,6 @@ {{t this.description}} - {{t "actions.help"}} + {{#if this.helpRoute}} + {{t "actions.help"}} + {{/if}} \ No newline at end of file diff --git a/ui/core/app/components/error/message.js b/ui/core/app/components/error/message.js index c949ba822..1c476e71f 100644 --- a/ui/core/app/components/error/message.js +++ b/ui/core/app/components/error/message.js @@ -1,50 +1,50 @@ import Message from 'rose/components/rose/message'; /* - * Helpful error booleans are attached based on the error status code: - * - * - 401 - `error.isUnauthenticated`: the session isn't authenticated - * - 403 - `error.isForbidden`: the session is authenticated but does not have - * permission to perform the requested action - * - 404 - `error.isNotFound`: the requested resource could not be found - * - 500 - `error.isServer`: an internal server error occurred - * - * For an unknown error state, i.e error state not matching to the above defined list: - * - unknown - `error.isUnknown`: an error occurred, but we don't know which or - * we don't distinguish it yet -*/ + * Helpful error booleans are attached based on the error status code: + * + * - 401 - `error.isUnauthenticated`: the session isn't authenticated + * - 403 - `error.isForbidden`: the session is authenticated but does not have + * permission to perform the requested action + * - 404 - `error.isNotFound`: the requested resource could not be found + * - 500 - `error.isServer`: an internal server error occurred + * + * For an unknown error state, i.e error state not matching to the above defined list: + * - unknown - `error.isUnknown`: an error occurred, but we don't know which or + * we don't distinguish it yet + */ const status_messages = { - '401' : { + '401': { icon: 'disabled', title: 'errors.titles.unauthenticated-error', subtitle: 'errors.subtitles.unauthenticated-error', description: 'errors.descriptions.unauthenticated-error', }, - '403' : { + '403': { icon: 'disabled', title: 'errors.titles.forbidden-error', subtitle: 'errors.subtitles.forbidden-error', description: 'errors.descriptions.forbidden-error', }, - '404' : { + '404': { icon: 'help-circle-outline', title: 'errors.titles.notfound-error', subtitle: 'errors.subtitles.notfound-error', description: 'errors.descriptions.notfound-error', }, - '500' : { + '500': { icon: 'alert-circle-outline', title: 'errors.titles.server-error', subtitle: 'errors.subtitles.server-error', description: 'errors.descriptions.server-error', }, - 'unknown' : { + unknown: { icon: 'alert-circle-outline', title: 'errors.titles.unknown-error', subtitle: 'errors.subtitles.unknown-error', description: 'errors.descriptions.unknown-error', - } + }, }; export default class ErrorMessageComponent extends Message { @@ -82,23 +82,26 @@ export default class ErrorMessageComponent extends Message { return this.statusMessage.description; } + // Disable until help documentation can be linked in. /** * Returns an route string for help text in message. * @return {string} */ - get helpRoute() { - return 'index' - } + // get helpRoute() { + // return 'index' + // } /** * Returns an object with message details based on error status. - * Return message for `unkown` error when error status isn't part of + * Return message for `unknown` error when error status isn't part of * predefined message set. * @return {object} */ get statusMessage() { let status = this.status; - if(!status_messages[status]) { status = 'unknown'; } + if (!status_messages[status]) { + status = 'unknown'; + } return status_messages[status]; } -} \ No newline at end of file +} diff --git a/ui/core/app/routes/scopes/scope.js b/ui/core/app/routes/scopes/scope.js index def116b51..820f7caf2 100644 --- a/ui/core/app/routes/scopes/scope.js +++ b/ui/core/app/routes/scopes/scope.js @@ -33,5 +33,4 @@ export default class ScopesScopeRoute extends Route { ); }); } - } diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 846da6b99..e0f1cfce3 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -1,6 +1,7 @@ import config from '../config/environment'; import { authHandler, deauthHandler } from './route-handlers/auth'; -import { Response } from 'miragejs'; +// Uncomment to use in simulating error responses +// import { Response } from 'miragejs'; export default function() { diff --git a/ui/core/tests/integration/components/error/message-test.js b/ui/core/tests/integration/components/error/message-test.js index 811a0a138..e4e6b1428 100644 --- a/ui/core/tests/integration/components/error/message-test.js +++ b/ui/core/tests/integration/components/error/message-test.js @@ -1,26 +1,41 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; +import { render, find } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; -module('Integration | Component | error/message', function(hooks) { +module('Integration | Component | error/message', function (hooks) { setupRenderingTest(hooks); - test('it renders', async function(assert) { - // Set any properties with this.set('myProperty', 'value'); - // Handle any actions with this.set('myAction', function(val) { ... }); + test('it renders predefined message for known error status', async function (assert) { + this.set('status', '401'); - await render(hbs``); + await render(hbs``); + assert.equal( + find('.rose-message-title').textContent.trim(), + 'You are not signed in' + ); + assert.equal( + find('.rose-message-subtitle').textContent.trim(), + 'Error 401' + ); + assert.equal( + find('.rose-message-description').textContent.trim(), + 'You are not authenticated. Please authenticate and try again later.' + ); + }); - assert.equal(this.element.textContent.trim(), ''); + test('it renders default error for undefined error status', async function (assert) { + this.set('status', '510'); - // Template block usage: - await render(hbs` - - template block text - - `); - - assert.equal(this.element.textContent.trim(), 'template block text'); + await render(hbs``); + assert.equal( + find('.rose-message-title').textContent.trim(), + 'Something went wrong' + ); + assert.equal(find('.rose-message-subtitle').textContent.trim(), 'Error'); + assert.equal( + find('.rose-message-description').textContent.trim(), + 'Please contact your administrator or try again later.' + ); }); }); From 5b7bbf979ef7a9c77930729cc3ec93ea45192f68 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 16 Sep 2020 15:29:45 -0400 Subject: [PATCH 3/5] style: Add in accidentally deleted actions jsdoc notation --- ui/core/app/routes/application.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/core/app/routes/application.js b/ui/core/app/routes/application.js index 1ee2b2ce2..0a6c58271 100644 --- a/ui/core/app/routes/application.js +++ b/ui/core/app/routes/application.js @@ -50,6 +50,8 @@ export default class ApplicationRoute extends Route.extend( if (!Ember.testing) later(location, location.reload, 150); } + // =actions + /** * Delegates invalidation to the session service. */ From 0690620a8ef1bc6d8e132da029cefec2b2047564 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 18 Sep 2020 12:00:52 -0400 Subject: [PATCH 4/5] refactor: Simplify error message component --- ui/core/app/components/error/message.hbs | 4 +- ui/core/app/components/error/message.js | 92 +++++-------------- ui/core/app/templates/scopes/scope/error.hbs | 2 +- .../components/error/message-test.js | 12 +-- ui/core/translations/en-us.yaml | 33 ++++--- 5 files changed, 46 insertions(+), 97 deletions(-) diff --git a/ui/core/app/components/error/message.hbs b/ui/core/app/components/error/message.hbs index 200395481..54813a844 100644 --- a/ui/core/app/components/error/message.hbs +++ b/ui/core/app/components/error/message.hbs @@ -1,5 +1,5 @@ - - {{t this.description}} + + {{t (concat "errors." statusCode ".description")}} {{#if this.helpRoute}} {{t "actions.help"}} {{/if}} diff --git a/ui/core/app/components/error/message.js b/ui/core/app/components/error/message.js index 1c476e71f..7ae6ca0d1 100644 --- a/ui/core/app/components/error/message.js +++ b/ui/core/app/components/error/message.js @@ -1,4 +1,5 @@ import Message from 'rose/components/rose/message'; +import { computed } from '@ember/object'; /* * Helpful error booleans are attached based on the error status code: @@ -14,75 +15,29 @@ import Message from 'rose/components/rose/message'; * we don't distinguish it yet */ -const status_messages = { - '401': { - icon: 'disabled', - title: 'errors.titles.unauthenticated-error', - subtitle: 'errors.subtitles.unauthenticated-error', - description: 'errors.descriptions.unauthenticated-error', - }, - '403': { - icon: 'disabled', - title: 'errors.titles.forbidden-error', - subtitle: 'errors.subtitles.forbidden-error', - description: 'errors.descriptions.forbidden-error', - }, - '404': { - icon: 'help-circle-outline', - title: 'errors.titles.notfound-error', - subtitle: 'errors.subtitles.notfound-error', - description: 'errors.descriptions.notfound-error', - }, - '500': { - icon: 'alert-circle-outline', - title: 'errors.titles.server-error', - subtitle: 'errors.subtitles.server-error', - description: 'errors.descriptions.server-error', - }, - unknown: { - icon: 'alert-circle-outline', - title: 'errors.titles.unknown-error', - subtitle: 'errors.subtitles.unknown-error', - description: 'errors.descriptions.unknown-error', - }, -}; +const statuses = ['401', '403', '404', '500']; export default class ErrorMessageComponent extends Message { // =methods /** - * Returns an icon name based on error status. + * Returns an icon for error status. * @return {string} */ + @computed('status') get icon() { - return this.statusMessage.icon; + switch (this.status) { + case '401': + case '403': + return 'disabled'; + case '404': + return 'help-circle-outline'; + default: + return 'alert-circle-outline'; + } } - /** - * Returns a message title based on error status. - * @return {string} - */ - get title() { - return this.statusMessage.title; - } - - /** - * Returns a message subtitle of error status prefixed with 'Error'. - * @return {string} - */ - get subtitle() { - return this.statusMessage.subtitle; - } - - /** - * Returns an error description based on error status. - * @return {string} - */ - get description() { - return this.statusMessage.description; - } - - // Disable until help documentation can be linked in. + // TODO: Enable when help documentation can be linked in. /** * Returns an route string for help text in message. * @return {string} @@ -92,16 +47,15 @@ export default class ErrorMessageComponent extends Message { // } /** - * Returns an object with message details based on error status. - * Return message for `unknown` error when error status isn't part of - * predefined message set. - * @return {object} + * Returns 'unknown' status code when provided error status + * isn't part of predefined statuses. + * @return {string} */ - get statusMessage() { - let status = this.status; - if (!status_messages[status]) { - status = 'unknown'; - } - return status_messages[status]; + @computed('status') + get statusCode() { + let statusCode = this.status; + if(!statuses.includes(statusCode)) statusCode = 'unknown'; + return statusCode; } + } diff --git a/ui/core/app/templates/scopes/scope/error.hbs b/ui/core/app/templates/scopes/scope/error.hbs index 417feb0ea..8d56c81ce 100644 --- a/ui/core/app/templates/scopes/scope/error.hbs +++ b/ui/core/app/templates/scopes/scope/error.hbs @@ -1,6 +1,6 @@ +{{title (t "errors.titles.error") }} {{#each @model.errors as |error|}} - {{title error.status}} {{/each}} \ No newline at end of file diff --git a/ui/core/tests/integration/components/error/message-test.js b/ui/core/tests/integration/components/error/message-test.js index e4e6b1428..2671c9b7e 100644 --- a/ui/core/tests/integration/components/error/message-test.js +++ b/ui/core/tests/integration/components/error/message-test.js @@ -7,9 +7,7 @@ module('Integration | Component | error/message', function (hooks) { setupRenderingTest(hooks); test('it renders predefined message for known error status', async function (assert) { - this.set('status', '401'); - - await render(hbs``); + await render(hbs``); assert.equal( find('.rose-message-title').textContent.trim(), 'You are not signed in' @@ -24,15 +22,13 @@ module('Integration | Component | error/message', function (hooks) { ); }); - test('it renders default error for undefined error status', async function (assert) { - this.set('status', '510'); - - await render(hbs``); + test('it renders default error for unknown error status', async function (assert) { + await render(hbs``); assert.equal( find('.rose-message-title').textContent.trim(), 'Something went wrong' ); - assert.equal(find('.rose-message-subtitle').textContent.trim(), 'Error'); + assert.equal(find('.rose-message-subtitle').textContent.trim(), 'Error 599'); assert.equal( find('.rose-message-description').textContent.trim(), 'Please contact your administrator or try again later.' diff --git a/ui/core/translations/en-us.yaml b/ui/core/translations/en-us.yaml index f5708a5a0..fdfb779f9 100644 --- a/ui/core/translations/en-us.yaml +++ b/ui/core/translations/en-us.yaml @@ -158,25 +158,24 @@ form: current_password: Current Password new_password: New Password errors: + 401: + title: You are not signed in + description: You are not authenticated. Please authenticate and try again later. + 403: + title: You are not authorized + description: You must be granted permissions to view this data. Ask your administrator if you think you should have access. + 404: + title: Resource not found + description: We could not find the requested resource. You can ask your administrator or try again later. + 500: + title: Server error + description: We ran into a problem and could not continue. You can ask your administrator or try again later. + unknown: + title: Something went wrong + description: Please contact your administrator or try again later. titles: - unauthenticated-error: You are not signed in - forbidden-error: You are not authorized - notfound-error: Resource not found - server-error: Server error - unknown-error: Something went wrong authentication-failed: Authentication Failed - subtitles: - unauthenticated-error: Error 401 - forbidden-error: Error 403 - notfound-error: Error 404 - server-error: Error 500 - unknown-error: Error - descriptions: - unauthenticated-error: You are not authenticated. Please authenticate and try again later. - forbidden-error: You must be granted permissions to view this data. Ask your administrator if you think you should have access. - notfound-error: We could not find the requested resource. You can ask your administrator or try again later. - server-error: We ran into a problem and could not continue. You can ask your administrator or try again later. - unknown-error: Please contact your administrator or try again later. + error: Error notify: create-success: Created succesfully. save-success: Saved succesfully. From 8768118aba49cfff0b319a39a1a8d576aa2d411a Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 18 Sep 2020 14:22:54 -0400 Subject: [PATCH 5/5] fix: Error message component lint errors --- ui/core/app/components/error/message.hbs | 4 ++-- ui/core/app/components/error/message.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ui/core/app/components/error/message.hbs b/ui/core/app/components/error/message.hbs index 54813a844..90c471554 100644 --- a/ui/core/app/components/error/message.hbs +++ b/ui/core/app/components/error/message.hbs @@ -1,5 +1,5 @@ - - {{t (concat "errors." statusCode ".description")}} + + {{t (concat "errors." this.messageCode ".description")}} {{#if this.helpRoute}} {{t "actions.help"}} {{/if}} diff --git a/ui/core/app/components/error/message.js b/ui/core/app/components/error/message.js index 7ae6ca0d1..21b362c5f 100644 --- a/ui/core/app/components/error/message.js +++ b/ui/core/app/components/error/message.js @@ -52,10 +52,10 @@ export default class ErrorMessageComponent extends Message { * @return {string} */ @computed('status') - get statusCode() { - let statusCode = this.status; - if(!statuses.includes(statusCode)) statusCode = 'unknown'; - return statusCode; + get messageCode() { + let messageCode = this.status; + if(!statuses.includes(messageCode)) messageCode = 'unknown'; + return messageCode; } }