From 39d3174207e79f075f123d3d8fd761b45de95540 Mon Sep 17 00:00:00 2001
From: Buck Doyle <buck@hashicorp.com>
Date: Mon, 20 Jul 2020 16:07:39 -0500
Subject: [PATCH] Add specificity to exec allocation URL generation (#8463)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Thanks to @notnoop for this UX improvement suggestion.
The allocation’s task group is always known, so it
might as well be preselected in the sidebar when the
exec window opens. Also, if the task group only has
one task, might as well preselect it too.
---
 ui/app/utils/generate-exec-url.js             | 39 ++++++++++++----
 ui/tests/unit/utils/generate-exec-url-test.js | 46 ++++++++++++++++---
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/ui/app/utils/generate-exec-url.js b/ui/app/utils/generate-exec-url.js
index 4773c5a037f1..c8621896be54 100644
--- a/ui/app/utils/generate-exec-url.js
+++ b/ui/app/utils/generate-exec-url.js
@@ -4,16 +4,39 @@ export default function generateExecUrl(router, { job, taskGroup, task, allocati
   const queryParams = router.currentRoute.queryParams;
 
   if (task) {
-    return router.urlFor('exec.task-group.task', get(job, 'plainId'), get(taskGroup, 'name'), get(task, 'name'), {
-      queryParams: {
-        allocation: get(allocation, 'shortId'),
-        ...queryParams,
-      },
-    });
+    return router.urlFor(
+      'exec.task-group.task',
+      get(job, 'plainId'),
+      get(taskGroup, 'name'),
+      get(task, 'name'),
+      {
+        queryParams: {
+          allocation: get(allocation, 'shortId'),
+          ...queryParams,
+        },
+      }
+    );
   } else if (taskGroup) {
-    return router.urlFor('exec.task-group', get(job, 'plainId'), get(taskGroup, 'name'), { queryParams });
+    return router.urlFor('exec.task-group', get(job, 'plainId'), get(taskGroup, 'name'), {
+      queryParams,
+    });
   } else if (allocation) {
-    return router.urlFor('exec', get(job, 'plainId'), { queryParams: { allocation: get(allocation, 'shortId'), ...queryParams } });
+    if (get(allocation, 'taskGroup.tasks.length') === 1) {
+      return router.urlFor(
+        'exec.task-group.task',
+        get(job, 'plainId'),
+        get(allocation, 'taskGroup.name'),
+        get(allocation, 'taskGroup.tasks.firstObject.name'),
+        { queryParams: { allocation: get(allocation, 'shortId'), ...queryParams } }
+      );
+    } else {
+      return router.urlFor(
+        'exec.task-group',
+        get(job, 'plainId'),
+        get(allocation, 'taskGroup.name'),
+        { queryParams: { allocation: get(allocation, 'shortId'), ...queryParams } }
+      );
+    }
   } else {
     return router.urlFor('exec', get(job, 'plainId'), { queryParams });
   }
diff --git a/ui/tests/unit/utils/generate-exec-url-test.js b/ui/tests/unit/utils/generate-exec-url-test.js
index 0ec05828b684..958508edc5ad 100644
--- a/ui/tests/unit/utils/generate-exec-url-test.js
+++ b/ui/tests/unit/utils/generate-exec-url-test.js
@@ -16,18 +16,49 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
     assert.ok(this.urlForSpy.calledWith('exec', 'job-name', emptyOptions));
   });
 
-  test('it generates an exec job URL with an allocation', function(assert) {
-    generateExecUrl(this.router, { job: { plainId: 'job-name' }, allocation: { shortId: 'allocation-short-id' } });
+  test('it generates an exec job URL with an allocation and task group when there are multiple tasks', function(assert) {
+    generateExecUrl(this.router, {
+      job: { plainId: 'job-name' },
+      allocation: {
+        shortId: 'allocation-short-id',
+        taskGroup: { name: 'task-group-name', tasks: [0, 1, 2] },
+      },
+    });
 
     assert.ok(
-      this.urlForSpy.calledWith('exec', 'job-name', {
+      this.urlForSpy.calledWith('exec.task-group', 'job-name', 'task-group-name', {
         queryParams: { allocation: 'allocation-short-id' },
       })
     );
   });
 
+  test('it generates an exec job URL with an allocation, task group, and task when there is only one task', function(assert) {
+    generateExecUrl(this.router, {
+      job: { plainId: 'job-name' },
+      allocation: {
+        shortId: 'allocation-short-id',
+        taskGroup: { name: 'task-group-name', tasks: [{ name: 'task-name' }] },
+      },
+    });
+
+    assert.ok(
+      this.urlForSpy.calledWith(
+        'exec.task-group.task',
+        'job-name',
+        'task-group-name',
+        'task-name',
+        {
+          queryParams: { allocation: 'allocation-short-id' },
+        }
+      )
+    );
+  });
+
   test('it generates an exec task group URL', function(assert) {
-    generateExecUrl(this.router, { job: { plainId: 'job-name' }, taskGroup: { name: 'task-group-name' } });
+    generateExecUrl(this.router, {
+      job: { plainId: 'job-name' },
+      taskGroup: { name: 'task-group-name' },
+    });
 
     assert.ok(
       this.urlForSpy.calledWith('exec.task-group', 'job-name', 'task-group-name', emptyOptions)
@@ -59,10 +90,13 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
       region: 'a-region',
     };
 
-    generateExecUrl(this.router, { job: { plainId: 'job-name' }, allocation: { shortId: 'id' } });
+    generateExecUrl(this.router, {
+      job: { plainId: 'job-name' },
+      allocation: { shortId: 'id', taskGroup: { name: 'task-group-name', tasks: [0, 1] } },
+    });
 
     assert.ok(
-      this.urlForSpy.calledWith('exec', 'job-name', {
+      this.urlForSpy.calledWith('exec.task-group', 'job-name', 'task-group-name', {
         queryParams: { allocation: 'id', namespace: 'a-namespace', region: 'a-region' },
       })
     );