From 1072be1eff00305f03add51566616cba537ccc9f Mon Sep 17 00:00:00 2001
From: Dharini Jeeva <dharini@hashicorp.com>
Date: Tue, 12 Apr 2022 14:05:16 -0700
Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20improve=20target=20c?=
 =?UTF-8?q?reation=20(#1050)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* refactor: 💡 improve target creation UX
---
 .../components/form/target/details/index.hbs  | 52 +++++++++++++------
 ui/admin/app/components/form/target/index.hbs |  1 +
 .../app/components/form/target/ssh/index.hbs  |  1 +
 .../app/components/form/target/tcp/index.hbs  |  1 +
 .../app/routes/scopes/scope/targets/new.js    | 29 +++++++++--
 .../templates/scopes/scope/targets/index.hbs  | 23 ++------
 .../templates/scopes/scope/targets/new.hbs    |  1 +
 .../tests/acceptance/targets/create-test.js   | 12 ++---
 8 files changed, 74 insertions(+), 46 deletions(-)

diff --git a/ui/admin/app/components/form/target/details/index.hbs b/ui/admin/app/components/form/target/details/index.hbs
index 41f8d8c7c..60f6071da 100644
--- a/ui/admin/app/components/form/target/details/index.hbs
+++ b/ui/admin/app/components/form/target/details/index.hbs
@@ -45,21 +45,43 @@
   <form.fieldset>
     <:title>{{t 'form.type.label'}}</:title>
     <:body>
-      <form.radioGroup
-        @name='types'
-        @selectedValue={{@model.type}}
-        @changed={{@changeType}}
-        as |radioGroup|
-      >
-        <radioGroup.card
-          @label={{t (concat 'resources.target.types.' @model.type)}}
-          @helperText={{t (concat 'resources.target.help.' @model.type)}}
-          @value={{@model.type}}
-          @layout='wide'
-          readonly={{true}}
-          @disabled={{true}}
-        />
-      </form.radioGroup>
+      {{#if @model.isNew}}
+        <form.radioGroup
+          @name='types'
+          @selectedValue={{@model.type}}
+          @changed={{@changeType}}
+          as |radioGroup|
+        >
+          <radioGroup.card
+            @label={{t 'resources.target.types.tcp'}}
+            @helperText={{t 'resources.target.help.tcp'}}
+            @value='tcp'
+            @layout='wide'
+          />
+          <radioGroup.card
+            @label={{t 'resources.target.types.ssh'}}
+            @helperText={{t 'resources.target.help.ssh'}}
+            @value='ssh'
+            @layout='wide'
+          />
+        </form.radioGroup>
+      {{else}}
+        <form.radioGroup
+          @name='types'
+          @selectedValue={{@model.type}}
+          as |radioGroup|
+        >
+          <radioGroup.card
+            @label={{t (concat 'resources.target.types.' @model.type)}}
+            @helperText={{t (concat 'resources.target.help.' @model.type)}}
+            @value={{@model.type}}
+            @layout='wide'
+            readonly={{true}}
+            @disabled={{true}}
+          />
+
+        </form.radioGroup>
+      {{/if}}
     </:body>
   </form.fieldset>
 
diff --git a/ui/admin/app/components/form/target/index.hbs b/ui/admin/app/components/form/target/index.hbs
index 35086d4a9..7f36017dd 100644
--- a/ui/admin/app/components/form/target/index.hbs
+++ b/ui/admin/app/components/form/target/index.hbs
@@ -3,4 +3,5 @@
   model=@model
   submit=@submit
   cancel=@cancel
+  changeType=@changeType
 }}
\ No newline at end of file
diff --git a/ui/admin/app/components/form/target/ssh/index.hbs b/ui/admin/app/components/form/target/ssh/index.hbs
index 7deff4078..fe08f1cf5 100644
--- a/ui/admin/app/components/form/target/ssh/index.hbs
+++ b/ui/admin/app/components/form/target/ssh/index.hbs
@@ -3,4 +3,5 @@
   @defaultPort='22'
   @submit={{@submit}}
   @cancel={{@cancel}}
+  @changeType={{@changeType}}
 />
\ No newline at end of file
diff --git a/ui/admin/app/components/form/target/tcp/index.hbs b/ui/admin/app/components/form/target/tcp/index.hbs
index 878c8e6f3..9dfa17de0 100644
--- a/ui/admin/app/components/form/target/tcp/index.hbs
+++ b/ui/admin/app/components/form/target/tcp/index.hbs
@@ -3,4 +3,5 @@
   @defaultPort='443'
   @submit={{@submit}}
   @cancel={{@cancel}}
+  @changeType={{@changeType}}
 />
\ No newline at end of file
diff --git a/ui/admin/app/routes/scopes/scope/targets/new.js b/ui/admin/app/routes/scopes/scope/targets/new.js
index 98fa5dd38..f5cf1893d 100644
--- a/ui/admin/app/routes/scopes/scope/targets/new.js
+++ b/ui/admin/app/routes/scopes/scope/targets/new.js
@@ -1,6 +1,11 @@
 import Route from '@ember/routing/route';
+import { action } from '@ember/object';
+import { inject as service } from '@ember/service';
 
 export default class ScopesScopeTargetsNewRoute extends Route {
+  // =services
+
+  @service router;
   // =methods
 
   // =attributes
@@ -11,17 +16,33 @@ export default class ScopesScopeTargetsNewRoute extends Route {
   };
   /**
    * Creates a new unsaved target in current scope.
+   * Also rollback/destroy any new, unsaved instances from this route before
+   * creating another, but reuse name/description if available.
    * @return {TargetModel}
    */
 
-  model(params) {
+  model({ type = 'tcp' }) {
     const scopeModel = this.modelFor('scopes.scope');
-    if (!params.type) {
-      params.type = 'tcp'; //tcp is the default type
+    let name, description;
+    if (this.currentModel?.isNew) {
+      ({ name, description } = this.currentModel);
+      this.currentModel.rollbackAttributes();
     }
+
     return this.store.createRecord('target', {
-      type: params.type,
       scopeModel,
+      type,
+      name,
+      description,
     });
   }
+
+  /**
+   * Update type of target
+   * @param {string} type
+   */
+  @action
+  changeType(type) {
+    this.router.replaceWith({ queryParams: { type } });
+  }
 }
diff --git a/ui/admin/app/templates/scopes/scope/targets/index.hbs b/ui/admin/app/templates/scopes/scope/targets/index.hbs
index 8215961d1..41b74500d 100644
--- a/ui/admin/app/templates/scopes/scope/targets/index.hbs
+++ b/ui/admin/app/templates/scopes/scope/targets/index.hbs
@@ -17,26 +17,9 @@
   <page.actions>
     {{#if (can 'create model' this.scope collection='targets')}}
       {{#if @model}}
-        <Rose::Dropdown
-          @text={{t 'titles.new'}}
-          @dropdownRight={{true}}
-          as |dropdown|
-        >
-          <dropdown.link
-            @route='scopes.scope.targets.new'
-            @query={{hash type='tcp'}}
-          >
-            {{t 'resources.target.types.tcp'}}
-          </dropdown.link>
-          {{#if (feature-flag 'ssh-target')}}
-            <dropdown.link
-              @route='scopes.scope.targets.new'
-              @query={{hash type='ssh'}}
-            >
-              {{t 'resources.target.types.ssh'}}
-            </dropdown.link>
-          {{/if}}
-        </Rose::Dropdown>
+        <Rose::LinkButton @route='scopes.scope.targets.new' @style='primary'>{{t
+            'titles.new'
+          }}</Rose::LinkButton>
       {{/if}}
     {{/if}}
   </page.actions>
diff --git a/ui/admin/app/templates/scopes/scope/targets/new.hbs b/ui/admin/app/templates/scopes/scope/targets/new.hbs
index c8d37f564..285da36b2 100644
--- a/ui/admin/app/templates/scopes/scope/targets/new.hbs
+++ b/ui/admin/app/templates/scopes/scope/targets/new.hbs
@@ -18,6 +18,7 @@
       @model={{@model}}
       @submit={{route-action 'save' @model}}
       @cancel={{route-action 'cancel' @model}}
+      @changeType={{route-action 'changeType'}}
     />
   </page.body>
 
diff --git a/ui/admin/tests/acceptance/targets/create-test.js b/ui/admin/tests/acceptance/targets/create-test.js
index 371f9e050..c62d5906f 100644
--- a/ui/admin/tests/acceptance/targets/create-test.js
+++ b/ui/admin/tests/acceptance/targets/create-test.js
@@ -74,7 +74,7 @@ module('Acceptance | targets | create', function (hooks) {
   test('defaults to a new TCP target when no query param provided', async function (assert) {
     assert.expect(1);
     await visit(urls.newTarget);
-    assert.equal(find('input[disabled]').value, 'tcp');
+    assert.equal(find('input[name=types]:checked').value, 'tcp');
   });
 
   test('can create new targets of type TCP', async function (assert) {
@@ -96,19 +96,18 @@ module('Acceptance | targets | create', function (hooks) {
   });
 
   test('can navigate to new targets route with proper authorization', async function (assert) {
-    assert.expect(3);
+    assert.expect(2);
     await visit(urls.targets);
     assert.ok(
       instances.scopes.project.authorized_collection_actions.targets.includes(
         'create'
       )
     );
-    assert.ok(find(`[href="${urls.newTCPTarget}"]`));
-    assert.ok(find(`[href="${urls.newSSHTarget}"]`));
+    assert.ok(find(`[href="${urls.newTarget}"]`));
   });
 
   test('cannot navigate to new targets route without proper authorization', async function (assert) {
-    assert.expect(3);
+    assert.expect(2);
     instances.scopes.project.authorized_collection_actions.targets = [];
     await visit(urls.targets);
     assert.notOk(
@@ -116,8 +115,7 @@ module('Acceptance | targets | create', function (hooks) {
         'create'
       )
     );
-    assert.notOk(find(`[href="${urls.newTCPTarget}"]`));
-    assert.notOk(find(`[href="${urls.newSSHTarget}"]`));
+    assert.notOk(find(`[href="${urls.newTarget}"]`));
   });
 
   test('cannot navigate to new SSH targets route when ssh feature is disabled', async function (assert) {