Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 🐛 host modal renders whenever there is an error during connection #1936

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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