12986 UI fails to load job when there is an "@" in job name in nomad 130 (#13012)

* LastIndexOf and always append a namespace on job links

* Confirmed the volume equivalent and simplified idWIthNamespace logic

* Changelog added

* PR comments addressed

* Drop the redirect for the time being

* Tests updated to reflect namespace on links

* Task detail test default namespace link for test
This commit is contained in:
Phil Renaud 2022-05-13 17:01:27 -04:00 committed by GitHub
parent faeb3fcd44
commit 45dc1cfd58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 26 additions and 17 deletions

3
.changelog/13012.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ui: fixed a bug where links to jobs with "@" in their name would mis-identify namespace and 404
```

View File

@ -37,13 +37,7 @@ export default class Job extends Model {
@computed('plainId') @computed('plainId')
get idWithNamespace() { get idWithNamespace() {
const namespaceId = this.belongsTo('namespace').id(); return `${this.plainId}@${this.belongsTo('namespace').id() ?? 'default'}`;
if (!namespaceId || namespaceId === 'default') {
return this.plainId;
} else {
return `${this.plainId}@${namespaceId}`;
}
} }
@computed('periodic', 'parameterized', 'dispatched') @computed('periodic', 'parameterized', 'dispatched')

View File

@ -42,8 +42,6 @@ export default class Volume extends Model {
@computed('plainId') @computed('plainId')
get idWithNamespace() { get idWithNamespace() {
// does this handle default namespace -- I think the backend handles this for us
// but the client would always need to recreate that logic
return `${this.plainId}@${this.belongsTo('namespace').id()}`; return `${this.plainId}@${this.belongsTo('namespace').id()}`;
} }

View File

@ -9,13 +9,23 @@ export default class JobRoute extends Route {
@service can; @service can;
@service store; @service store;
@service token; @service token;
@service router;
serialize(model) { serialize(model) {
return { job_name: model.get('idWithNamespace') }; return { job_name: model.get('idWithNamespace') };
} }
model(params) { model(params) {
const [name, namespace = 'default'] = params.job_name.split('@'); let name,
namespace = 'default';
const { job_name } = params;
const delimiter = job_name.lastIndexOf('@');
if (delimiter !== -1) {
name = job_name.slice(0, delimiter);
namespace = job_name.slice(delimiter + 1);
} else {
name = job_name;
}
const fullId = JSON.stringify([name, namespace]); const fullId = JSON.stringify([name, namespace]);

View File

@ -503,7 +503,7 @@ module('Acceptance | allocation detail (preemptions)', function (hooks) {
await Allocation.preempter.visitJob(); await Allocation.preempter.visitJob();
assert.equal( assert.equal(
currentURL(), currentURL(),
`/jobs/${preempterJob.id}`, `/jobs/${preempterJob.id}@default`,
'Clicking the preempter job link navigates to the preempter job page' 'Clicking the preempter job link navigates to the preempter job page'
); );

View File

@ -64,7 +64,7 @@ module('Acceptance | jobs list', function (hooks) {
assert.equal(jobRow.name, job.name, 'Name'); assert.equal(jobRow.name, job.name, 'Name');
assert.notOk(jobRow.hasNamespace); assert.notOk(jobRow.hasNamespace);
assert.equal(jobRow.link, `/ui/jobs/${job.id}`, 'Detail Link'); assert.equal(jobRow.link, `/ui/jobs/${job.id}@default`, 'Detail Link');
assert.equal(jobRow.status, job.status, 'Status'); assert.equal(jobRow.status, job.status, 'Status');
assert.equal(jobRow.type, typeForJob(job), 'Type'); assert.equal(jobRow.type, typeForJob(job), 'Type');
assert.equal(jobRow.priority, job.priority, 'Priority'); assert.equal(jobRow.priority, job.priority, 'Priority');
@ -78,7 +78,7 @@ module('Acceptance | jobs list', function (hooks) {
await JobsList.visit(); await JobsList.visit();
await JobsList.jobs.objectAt(0).clickName(); await JobsList.jobs.objectAt(0).clickName();
assert.equal(currentURL(), `/jobs/${job.id}`); assert.equal(currentURL(), `/jobs/${job.id}@default`);
}); });
test('the new job button transitions to the new job page', async function (assert) { test('the new job button transitions to the new job page', async function (assert) {

View File

@ -54,7 +54,11 @@ module('Acceptance | regions (only one)', function (hooks) {
const jobId = JobsList.jobs.objectAt(0).id; const jobId = JobsList.jobs.objectAt(0).id;
await JobsList.jobs.objectAt(0).clickRow(); await JobsList.jobs.objectAt(0).clickRow();
assert.equal(currentURL(), `/jobs/${jobId}`, 'No region query param'); assert.equal(
currentURL(),
`/jobs/${jobId}@default`,
'No region query param'
);
await ClientsList.visit(); await ClientsList.visit();
assert.equal(currentURL(), '/clients', 'No region query param'); assert.equal(currentURL(), '/clients', 'No region query param');

View File

@ -104,7 +104,7 @@ module('Acceptance | task detail', function (hooks) {
await Layout.breadcrumbFor('jobs.job.index').visit(); await Layout.breadcrumbFor('jobs.job.index').visit();
assert.equal( assert.equal(
currentURL(), currentURL(),
`/jobs/${job.id}`, `/jobs/${job.id}@default`,
'Job breadcrumb links correctly' 'Job breadcrumb links correctly'
); );
@ -112,7 +112,7 @@ module('Acceptance | task detail', function (hooks) {
await Layout.breadcrumbFor('jobs.job.task-group').visit(); await Layout.breadcrumbFor('jobs.job.task-group').visit();
assert.equal( assert.equal(
currentURL(), currentURL(),
`/jobs/${job.id}/${taskGroup}`, `/jobs/${job.id}@default/${taskGroup}`,
'Task Group breadcrumb links correctly' 'Task Group breadcrumb links correctly'
); );

View File

@ -184,7 +184,7 @@ module('Acceptance | topology', function (hooks) {
await reset(); await reset();
await Topology.allocInfoPanel.visitJob(); await Topology.allocInfoPanel.visitJob();
assert.equal(currentURL(), `/jobs/${job.id}`); assert.equal(currentURL(), `/jobs/${job.id}@default`);
await reset(); await reset();