From 6efca8400be587563819762fc25c2604ae22c4a2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 14:41:54 -0700 Subject: [PATCH 1/6] Correctly wire up job relationships --- ui/mirage/factories/evaluation.js | 1 + ui/mirage/factories/job.js | 1 + ui/mirage/serializers/application.js | 2 ++ 3 files changed, 4 insertions(+) diff --git a/ui/mirage/factories/evaluation.js b/ui/mirage/factories/evaluation.js index 84b18419e075..d31fcb391fd7 100644 --- a/ui/mirage/factories/evaluation.js +++ b/ui/mirage/factories/evaluation.js @@ -106,5 +106,6 @@ function assignJob(evaluation, server) { const job = evaluation.jobId ? server.db.jobs.find(evaluation.jobId) : pickOne(server.db.jobs); evaluation.update({ jobId: job.id, + job_id: job.id, }); } diff --git a/ui/mirage/factories/job.js b/ui/mirage/factories/job.js index f2cfa9fadfcb..c799d14ef2b5 100644 --- a/ui/mirage/factories/job.js +++ b/ui/mirage/factories/job.js @@ -114,6 +114,7 @@ export default Factory.extend({ groupNames: groups.mapBy('name'), job, job_id: job.id, + JobID: job.id, namespace: job.namespace, }); diff --git a/ui/mirage/serializers/application.js b/ui/mirage/serializers/application.js index 6957d8991ce1..a39f8167e754 100644 --- a/ui/mirage/serializers/application.js +++ b/ui/mirage/serializers/application.js @@ -19,7 +19,9 @@ export default RestSerializer.extend({ } }, + keyForCollection: keyCase, keyForAttribute: keyCase, keyForRelationship: keyCase, + keyForRelationshipIds: keyCase, keyForEmbeddedRelationship: keyCase, }); From 54b1101989e274b9ea86ae2d43e02ba6a882af6f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 14:42:38 -0700 Subject: [PATCH 2/6] QueryParams primitive ported with a helper This QueryParams object is defined in Ember source, but it isn't public, which means there is no out of the box way to construct the query params arg for LinkTo in JavaScript --- ui/app/utils/classes/query-params.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 ui/app/utils/classes/query-params.js diff --git a/ui/app/utils/classes/query-params.js b/ui/app/utils/classes/query-params.js new file mode 100644 index 000000000000..4f4efc08dd24 --- /dev/null +++ b/ui/app/utils/classes/query-params.js @@ -0,0 +1,12 @@ +// Copied from source since it isn't exposed to import +// https://github.com/emberjs/ember.js/blob/v2.18.2/packages/ember-routing/lib/system/query_params.js +import EmberObject from '@ember/object'; + +const QueryParams = EmberObject.extend({ + isQueryParams: true, + values: null, +}); + +export const qpBuilder = values => QueryParams.create({ values }); + +export default QueryParams; From efeb5abbcbcd81603c4194dcbd61af3a5821badb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 14:47:09 -0700 Subject: [PATCH 3/6] Fix a bug where job links didn't always include the namespace QP --- ui/app/components/job-page/abstract.js | 9 ++++++++- ui/app/controllers/jobs/job.js | 9 ++++++++- ui/app/controllers/jobs/job/task-group.js | 10 +++++++++- ui/app/templates/jobs/job/task-group.hbs | 5 +---- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/ui/app/components/job-page/abstract.js b/ui/app/components/job-page/abstract.js index eb80479d786e..01358cf34809 100644 --- a/ui/app/components/job-page/abstract.js +++ b/ui/app/components/job-page/abstract.js @@ -1,6 +1,7 @@ import Component from '@ember/component'; import { computed } from '@ember/object'; import { inject as service } from '@ember/service'; +import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; export default Component.extend({ system: service(), @@ -22,7 +23,13 @@ export default Component.extend({ { label: 'Jobs', args: ['jobs'] }, { label: job.get('name'), - args: ['jobs.job', job], + args: [ + 'jobs.job', + job, + qpBuilder({ + jobNamespace: job.get('namespace.name') || 'default', + }), + ], }, ]; }), diff --git a/ui/app/controllers/jobs/job.js b/ui/app/controllers/jobs/job.js index 5b8865a11bec..d641d5d0a918 100644 --- a/ui/app/controllers/jobs/job.js +++ b/ui/app/controllers/jobs/job.js @@ -1,5 +1,6 @@ import Controller from '@ember/controller'; import { computed } from '@ember/object'; +import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; export default Controller.extend({ breadcrumbs: computed('model.{name,id}', function() { @@ -7,7 +8,13 @@ export default Controller.extend({ { label: 'Jobs', args: ['jobs'] }, { label: this.get('model.name'), - args: ['jobs.job', this.get('model')], + args: [ + 'jobs.job', + this.get('model'), + qpBuilder({ + jobNamespace: this.get('model.namespace.name') || 'default', + }), + ], }, ]; }), diff --git a/ui/app/controllers/jobs/job/task-group.js b/ui/app/controllers/jobs/job/task-group.js index 2e2ab25e72b7..1ace41f8fe81 100644 --- a/ui/app/controllers/jobs/job/task-group.js +++ b/ui/app/controllers/jobs/job/task-group.js @@ -4,6 +4,7 @@ import { computed } from '@ember/object'; import Sortable from 'nomad-ui/mixins/sortable'; import Searchable from 'nomad-ui/mixins/searchable'; import WithNamespaceResetting from 'nomad-ui/mixins/with-namespace-resetting'; +import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; export default Controller.extend(Sortable, Searchable, WithNamespaceResetting, { jobController: controller('jobs.job'), @@ -33,7 +34,14 @@ export default Controller.extend(Sortable, Searchable, WithNamespaceResetting, { breadcrumbs: computed('jobController.breadcrumbs.[]', 'model.{name}', function() { return this.get('jobController.breadcrumbs').concat([ - { label: this.get('model.name'), args: ['jobs.job.task-group', this.get('model.name')] }, + { + label: this.get('model.name'), + args: [ + 'jobs.job.task-group', + this.get('model.name'), + qpBuilder({ jobNamespace: this.get('model.job.namespace.name') || 'default' }), + ], + }, ]); }), diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index 2934d2c1a94c..f1ab0acb2d70 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -3,10 +3,7 @@
  • {{#link-to data-test-breadcrumb=breadcrumb.label - params=(append - breadcrumb.args - (query-params jobNamespace=(or model.namespace.id "default")) - )}} + params=breadcrumb.args}} {{breadcrumb.label}} {{/link-to}}
    • From 79ecf4793664695cf2588d507272bbb66448282e Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 14:58:22 -0700 Subject: [PATCH 4/6] Make allocation pages extend the job breadcrumb trail --- ui/app/controllers/allocations/allocation.js | 35 ++++++++++++++++++- .../allocations/allocation/index.js | 6 +++- ui/app/routes/allocations/allocation.js | 11 ++++-- .../allocations/allocation/index.hbs | 13 ++++--- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/ui/app/controllers/allocations/allocation.js b/ui/app/controllers/allocations/allocation.js index 75ceaaec3c18..6aa0e219ba74 100644 --- a/ui/app/controllers/allocations/allocation.js +++ b/ui/app/controllers/allocations/allocation.js @@ -1,3 +1,36 @@ import Controller from '@ember/controller'; +import { computed } from '@ember/object'; +import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; -export default Controller.extend({}); +export default Controller.extend({ + breadcrumbs: computed('model.job', function() { + return [ + { label: 'Jobs', args: ['jobs'] }, + { + label: this.get('model.job.name'), + args: [ + 'jobs.job', + this.get('model.job'), + qpBuilder({ + jobNamespace: this.get('model.namespace.name') || 'default', + }), + ], + }, + { + label: this.get('model.taskGroupName'), + args: [ + 'jobs.job.task-group', + this.get('model.job'), + this.get('model.taskGroupName'), + qpBuilder({ + jobNamespace: this.get('model.namespace.name') || 'default', + }), + ], + }, + { + label: this.get('model.shortId'), + args: ['allocations.allocation', this.get('model')], + }, + ]; + }), +}); diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 810d0f728296..4e156c69f5cf 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -1,8 +1,10 @@ import { alias } from '@ember/object/computed'; -import Controller from '@ember/controller'; +import Controller, { inject as controller } from '@ember/controller'; import Sortable from 'nomad-ui/mixins/sortable'; export default Controller.extend(Sortable, { + allocationController: controller('allocations.allocation'), + queryParams: { sortProperty: 'sort', sortDescending: 'desc', @@ -11,6 +13,8 @@ export default Controller.extend(Sortable, { sortProperty: 'name', sortDescending: false, + breadcrumbs: alias('allocationController.breadcrumbs'), + listToSort: alias('model.states'), sortedStates: alias('listSorted'), }); diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index 3f46faa2f7cf..b09dc1cb376f 100644 --- a/ui/app/routes/allocations/allocation.js +++ b/ui/app/routes/allocations/allocation.js @@ -1,14 +1,21 @@ import Route from '@ember/routing/route'; -import WithModelErrorHandling from 'nomad-ui/mixins/with-model-error-handling'; import { collect } from '@ember/object/computed'; import { watchRecord } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; +import notifyError from 'nomad-ui/utils/notify-error'; -export default Route.extend(WithModelErrorHandling, WithWatchers, { +export default Route.extend(WithWatchers, { startWatchers(controller, model) { controller.set('watcher', this.get('watch').perform(model)); }, + model() { + // Preload the job for the allocation since it's required for the breadcrumb trail + return this._super(...arguments) + .then(allocation => allocation.get('job').then(() => allocation)) + .catch(notifyError(this)); + }, + watch: watchRecord('allocation'), watchers: collect('watch'), diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 659329b6545a..77099f6b0ed6 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -1,8 +1,13 @@ {{#global-header class="page-header"}} -
  • Allocations
    • -
  • - {{#link-to "allocations.allocation" model}}{{model.shortId}}{{/link-to}} -
    • + {{#each breadcrumbs as |breadcrumb index|}} +
  • + {{#link-to + data-test-breadcrumb=breadcrumb.label + params=breadcrumb.args}} + {{breadcrumb.label}} + {{/link-to}} +
    • + {{/each}} {{/global-header}} {{#gutter-menu class="page-body"}}
    From c8693ba5250d542752e8fec46b8f1f006f4124c1 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 14:59:03 -0700 Subject: [PATCH 5/6] Make task page breadcrumbs extend the allocation breadcrumbs --- .../allocations/allocation/task.js | 15 ++++++++++++++ .../allocations/allocation/task/index.js | 8 ++++++-- .../allocations/allocation/task/logs.js | 7 +++++++ .../allocations/allocation/task/index.hbs | 20 +++++++++---------- .../allocations/allocation/task/logs.hbs | 20 +++++++++---------- 5 files changed, 46 insertions(+), 24 deletions(-) create mode 100644 ui/app/controllers/allocations/allocation/task.js create mode 100644 ui/app/controllers/allocations/allocation/task/logs.js diff --git a/ui/app/controllers/allocations/allocation/task.js b/ui/app/controllers/allocations/allocation/task.js new file mode 100644 index 000000000000..616df57a3a5d --- /dev/null +++ b/ui/app/controllers/allocations/allocation/task.js @@ -0,0 +1,15 @@ +import Controller, { inject as controller } from '@ember/controller'; +import { computed } from '@ember/object'; + +export default Controller.extend({ + allocationController: controller('allocations.allocation'), + + breadcrumbs: computed('allocationController.breadcrumbs.[]', 'model.name', function() { + return this.get('allocationController.breadcrumbs').concat([ + { + label: this.get('model.name'), + args: ['allocations.allocation.task', this.get('model.allocation'), this.get('model')], + }, + ]); + }), +}); diff --git a/ui/app/controllers/allocations/allocation/task/index.js b/ui/app/controllers/allocations/allocation/task/index.js index 790e190ec6dd..e45f85230ee2 100644 --- a/ui/app/controllers/allocations/allocation/task/index.js +++ b/ui/app/controllers/allocations/allocation/task/index.js @@ -1,8 +1,12 @@ -import { alias } from '@ember/object/computed'; -import Controller from '@ember/controller'; +import Controller, { inject as controller } from '@ember/controller'; import { computed } from '@ember/object'; +import { alias } from '@ember/object/computed'; export default Controller.extend({ + taskController: controller('allocations.allocation.task'), + + breadcrumbs: alias('taskController.breadcrumbs'), + network: alias('model.resources.networks.firstObject'), ports: computed('network.reservedPorts.[]', 'network.dynamicPorts.[]', function() { return (this.get('network.reservedPorts') || []) diff --git a/ui/app/controllers/allocations/allocation/task/logs.js b/ui/app/controllers/allocations/allocation/task/logs.js new file mode 100644 index 000000000000..6ce1642b542e --- /dev/null +++ b/ui/app/controllers/allocations/allocation/task/logs.js @@ -0,0 +1,7 @@ +import Controller, { inject as controller } from '@ember/controller'; +import { alias } from '@ember/object/computed'; + +export default Controller.extend({ + taskController: controller('allocations.allocation.task'), + breadcrumbs: alias('taskController.breadcrumbs'), +}); diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 60a311bfac8f..46fdd49f97df 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -1,15 +1,13 @@ {{#global-header class="page-header"}} -
  • Allocations
    • -
  • - {{#link-to "allocations.allocation" model.allocation data-test-breadcrumb="allocation"}} - {{model.allocation.shortId}} - {{/link-to}} -
    • -
  • - {{#link-to "allocations.allocation.task" model.allocation model data-test-breadcrumb="task"}} - {{model.name}} - {{/link-to}} -
    • + {{#each breadcrumbs as |breadcrumb index|}} +
  • + {{#link-to + data-test-breadcrumb=breadcrumb.label + params=breadcrumb.args}} + {{breadcrumb.label}} + {{/link-to}} +
    • + {{/each}} {{/global-header}} {{#gutter-menu class="page-body"}} {{partial "allocations/allocation/task/subnav"}} diff --git a/ui/app/templates/allocations/allocation/task/logs.hbs b/ui/app/templates/allocations/allocation/task/logs.hbs index 01a7440cfc01..d4ee3da568b4 100644 --- a/ui/app/templates/allocations/allocation/task/logs.hbs +++ b/ui/app/templates/allocations/allocation/task/logs.hbs @@ -1,15 +1,13 @@ {{#global-header class="page-header"}} -
  • Allocations
    • -
  • - {{#link-to "allocations.allocation" model.allocation}} - {{model.allocation.shortId}} - {{/link-to}} -
    • -
  • - {{#link-to "allocations.allocation.task" model.allocation model}} - {{model.name}} - {{/link-to}} -
    • + {{#each breadcrumbs as |breadcrumb index|}} +
  • + {{#link-to + data-test-breadcrumb=breadcrumb.label + params=breadcrumb.args}} + {{breadcrumb.label}} + {{/link-to}} +
    • + {{/each}} {{/global-header}} {{#gutter-menu class="page-body"}} {{partial "allocations/allocation/task/subnav"}} From 7ac2b8ebd6d12c567371ed0b437ae833c180385d Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 13 Mar 2018 16:32:56 -0700 Subject: [PATCH 6/6] Update tests to reflect new breadcrumbs --- ui/tests/acceptance/task-detail-test.js | 69 ++++++++++++++++++++----- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index b79af1f57aa9..486dda09cc3f 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -31,34 +31,75 @@ test('/allocation/:id/:task_name should name the task and list high-level task i ); }); -test('breadcrumbs includes allocations and link to the allocation detail page', function(assert) { +test('breadcrumbs match jobs / job / task group / allocation / task', function(assert) { + const { jobId, taskGroup } = allocation; + const job = server.db.jobs.find(jobId); + + const shortId = allocation.id.split('-')[0]; + + assert.equal( + find('[data-test-breadcrumb="Jobs"]').textContent.trim(), + 'Jobs', + 'Jobs is the first breadcrumb' + ); assert.equal( - find('[data-test-breadcrumb="allocations"]').textContent.trim(), - 'Allocations', - 'Allocations is the first breadcrumb' + find(`[data-test-breadcrumb="${job.name}"]`).textContent.trim(), + job.name, + 'Job is the second breadcrumb' ); assert.equal( - find('[data-test-breadcrumb="allocations"]').getAttribute('href'), - '#', - "Allocations breadcrumb doesn't link anywhere" + find(`[data-test-breadcrumb="${taskGroup}`).textContent.trim(), + taskGroup, + 'Task Group is the third breadcrumb' ); assert.equal( - find('[data-test-breadcrumb="allocation"]').textContent.trim(), - allocation.id.split('-')[0], - 'Allocation short id is the second breadcrumb' + find(`[data-test-breadcrumb="${shortId}"]`).textContent.trim(), + shortId, + 'Allocation short id is the fourth breadcrumb' ); assert.equal( - find('[data-test-breadcrumb="task"]').textContent.trim(), + find(`[data-test-breadcrumb="${task.name}"]`).textContent.trim(), task.name, - 'Task name is the third breadcrumb' + 'Task name is the fifth breadcrumb' ); - click('[data-test-breadcrumb="allocation"]'); + click('[data-test-breadcrumb="Jobs"]'); + andThen(() => { + assert.equal(currentURL(), '/jobs', 'Jobs breadcrumb links correctly'); + }); + andThen(() => { + visit(`/allocations/${allocation.id}/${task.name}`); + }); + andThen(() => { + click(`[data-test-breadcrumb="${job.name}"]`); + }); + andThen(() => { + assert.equal(currentURL(), `/jobs/${job.id}`, 'Job breadcrumb links correctly'); + }); + andThen(() => { + visit(`/allocations/${allocation.id}/${task.name}`); + }); + andThen(() => { + click(`[data-test-breadcrumb="${taskGroup}"]`); + }); + andThen(() => { + assert.equal( + currentURL(), + `/jobs/${job.id}/${taskGroup}`, + 'Task Group breadcrumb links correctly' + ); + }); + andThen(() => { + visit(`/allocations/${allocation.id}/${task.name}`); + }); + andThen(() => { + click(`[data-test-breadcrumb="${shortId}"]`); + }); andThen(() => { assert.equal( currentURL(), `/allocations/${allocation.id}`, - 'Second breadcrumb links back to the allocation detail' + 'Allocations breadcrumb links correctly' ); }); });