-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix ACL requirements for job details UI #11672
Changes from all commits
eefcfe4
5c9b6af
c92304b
a11b171
0c04a38
3d8a615
f837114
cac1550
96bf92f
dde13e7
e5e6fea
fe643d1
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,3 @@ | ||
```release-note:bug | ||
ui: Fix the ACL requirements for displaying the job details page | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,5 @@ | ||
import AbstractJobPage from './abstract'; | ||
import classic from 'ember-classic-decorator'; | ||
import { inject as service } from '@ember/service'; | ||
import jobClientStatus from 'nomad-ui/utils/properties/job-client-status'; | ||
|
||
@classic | ||
export default class Sysbatch extends AbstractJobPage { | ||
@service store; | ||
|
||
@jobClientStatus('nodes', 'job') jobClientStatus; | ||
|
||
get nodes() { | ||
return this.store.peekAll('node'); | ||
} | ||
} | ||
export default class Sysbatch extends AbstractJobPage {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,5 @@ | ||
import AbstractJobPage from './abstract'; | ||
import classic from 'ember-classic-decorator'; | ||
import { inject as service } from '@ember/service'; | ||
import jobClientStatus from 'nomad-ui/utils/properties/job-client-status'; | ||
|
||
@classic | ||
export default class System extends AbstractJobPage { | ||
@service store; | ||
|
||
@jobClientStatus('nodes', 'job') jobClientStatus; | ||
|
||
get nodes() { | ||
return this.store.peekAll('node'); | ||
} | ||
} | ||
export default class System extends AbstractJobPage {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import Route from '@ember/routing/route'; | ||
import { inject as service } from '@ember/service'; | ||
import { collect } from '@ember/object/computed'; | ||
import { | ||
watchRecord, | ||
|
@@ -9,34 +10,49 @@ import { | |
import WithWatchers from 'nomad-ui/mixins/with-watchers'; | ||
|
||
export default class IndexRoute extends Route.extend(WithWatchers) { | ||
@service can; | ||
@service store; | ||
|
||
async model() { | ||
// Optimizing future node look ups by preemptively loading everything | ||
await this.store.findAll('node'); | ||
return this.modelFor('jobs.job'); | ||
const job = this.modelFor('jobs.job'); | ||
if (!job) { | ||
return { job, nodes: [] }; | ||
} | ||
|
||
// Optimizing future node look ups by preemptively loading all nodes if | ||
// necessary and allowed. | ||
if (this.can.can('read client') && job.get('hasClientStatus')) { | ||
await this.store.findAll('node'); | ||
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. Do users with write access not need this optimization for some reason? 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. I'm not sure if this is how it works in other projects, but in Nomad ACL system a |
||
} | ||
const nodes = this.store.peekAll('node'); | ||
return { job, nodes }; | ||
} | ||
|
||
startWatchers(controller, model) { | ||
if (!model) { | ||
if (!model.job) { | ||
return; | ||
} | ||
controller.set('watchers', { | ||
model: this.watch.perform(model), | ||
summary: this.watchSummary.perform(model.get('summary')), | ||
allocations: this.watchAllocations.perform(model), | ||
evaluations: this.watchEvaluations.perform(model), | ||
model: this.watch.perform(model.job), | ||
summary: this.watchSummary.perform(model.job.get('summary')), | ||
allocations: this.watchAllocations.perform(model.job), | ||
evaluations: this.watchEvaluations.perform(model.job), | ||
latestDeployment: | ||
model.get('supportsDeployments') && this.watchLatestDeployment.perform(model), | ||
model.job.get('supportsDeployments') && this.watchLatestDeployment.perform(model.job), | ||
list: | ||
model.get('hasChildren') && | ||
this.watchAllJobs.perform({ namespace: model.namespace.get('name') }), | ||
nodes: model.get('hasClientStatus') && this.watchNodes.perform(), | ||
model.job.get('hasChildren') && | ||
this.watchAllJobs.perform({ namespace: model.job.namespace.get('name') }), | ||
nodes: | ||
this.can.can('read client') && | ||
model.job.get('hasClientStatus') && | ||
this.watchNodes.perform(), | ||
}); | ||
} | ||
|
||
setupController(controller, model) { | ||
// Parameterized and periodic detail pages, which list children jobs, | ||
// should sort by submit time. | ||
if (model && ['periodic', 'parameterized'].includes(model.templateType)) { | ||
if (model.job && ['periodic', 'parameterized'].includes(model.job.templateType)) { | ||
controller.setProperties({ | ||
sortProperty: 'submitTime', | ||
sortDescending: true, | ||
|
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.
Lets turn
forceCollapsed
into a reactive getter and then this no longer needs to be a computed property.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.
Not sure if I follow.
forcedCollapsed
is an external property that is set by the caller to force the collapse state.isExpanded
is the getter used to set the component state.