From 3e224e570aace43493b66a4af29c61be991b8e1a Mon Sep 17 00:00:00 2001 From: Artur Kremens Date: Thu, 23 May 2019 18:14:03 +0200 Subject: [PATCH] Repo activation 409 handle (#2029) * Repo activation 409 handle https://github.com/travis-ci/bugs-questions-comments-ideas/issues/159 * Code review update + add specs * Update api-errors util --- app/components/repository-status-toggle.js | 10 ++- .../components/repository-status-toggle.hbs | 27 ++++---- app/utils/api-errors.js | 9 +++ .../repository-status-toggle-test.js | 65 ++++++++++++++++++- tests/unit/utils/api-errors-test.js | 19 ++++++ 5 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 app/utils/api-errors.js create mode 100644 tests/unit/utils/api-errors-test.js diff --git a/app/components/repository-status-toggle.js b/app/components/repository-status-toggle.js index f1c6cb0e5b..dff420c97d 100644 --- a/app/components/repository-status-toggle.js +++ b/app/components/repository-status-toggle.js @@ -3,6 +3,7 @@ import config from 'travis/config/environment'; import { task } from 'ember-concurrency'; import { computed } from '@ember/object'; import { inject as service } from '@ember/service'; +import hasErrorWithStatus from 'travis/utils/api-errors'; export default Component.extend({ externalLinks: service(), @@ -37,9 +38,8 @@ export default Component.extend({ }, resetErrors() { - return this.set('showError', false); + return this.set('apiError', null); } - }, toggleRepositoryTask: task(function* () { @@ -49,7 +49,11 @@ export default Component.extend({ yield repository.reload(); this.pusher.subscribe(`repo-${repository.id}`); } catch (error) { - this.set('showError', true); + this.set('apiError', error); } }), + + is409error: computed('apiError', function () { + return hasErrorWithStatus(this.apiError, '409'); + }) }); diff --git a/app/templates/components/repository-status-toggle.hbs b/app/templates/components/repository-status-toggle.hbs index dc2ac29a70..b0c94d4fed 100644 --- a/app/templates/components/repository-status-toggle.hbs +++ b/app/templates/components/repository-status-toggle.hbs @@ -30,20 +30,25 @@ {{/if}} {{/if}} -{{#if showError}} +{{#if apiError}}

- An error happened when we tried to alter settings on GitHub. - {{#if githubOrgsOauthAccessSettingsUrl}} - It may be caused by API restrictions, please - - review and add - your authorized Orgs. + {{#if is409error}} + Request cannot be completed because the repository ssh key is still pending to be created. + Please retry in a bit, or try syncing the repository if this condition does not resolve. + {{else}} + An error happened when we tried to alter settings on GitHub. + {{#if githubOrgsOauthAccessSettingsUrl}} + It may be caused by API restrictions, please + + review and add + your authorized Orgs. + {{/if}} + {{/if}} -

{{/if}} diff --git a/app/utils/api-errors.js b/app/utils/api-errors.js new file mode 100644 index 0000000000..df6f3fd39e --- /dev/null +++ b/app/utils/api-errors.js @@ -0,0 +1,9 @@ +/** + * Check if the status error occurs in the response errors collection + * + */ +export default function hasErrorWithStatus(errorResponse, status) { + const { errors = [] } = errorResponse || {}; + + return errors.isAny('status', status); +} diff --git a/tests/integration/components/repository-status-toggle-test.js b/tests/integration/components/repository-status-toggle-test.js index 842ad6430c..c0b1052d18 100644 --- a/tests/integration/components/repository-status-toggle-test.js +++ b/tests/integration/components/repository-status-toggle-test.js @@ -1,7 +1,8 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; +import { render, settled } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; +import { Response } from 'ember-cli-mirage'; module('RepositoryStatusToggleComponent', function (hooks) { setupRenderingTest(hooks); @@ -26,4 +27,66 @@ module('RepositoryStatusToggleComponent', function (hooks) { assert.dom('.switch').hasClass('active', 'switch should have active class'); }); + + test('should display correct error message on 409 fail', async function (assert) { + let store = this.owner.lookup('service:store'); + let repo = store.push({ + data: { + id: 10001, + type: 'repo', + attributes: { + slug: 'travis-ci/travis-web', + active: false, + permissions: { + admin: true + }, + } + } + }); + + this.set('repository', repo); + + server.post('/repo/:id/activate', (schema, request) => { + return new Response(409, {}, {}); + }); + + await render(hbs`{{repository-status-toggle repository=repository}}`); + assert.dom('.switch').findElement().click(); + settled().then(() => { + assert.dom('.repositories-error').hasText( + 'Request cannot be completed because the repository ssh key is still pending to be created. Please retry in a bit, or try syncing the repository if this condition does not resolve.' + ); + }); + }); + + test('should display correct error message on non 409 fail', async function (assert) { + let store = this.owner.lookup('service:store'); + let repo = store.push({ + data: { + id: 10001, + type: 'repo', + attributes: { + slug: 'travis-ci/travis-web', + active: false, + permissions: { + admin: true + }, + } + } + }); + + this.set('repository', repo); + + server.post('/repo/:id/activate', (schema, request) => { + return new Response(404, {}, {}); + }); + + await render(hbs`{{repository-status-toggle repository=repository}}`); + assert.dom('.switch').findElement().click(); + settled().then(() => { + assert.dom('.repositories-error').hasText( + 'An error happened when we tried to alter settings on GitHub. It may be caused by API restrictions, please review and add your authorized Orgs.' + ); + }); + }); }); diff --git a/tests/unit/utils/api-errors-test.js b/tests/unit/utils/api-errors-test.js new file mode 100644 index 0000000000..48633f7ed7 --- /dev/null +++ b/tests/unit/utils/api-errors-test.js @@ -0,0 +1,19 @@ +import hasErrorWithStatus from 'travis/utils/api-errors'; +import { module, test } from 'qunit'; + + +module('api-error', function () { + let error = { + errors: [ + { status: 409 } + ] + }; + + test('returns true if error exists', function (assert) { + assert.equal(hasErrorWithStatus(error, 409), true); + }); + + test('returns false if error does not exists', function (assert) { + assert.equal(hasErrorWithStatus(error, 404), false); + }); +});