Skip to content

Commit

Permalink
fix: 🐛 host modal renders whenever there is an error during connection (
Browse files Browse the repository at this point in the history
#1936)

* fix: 🐛 host modal renders whenever there is an error

✅ Closes: ICU-11231

* test: 💍 add test case to make sure fix is working properly

* fix: 🐛 toggle flag back to false and remove @Tracked

* fix: 🐛 host modal should also close after host connection error

* refactor: 💡 change toggle to toggleModal

* test: 💍 rename toggle in test cases
  • Loading branch information
lisbet-alvarez authored Oct 2, 2023
1 parent 406e45f commit bac3593
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 18 deletions.
2 changes: 1 addition & 1 deletion ui/desktop/app/components/target/host-modal/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
}}

{{#if @showModal}}
<Hds::Modal data-test-host-modal @onClose={{@toggle}} as |M|>
<Hds::Modal data-test-host-modal @onClose={{fn @toggleModal false}} as |M|>
<M.Header @tagline={{@target.displayName}}>
{{t 'resources.session.actions.host'}}
</M.Header>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class ScopesScopeProjectsTargetsTargetController extends Controll
// =methods

@action
toggle() {
this.isConnecting = !this.isConnecting;
toggleModal(value) {
this.isConnecting = value;
}
}
45 changes: 43 additions & 2 deletions ui/desktop/app/routes/scopes/scope/projects/targets/target.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
@service router;
@service session;

// =attributes

isConnectionError = false;

// =methods

/**
Expand Down Expand Up @@ -65,6 +69,42 @@ export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
}
}

/**
* Sets the 'isConnecting' queryParam to false if connection failed.
* @returns {boolean}
*/
@action
didTransition() {
if (this.isConnectionError) {
/* eslint-disable-next-line ember/no-controller-access-in-routes */
const controller = this.controllerFor(
'scopes.scope.projects.targets.target'
);
controller.set('isConnecting', false);
this.isConnectionError = false;
}
return true;
}

/**
* Establish a session to current target.
* @param {TargetModel} target
* @param {HostModel} host
* hostConnect is only called when making a connection with a host and ensures that the host modal is automatically closed in the case of a connection error.
*/
@action
async hostConnect(target, host) {
await this.connect(target, host);
if (this.isConnectionError) {
/* eslint-disable-next-line ember/no-controller-access-in-routes */
const controller = this.controllerFor(
'scopes.scope.projects.targets.target'
);
controller.set('isConnecting', false);
this.isConnectionError = false;
}
}

/**
* Determine if we show host modal or quick connect based on target attributes.
* @param {TargetModel} model
Expand All @@ -75,7 +115,7 @@ export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
if (model.target.address || model.hosts.length === 1) {
await this.connect(model.target);
} else {
toggleModal();
toggleModal(true);
}
}

Expand Down Expand Up @@ -127,10 +167,11 @@ export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
session
);
} catch (e) {
this.isConnectionError = true;
this.confirm
.confirm(e.message, { isConnectError: true })
// Retry
.then(() => this.connect(model))
.then(() => this.connect(model, host))
.catch(() => null /* no op */);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@icon='entry-point'
@iconPosition='trailing'
disabled={{not (can 'initiateConnection target' @model.target)}}
{{on 'click' (route-action 'preConnect' @model this.toggle)}}
{{on 'click' (route-action 'preConnect' @model this.toggleModal)}}
/>
{{/if}}
</page.actions>
Expand All @@ -32,8 +32,8 @@
{{#if (can 'initiateConnection target' @model.target)}}
<Target::HostModal
@showModal={{this.isConnecting}}
@toggle={{this.toggle}}
@connect={{route-action 'connect'}}
@toggleModal={{this.toggleModal}}
@connect={{route-action 'hostConnect'}}
@hosts={{@model.hosts}}
@target={{@model.target}}
/>
Expand Down
15 changes: 15 additions & 0 deletions ui/desktop/tests/acceptance/projects/targets/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ module('Acceptance | projects | targets', function (hooks) {
const APP_STATE_TITLE = '.hds-application-state__title';
const TARGET_DETAILS_ROUTE_NAME =
'scopes.scope.projects.targets.target.index';
const ROSE_DIALOG_MODAL = '.rose-dialog-error';
const CHOOSE_HOST_MODAL = '[data-test-host-modal]';

const instances = {
scopes: {
Expand Down Expand Up @@ -240,4 +242,17 @@ module('Acceptance | projects | targets', function (hooks) {
assert.strictEqual(currentRouteName(), TARGET_DETAILS_ROUTE_NAME);
assert.dom(APP_STATE_TITLE).includesText('Cannot connect');
});

test('choose host modal does not render when there is a connect error', async function (assert) {
assert.expect(2);
const confirmService = this.owner.lookup('service:confirm');
confirmService.enabled = true;
await visit(urls.projects);

await click(`[href="${urls.targets}"]`);
await click(`[data-test-targets-connect-button="${instances.target.id}"]`);

assert.dom(ROSE_DIALOG_MODAL).exists();
assert.dom(CHOOSE_HOST_MODAL).doesNotExist();
});
});
20 changes: 10 additions & 10 deletions ui/desktop/tests/integration/components/target/host-modal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ module('Integration | Component | target/host-modal', function (hooks) {
let hostA;
let hostB;
let connect;
let toggle;
let toggleModal;

hooks.beforeEach(function () {
connect = () => {
this.set('called', true);
};
toggle = () => {
toggleModal = () => {
this.set('called', true);
};
store = this.owner.lookup('service:store');
Expand All @@ -47,13 +47,13 @@ module('Integration | Component | target/host-modal', function (hooks) {
test('it renders', async function (assert) {
assert.expect(1);
this.set('connect', connect);
this.set('toggle', toggle);
this.set('toggleModal', toggleModal);
this.set('hosts', [hostA, hostB]);
this.set('target', target);

await render(hbs`<Target::HostModal
@showModal={{true}}
@toggle={{this.toggle}}
@toggleModal={{this.toggleModal}}
@connect={{this.connect}}
@hosts={{this.hosts}}
@target={{this.target}} />`);
Expand All @@ -67,14 +67,14 @@ module('Integration | Component | target/host-modal', function (hooks) {
this.set('showModal', false);
};
this.set('connect', connect);
this.set('toggle', toggleModal);
this.set('toggleModal', toggleModal);
this.set('showModal', true);
this.set('hosts', [hostA, hostB]);
this.set('target', target);

await render(hbs`<Target::HostModal
@showModal={{this.showModal}}
@toggle={{this.toggle}}
@toggleModal={{this.toggleModal}}
@connect={{this.connect}}
@hosts={{this.hosts}}
@target={{this.target}} />`);
Expand All @@ -89,13 +89,13 @@ module('Integration | Component | target/host-modal', function (hooks) {
test('it calls connect when Quick Connect button clicked', async function (assert) {
assert.expect(1);
this.set('connect', connect);
this.set('toggle', toggle);
this.set('toggleModal', toggleModal);
this.set('hosts', [hostA, hostB]);
this.set('target', target);

await render(hbs`<Target::HostModal
@showModal={{true}}
@toggle={{this.toggle}}
@toggleModal={{this.toggleModal}}
@connect={{this.connect}}
@hosts={{this.hosts}}
@target={{this.target}} />`);
Expand All @@ -108,13 +108,13 @@ module('Integration | Component | target/host-modal', function (hooks) {
test('it calls connect when host item is clicked', async function (assert) {
assert.expect(1);
this.set('connect', connect);
this.set('toggle', toggle);
this.set('toggleModal', toggleModal);
this.set('hosts', [hostA, hostB]);
this.set('target', target);

await render(hbs`<Target::HostModal
@showModal={{true}}
@toggle={{this.toggle}}
@toggleModal={{this.toggleModal}}
@connect={{this.connect}}
@hosts={{this.hosts}}
@target={{this.target}} />`);
Expand Down

2 comments on commit bac3593

@vercel
Copy link

@vercel vercel bot commented on bac3593 Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

boundary-ui-desktop – ./ui/desktop

boundary-ui-desktop-git-main-hashicorp.vercel.app
boundary-ui-desktop.vercel.app
boundary-ui-desktop-hashicorp.vercel.app

@vercel
Copy link

@vercel vercel bot commented on bac3593 Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

boundary-ui – ./ui/admin

boundary-ui-git-main-hashicorp.vercel.app
boundary-ui-hashicorp.vercel.app
boundary-ui.vercel.app

Please sign in to comment.