Add specificity to exec allocation URL generation

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. This is perhaps at or beyond the limit of where it makes sense to use non-Ember Data pseudo-models in the tests 😶
hashicorp · Jul 20, 2020 · b06403f · b06403f
1 parent 3400d77
commit b06403f
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' },
       })
     );