-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Specialized Job Detail Pages #3829
Changes from all commits
37c39a9
0c50472
552b958
eae275f
525312d
c6369a6
a47835f
ec35b5a
bb1e0d6
0ea1d0d
b4bdc61
ab22e95
81273dc
fbd166b
ec49a72
3fc0910
d7b9283
5238052
60cb1ac
dc0fa16
d264c43
35f388a
98b0068
3907f39
e8606f7
88f1349
80c2acd
3ca0d64
82de007
1671786
6e30864
b7f57ec
de27385
9e606f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { computed } from '@ember/object'; | ||
import DistributionBar from './distribution-bar'; | ||
|
||
export default DistributionBar.extend({ | ||
layoutName: 'components/distribution-bar', | ||
|
||
job: null, | ||
|
||
'data-test-children-status-bar': true, | ||
|
||
data: computed('job.{pendingChildren,runningChildren,deadChildren}', function() { | ||
if (!this.get('job')) { | ||
return []; | ||
} | ||
|
||
const children = this.get('job').getProperties( | ||
'pendingChildren', | ||
'runningChildren', | ||
'deadChildren' | ||
); | ||
return [ | ||
{ label: 'Pending', value: children.pendingChildren, className: 'queued' }, | ||
{ label: 'Running', value: children.runningChildren, className: 'running' }, | ||
{ label: 'Dead', value: children.deadChildren, className: 'complete' }, | ||
]; | ||
}), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import Component from '@ember/component'; | ||
import { computed } from '@ember/object'; | ||
import { inject as service } from '@ember/service'; | ||
|
||
export default Component.extend({ | ||
system: service(), | ||
|
||
job: null, | ||
|
||
// Provide a value that is bound to a query param | ||
sortProperty: null, | ||
sortDescending: null, | ||
|
||
// Provide actions that require routing | ||
onNamespaceChange() {}, | ||
gotoTaskGroup() {}, | ||
gotoJob() {}, | ||
|
||
breadcrumbs: computed('job.{name,id}', function() { | ||
const job = this.get('job'); | ||
return [ | ||
{ label: 'Jobs', args: ['jobs'] }, | ||
{ | ||
label: job.get('name'), | ||
args: ['jobs.job', job], | ||
}, | ||
]; | ||
}), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again wondering if composition is an option here. All of the templates have the same breadcrumbs markup, so if this logic was pulled into a component, there might be no need for an abstract parent class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactoring all of breadcrumbs is in my top three things to do. I'd prefer to leave this the way it is, understanding full well that it is strange and out of place, and clean it up when I refactor all of breadcrumbs. |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import AbstractJobPage from './abstract'; | ||
|
||
export default AbstractJobPage.extend(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { computed } from '@ember/object'; | ||
import { alias } from '@ember/object/computed'; | ||
import PeriodicChildJobPage from './periodic-child'; | ||
|
||
export default PeriodicChildJobPage.extend({ | ||
payload: alias('job.decodedPayload'), | ||
payloadJSON: computed('payload', function() { | ||
let json; | ||
try { | ||
json = JSON.parse(this.get('payload')); | ||
} catch (e) { | ||
// Swallow error and fall back to plain text rendering | ||
} | ||
return json; | ||
}), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import AbstractJobPage from './abstract'; | ||
|
||
export default AbstractJobPage.extend(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import Component from '@ember/component'; | ||
import { computed } from '@ember/object'; | ||
import { alias } from '@ember/object/computed'; | ||
import Sortable from 'nomad-ui/mixins/sortable'; | ||
|
||
export default Component.extend(Sortable, { | ||
job: null, | ||
|
||
classNames: ['boxed-section'], | ||
|
||
// Provide a value that is bound to a query param | ||
sortProperty: null, | ||
sortDescending: null, | ||
currentPage: null, | ||
|
||
// Provide an action with access to the router | ||
gotoJob() {}, | ||
|
||
pageSize: 10, | ||
|
||
taskGroups: computed('job.taskGroups.[]', function() { | ||
return this.get('job.taskGroups') || []; | ||
}), | ||
|
||
children: computed('job.children.[]', function() { | ||
return this.get('job.children') || []; | ||
}), | ||
|
||
listToSort: alias('children'), | ||
sortedChildren: alias('listSorted'), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import Component from '@ember/component'; | ||
import { computed } from '@ember/object'; | ||
|
||
export default Component.extend({ | ||
job: null, | ||
|
||
classNames: ['boxed-section'], | ||
|
||
sortedEvaluations: computed('[email protected]', function() { | ||
return (this.get('job.evaluations') || []).sortBy('modifyIndex').reverse(); | ||
}), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import Component from '@ember/component'; | ||
|
||
export default Component.extend({ | ||
job: null, | ||
tagName: '', | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import Component from '@ember/component'; | ||
|
||
export default Component.extend({ | ||
job: null, | ||
tagName: '', | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import Component from '@ember/component'; | ||
|
||
export default Component.extend({ | ||
job: null, | ||
|
||
classNames: ['boxed-section'], | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import Component from '@ember/component'; | ||
import { computed } from '@ember/object'; | ||
import { alias } from '@ember/object/computed'; | ||
import Sortable from 'nomad-ui/mixins/sortable'; | ||
|
||
export default Component.extend(Sortable, { | ||
job: null, | ||
|
||
classNames: ['boxed-section'], | ||
|
||
// Provide a value that is bound to a query param | ||
sortProperty: null, | ||
sortDescending: null, | ||
|
||
// Provide an action with access to the router | ||
gotoTaskGroup() {}, | ||
|
||
taskGroups: computed('job.taskGroups.[]', function() { | ||
return this.get('job.taskGroups') || []; | ||
}), | ||
|
||
listToSort: alias('taskGroups'), | ||
sortedTaskGroups: alias('listSorted'), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import AbstractJobPage from './abstract'; | ||
import { computed } from '@ember/object'; | ||
|
||
export default AbstractJobPage.extend({ | ||
breadcrumbs: computed('job.{name,id}', 'job.parent.{name,id}', function() { | ||
const job = this.get('job'); | ||
const parent = this.get('job.parent'); | ||
|
||
return [ | ||
{ label: 'Jobs', args: ['jobs'] }, | ||
{ | ||
label: parent.get('name'), | ||
args: ['jobs.job', parent], | ||
}, | ||
{ | ||
label: job.get('trimmedName'), | ||
args: ['jobs.job', job], | ||
}, | ||
]; | ||
}), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import AbstractJobPage from './abstract'; | ||
import { inject as service } from '@ember/service'; | ||
|
||
export default AbstractJobPage.extend({ | ||
store: service(), | ||
actions: { | ||
forceLaunch() { | ||
this.get('job') | ||
.forcePeriodic() | ||
.then(() => { | ||
this.get('store').findAll('job'); | ||
}); | ||
}, | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import AbstractJobPage from './abstract'; | ||
|
||
export default AbstractJobPage.extend(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { inject as service } from '@ember/service'; | ||
import { alias, filterBy } from '@ember/object/computed'; | ||
import { alias } from '@ember/object/computed'; | ||
import Controller, { inject as controller } from '@ember/controller'; | ||
import { computed } from '@ember/object'; | ||
import Sortable from 'nomad-ui/mixins/sortable'; | ||
|
@@ -11,10 +11,6 @@ export default Controller.extend(Sortable, Searchable, { | |
|
||
isForbidden: alias('jobsController.isForbidden'), | ||
|
||
pendingJobs: filterBy('model', 'status', 'pending'), | ||
runningJobs: filterBy('model', 'status', 'running'), | ||
deadJobs: filterBy('model', 'status', 'dead'), | ||
|
||
queryParams: { | ||
currentPage: 'page', | ||
searchTerm: 'search', | ||
|
@@ -30,16 +26,22 @@ export default Controller.extend(Sortable, Searchable, { | |
|
||
searchProps: computed(() => ['id', 'name']), | ||
|
||
/** | ||
Filtered jobs are those that match the selected namespace and aren't children | ||
of periodic or parameterized jobs. | ||
*/ | ||
filteredJobs: computed( | ||
'model.[]', | ||
'[email protected]', | ||
'system.activeNamespace', | ||
'system.namespaces.length', | ||
function() { | ||
if (this.get('system.namespaces.length')) { | ||
return this.get('model').filterBy('namespace.id', this.get('system.activeNamespace.id')); | ||
} else { | ||
return this.get('model'); | ||
} | ||
const hasNamespaces = this.get('system.namespaces.length'); | ||
const activeNamespace = this.get('system.activeNamespace.id'); | ||
|
||
return this.get('model') | ||
.filter(job => !hasNamespaces || job.get('namespace.id') === activeNamespace) | ||
.filter(job => !job.get('parent.content')); | ||
} | ||
), | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,9 @@ | ||
import { inject as service } from '@ember/service'; | ||
import { alias } from '@ember/object/computed'; | ||
import Controller, { inject as controller } from '@ember/controller'; | ||
import { computed } from '@ember/object'; | ||
import Sortable from 'nomad-ui/mixins/sortable'; | ||
import WithNamespaceResetting from 'nomad-ui/mixins/with-namespace-resetting'; | ||
|
||
export default Controller.extend(Sortable, WithNamespaceResetting, { | ||
export default Controller.extend(WithNamespaceResetting, { | ||
system: service(), | ||
jobController: controller('jobs.job'), | ||
|
||
|
@@ -16,28 +14,22 @@ export default Controller.extend(Sortable, WithNamespaceResetting, { | |
}, | ||
|
||
currentPage: 1, | ||
pageSize: 10, | ||
|
||
sortProperty: 'name', | ||
sortDescending: false, | ||
|
||
breadcrumbs: alias('jobController.breadcrumbs'), | ||
job: alias('model'), | ||
|
||
taskGroups: computed('model.taskGroups.[]', function() { | ||
return this.get('model.taskGroups') || []; | ||
}), | ||
|
||
listToSort: alias('taskGroups'), | ||
sortedTaskGroups: alias('listSorted'), | ||
|
||
sortedEvaluations: computed('[email protected]', function() { | ||
return (this.get('model.evaluations') || []).sortBy('modifyIndex').reverse(); | ||
}), | ||
|
||
actions: { | ||
gotoTaskGroup(taskGroup) { | ||
this.transitionToRoute('jobs.job.task-group', taskGroup.get('job'), taskGroup); | ||
}, | ||
|
||
gotoJob(job) { | ||
this.transitionToRoute('jobs.job', job, { | ||
queryParams: { jobNamespace: job.get('namespace.name') }, | ||
}); | ||
}, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be misreading this, but would it be possible to use composition instead of inheritance here? That is, have this component (and the other subclass) render
{{distribution-bar data=data}}
?It would mean rendering another component, but it might be clearer to understand the code that way. It took me a while to figure out how this subclass interacted with the parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely agree it would be clearer and more idiomatic. I just still have a gut aversion to making extra components. Especially when those components are in lists.
There are no lists in Nomad long enough to cause component allocation to drag the app though, so I shouldn't prematurely optimize.