Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ui, bug] UI fails to load job when there is an "@" in job name #13012

Merged
merged 7 commits into from
May 13, 2022
Merged

[ui, bug] UI fails to load job when there is an "@" in job name #13012

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
740047e
LastIndexOf and always append a namespace on job links
philrenaud May 13, 2022
73c10f0
Confirmed the volume equivalent and simplified idWIthNamespace logic
philrenaud May 13, 2022
fe959c7
Changelog added
philrenaud May 13, 2022
b4220d4
PR comments addressed
philrenaud May 13, 2022
93b9c39
Drop the redirect for the time being
philrenaud May 13, 2022
1efdb97
Tests updated to reflect namespace on links
philrenaud May 13, 2022
8ee9fde
Task detail test default namespace link for test
philrenaud May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions ui/app/models/job.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,7 @@ export default class Job extends Model {

@computed('plainId')
get idWithNamespace() {
const namespaceId = this.belongsTo('namespace').id();

if (!namespaceId || namespaceId === 'default') {
return this.plainId;
} else {
return `${this.plainId}@${namespaceId}`;
}
return `${this.plainId}@${this.belongsTo('namespace').id()}`;
philrenaud marked this conversation as resolved.
Show resolved Hide resolved
}

@computed('periodic', 'parameterized', 'dispatched')
Expand Down
2 changes: 0 additions & 2 deletions ui/app/models/volume.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ export default class Volume extends Model {

@computed('plainId')
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()}`;
}

Expand Down
12 changes: 11 additions & 1 deletion ui/app/routes/jobs/job.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,17 @@ export default class JobRoute extends Route {
}

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;
namespace = 'default';
philrenaud marked this conversation as resolved.
Show resolved Hide resolved
}
philrenaud marked this conversation as resolved.
Show resolved Hide resolved

const fullId = JSON.stringify([name, namespace]);

Expand Down