UI: Fix exec popup link for job id ≠ name (#7815)

This closes #7814. It makes URL-generation more central and changes the exec URL to include job id instead of name.
hashicorp · Apr 29, 2020 · d913f05 · d913f05
1 parent 5b8b86f
commit d913f05
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ BUG FIXES:
  * api: autoscaling policies should not be returned for stopped jobs [[GH-7768](https://github.com/hashicorp/nomad/issues/7768)]
  * core: job scale status endpoint was returning incorrect counts [[GH-7789](https://github.com/hashicorp/nomad/issues/7789)]
  * jobspec: autoscaling policy block should return a parsing error multiple `policy` blocks are provided [[GH-7716](https://github.com/hashicorp/nomad/issues/7716)]
+ * ui: Fixed a bug where exec popup had incorrect URL for jobs where name ≠ id [[GH-7814](https://github.com/hashicorp/nomad/issues/7814)]
 
 ## 0.11.1 (April 22, 2020)
 

diff --git a/ui/app/components/exec/open-button.js b/ui/app/components/exec/open-button.js
@@ -15,22 +15,11 @@ export default Component.extend({
   },
 
   generateUrl() {
-    let urlSegments = {
-      job: this.job.get('name'),
-    };
-
-    if (this.taskGroup) {
-      urlSegments.taskGroup = this.taskGroup.get('name');
-    }
-
-    if (this.task) {
-      urlSegments.task = this.task.get('name');
-    }
-
-    if (this.allocation) {
-      urlSegments.allocation = this.allocation.get('shortId');
-    }
-
-    return generateExecUrl(this.router, urlSegments);
+    return generateExecUrl(this.router, {
+      job: this.job,
+      taskGroup: this.taskGroup,
+      task: this.task,
+      allocation: this.task
+    });
   },
 });
diff --git a/ui/app/components/exec/task-group-parent.js b/ui/app/components/exec/task-group-parent.js
@@ -70,9 +70,9 @@ export default Component.extend({
 
     openInNewWindow(job, taskGroup, task) {
       let url = generateExecUrl(this.router, {
-        job: job.name,
-        taskGroup: taskGroup.name,
-        task: task.name,
+        job,
+        taskGroup,
+        task,
       });
 
       openExecUrl(url);

diff --git a/ui/app/templates/components/exec/task-group-parent.hbs b/ui/app/templates/components/exec/task-group-parent.hbs
@@ -13,7 +13,7 @@
             openInNewWindow=openInNewWindow}}
         </a>
       {{else}}
-        {{#link-to "exec.task-group.task" taskGroup.job.name taskGroup.name task.name class="task-item" data-test-task=true}}
+        {{#link-to "exec.task-group.task" taskGroup.job.plainId taskGroup.name task.name class="task-item" data-test-task=true}}
           {{exec/task-contents
             task=task
             active=(and currentRouteIsThisTaskGroup (eq task.name activeTaskName))

diff --git a/ui/app/utils/generate-exec-url.js b/ui/app/utils/generate-exec-url.js
@@ -2,17 +2,17 @@ export default function generateExecUrl(router, { job, taskGroup, task, allocati
   const queryParams = router.currentRoute.queryParams;
 
   if (task) {
-    return router.urlFor('exec.task-group.task', job, taskGroup, task, {
+    return router.urlFor('exec.task-group.task', job.plainId, taskGroup.name, task.name, {
       queryParams: {
-        allocation,
+        allocation: allocation.shortId,
         ...queryParams,
       },
     });
   } else if (taskGroup) {
-    return router.urlFor('exec.task-group', job, taskGroup, { queryParams });
+    return router.urlFor('exec.task-group', job.plainId, taskGroup.name, { queryParams });
   } else if (allocation) {
-    return router.urlFor('exec', job, { queryParams: { allocation, ...queryParams } });
+    return router.urlFor('exec', job.plainId, { queryParams: { allocation: allocation.shortId, ...queryParams } });
   } else {
-    return router.urlFor('exec', job, { queryParams });
+    return router.urlFor('exec', 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
@@ -11,13 +11,13 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
   });
 
   test('it generates an exec job URL', function(assert) {
-    generateExecUrl(this.router, { job: 'job-name' });
+    generateExecUrl(this.router, { job: { plainId: 'job-name' } });
 
     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: 'job-name', allocation: 'allocation-short-id' });
+    generateExecUrl(this.router, { job: { plainId: 'job-name' }, allocation: { shortId: 'allocation-short-id' } });
 
     assert.ok(
       this.urlForSpy.calledWith('exec', 'job-name', {
@@ -27,7 +27,7 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
   });
 
   test('it generates an exec task group URL', function(assert) {
-    generateExecUrl(this.router, { job: 'job-name', taskGroup: '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)
@@ -36,10 +36,10 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
 
   test('it generates an exec task URL', function(assert) {
     generateExecUrl(this.router, {
-      allocation: 'allocation-short-id',
-      job: 'job-name',
-      taskGroup: 'task-group-name',
-      task: 'task-name',
+      allocation: { shortId: 'allocation-short-id' },
+      job: { plainId: 'job-name' },
+      taskGroup: { name: 'task-group-name' },
+      task: { name: 'task-name' },
     });
 
     assert.ok(
@@ -59,7 +59,7 @@ module('Unit | Utility | generate-exec-url', function(hooks) {
       region: 'a-region',
     };
 
-    generateExecUrl(this.router, { job: 'job-name', allocation: 'id' });
+    generateExecUrl(this.router, { job: { plainId: 'job-name' }, allocation: { shortId: 'id' } });
 
     assert.ok(
       this.urlForSpy.calledWith('exec', 'job-name', {