Add specificity to exec allocation URL generation (#8463)

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.
hashicorp · Jul 20, 2020 · 39d3174 · 39d3174
1 parent a4a5343
commit 39d3174
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 14 deletions.
diff --git 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
@@ -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' },
       })
     );