-
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] Separate Diffs and Versions from the /versions endpoint as far as Ember is concerned #24145
Changes from all commits
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 |
---|---|---|
|
@@ -19,7 +19,9 @@ export default class JobVersionsStream extends Component { | |
// Passes through to the job-diff component | ||
verbose = true; | ||
|
||
@computed('versions.[]') | ||
diffs = []; | ||
|
||
@computed('versions.[]', 'diffs.[]') | ||
get annotatedVersions() { | ||
const versions = this.versions.sortBy('submitTime').reverse(); | ||
return versions.map((version, index) => { | ||
|
@@ -38,7 +40,8 @@ export default class JobVersionsStream extends Component { | |
} | ||
} | ||
|
||
return { version, meta }; | ||
const diff = this.diffs.objectAt(index); | ||
return { version, meta, diff }; | ||
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 is the component layer where we opt to join the Version and its Diff, which are otherwise same-indexed objects in separate arrays. |
||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,21 +15,8 @@ import { inject as service } from '@ember/service'; | |
export default class VersionsRoute extends Route.extend(WithWatchers) { | ||
@service store; | ||
|
||
queryParams = { | ||
diffVersion: { | ||
refreshModel: true, | ||
}, | ||
}; | ||
|
||
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. Now that the route doesn't care about Diff data (the controller does), we don't need to make it concerned with queryParams / it doesn't need to re-fetch the versions when the user switches the comparison version via dropdown |
||
async model(params) { | ||
async model() { | ||
const job = this.modelFor('jobs.job'); | ||
const versions = await job.getVersions(params.diffVersion); | ||
|
||
job.versions = job.versions.map((v, i) => { | ||
const diff = versions.Diffs[i]; | ||
v.set('diff', diff); | ||
return v; | ||
}); | ||
return job; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -210,7 +210,9 @@ export default class JobSerializer extends ApplicationSerializer { | |
}, | ||
versions: { | ||
links: { | ||
related: buildURL(`${jobURL}/versions`, { namespace, diffs: true }), | ||
related: buildURL(`${jobURL}/versions`, { | ||
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. We no longer need to get |
||
}, | ||
}, | ||
deployments: { | ||
|
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.
the
@computed
prefix to this getter indicates something that should trigger an update.