-
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: Edit a job #4612
UI: Edit a job #4612
Changes from all commits
d64491b
a2c12b9
e9190f4
8031a0d
1e4ca09
522f6d7
cdc37ec
91f8c15
d36270b
69d28de
b68ba61
b478d71
684f45c
4ae15be
6463efe
c1d44fb
e8db71a
85aaa53
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,102 @@ | ||
import Component from '@ember/component'; | ||
import { assert } from '@ember/debug'; | ||
import { inject as service } from '@ember/service'; | ||
import { computed } from '@ember/object'; | ||
import { task } from 'ember-concurrency'; | ||
import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; | ||
import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; | ||
|
||
export default Component.extend({ | ||
store: service(), | ||
config: service(), | ||
|
||
'data-test-job-editor': true, | ||
|
||
job: null, | ||
onSubmit() {}, | ||
context: computed({ | ||
get() { | ||
return this.get('_context'); | ||
}, | ||
set(key, value) { | ||
const allowedValues = ['new', 'edit']; | ||
|
||
assert(`context must be one of: ${allowedValues.join(', ')}`, allowedValues.includes(value)); | ||
|
||
this.set('_context', value); | ||
return value; | ||
}, | ||
}), | ||
|
||
_context: null, | ||
parseError: null, | ||
planError: null, | ||
runError: null, | ||
|
||
planOutput: null, | ||
|
||
showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), | ||
showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), | ||
|
||
stage: computed('planOutput', function() { | ||
return this.get('planOutput') ? 'plan' : 'editor'; | ||
}), | ||
|
||
plan: task(function*() { | ||
this.reset(); | ||
|
||
try { | ||
yield this.get('job').parse(); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not parse input'; | ||
this.set('parseError', error); | ||
this.scrollToError(); | ||
return; | ||
} | ||
|
||
try { | ||
const plan = yield this.get('job').plan(); | ||
this.set('planOutput', plan); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not plan job'; | ||
this.set('planError', error); | ||
this.scrollToError(); | ||
} | ||
}).drop(), | ||
|
||
submit: task(function*() { | ||
try { | ||
if (this.get('context') === 'new') { | ||
yield this.get('job').run(); | ||
} else { | ||
yield this.get('job').update(); | ||
} | ||
|
||
const id = this.get('job.plainId'); | ||
const namespace = this.get('job.namespace.name') || 'default'; | ||
|
||
this.reset(); | ||
|
||
// Treat the job as ephemeral and only provide ID parts. | ||
this.get('onSubmit')(id, namespace); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not submit job'; | ||
this.set('runError', error); | ||
this.set('planOutput', null); | ||
this.scrollToError(); | ||
} | ||
}), | ||
|
||
reset() { | ||
this.set('planOutput', null); | ||
this.set('planError', null); | ||
this.set('parseError', null); | ||
this.set('runError', null); | ||
}, | ||
|
||
scrollToError() { | ||
if (!this.get('config.isTest')) { | ||
window.scrollTo(0, 0); | ||
} | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,4 +4,22 @@ import { alias } from '@ember/object/computed'; | |
|
||
export default Controller.extend(WithNamespaceResetting, { | ||
job: alias('model.job'), | ||
definition: alias('model.definition'), | ||
|
||
isEditing: false, | ||
|
||
edit() { | ||
this.get('job').set('_newDefinition', JSON.stringify(this.get('definition'), null, 2)); | ||
this.set('isEditing', true); | ||
}, | ||
|
||
onCancel() { | ||
this.set('isEditing', false); | ||
}, | ||
|
||
onSubmit(id, namespace) { | ||
this.transitionToRoute('jobs.job', id, { | ||
queryParams: { jobNamespace: namespace }, | ||
}); | ||
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. This might be a stupid question, but bear with me. I think these methods are used as actions in the template, so why aren't they part of an 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 experimenting with the usefulness of the I'd like to hear your thoughts on the experiment! |
||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,69 +1,9 @@ | ||
import Controller from '@ember/controller'; | ||
import { inject as service } from '@ember/service'; | ||
import { computed } from '@ember/object'; | ||
import { task } from 'ember-concurrency'; | ||
import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; | ||
import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; | ||
|
||
export default Controller.extend({ | ||
store: service(), | ||
|
||
parseError: null, | ||
planError: null, | ||
runError: null, | ||
|
||
planOutput: null, | ||
|
||
showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), | ||
showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), | ||
|
||
stage: computed('planOutput', function() { | ||
return this.get('planOutput') ? 'plan' : 'editor'; | ||
}), | ||
|
||
plan: task(function*() { | ||
this.reset(); | ||
|
||
try { | ||
yield this.get('model').parse(); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not parse input'; | ||
this.set('parseError', error); | ||
return; | ||
} | ||
|
||
try { | ||
yield this.get('model').plan(); | ||
const plan = this.get('store').peekRecord('job-plan', this.get('model.id')); | ||
this.set('planOutput', plan); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not plan job'; | ||
this.set('planError', error); | ||
} | ||
}).drop(), | ||
|
||
submit: task(function*() { | ||
try { | ||
yield this.get('model').run(); | ||
|
||
const id = this.get('model.plainId'); | ||
const namespace = this.get('model.namespace.name') || 'default'; | ||
|
||
this.reset(); | ||
|
||
// navigate to the new job page | ||
this.transitionToRoute('jobs.job', id, { | ||
queryParams: { jobNamespace: namespace }, | ||
}); | ||
} catch (err) { | ||
const error = messageFromAdapterError(err) || 'Could not submit job'; | ||
this.set('runError', error); | ||
} | ||
}), | ||
|
||
reset() { | ||
this.set('planOutput', null); | ||
this.set('planError', null); | ||
this.set('parseError', null); | ||
onSubmit(id, namespace) { | ||
this.transitionToRoute('jobs.job', id, { | ||
queryParams: { jobNamespace: namespace }, | ||
}); | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,6 @@ export default Service.extend({ | |
}, | ||
|
||
setIndexFor(url, value) { | ||
list[url] = value; | ||
list[url] = +value; | ||
}, | ||
}); |
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.
My kingdom for static type checking.
This took me a minute to understand. If you find yourself doing this a lot, it might reduce the local noise to extract to a computed util (like the excellent
localStorageProperty
).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.
Totally agree. Something like
whitelistProperty(val1, val2, val3)
. I'm honestly not happy at all with this component. I didn't want to spend too much time thinking about how best to refactor it to not have this concept of "contexts", so I just went with it.