From cc676795349d5c205700e693e03e20921d4b2ec4 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Wed, 14 Feb 2024 15:25:27 -0500 Subject: [PATCH 1/4] Warning and better error handling for invalid-named variables and job templates --- ui/app/adapters/variable.js | 10 ++++++++ ui/app/components/variable-form.hbs | 5 ++++ ui/app/components/variable-form.js | 24 ++++++++++++++++---- ui/app/controllers/jobs/run/templates/new.js | 17 +++++++++++++- ui/app/templates/jobs/run/templates/new.hbs | 5 ++++ 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/ui/app/adapters/variable.js b/ui/app/adapters/variable.js index 14de0eca9a0..ee023f1e1d3 100644 --- a/ui/app/adapters/variable.js +++ b/ui/app/adapters/variable.js @@ -6,6 +6,7 @@ // @ts-check import ApplicationAdapter from './application'; import AdapterError from '@ember-data/adapter/error'; +import InvalidError from '@ember-data/adapter/error'; import { pluralize } from 'ember-inflector'; import classic from 'ember-classic-decorator'; import { ConflictError } from '@ember-data/adapter/error'; @@ -141,6 +142,15 @@ export default class VariableAdapter extends ApplicationAdapter { { detail: _normalizeConflictErrorObject(payload), status: 409 }, ]); } + if (status === 400) { + return new InvalidError([ + { + detail: + 'Invalid name. Name must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length.', + status: 400, + }, + ]); + } return super.handleResponse(...arguments); } } diff --git a/ui/app/components/variable-form.hbs b/ui/app/components/variable-form.hbs index cab126f6295..c4f307c0a21 100644 --- a/ui/app/components/variable-form.hbs +++ b/ui/app/components/variable-form.hbs @@ -79,6 +79,11 @@ . {{/if}} + {{#if this.hasInvalidPath}} + + Path must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length. + + {{/if}} {{#if this.isJobTemplateVariable}} Use this variable to generate job templates with diff --git a/ui/app/components/variable-form.js b/ui/app/components/variable-form.js index 0fd3e84f12b..51d00f08d22 100644 --- a/ui/app/components/variable-form.js +++ b/ui/app/components/variable-form.js @@ -154,6 +154,11 @@ export default class VariableFormComponent extends Component { } } + get hasInvalidPath() { + let pathNameRegex = new RegExp('^[a-zA-Z0-9-_~/]{1,128}$'); + return !pathNameRegex.test(trimPath([this.path])); + } + @action validateKey(entry, e) { const value = e.target.value; @@ -269,18 +274,27 @@ export default class VariableFormComponent extends Component { this.removeExitHandler(); this.router.transitionTo('variables.variable', this.args.model.id); - } catch (error) { - notifyConflict(this)(error); + } catch (e) { + notifyConflict(this)(e); if (!this.hasConflict) { + let errorMessage = e; + if (e.errors && e.errors.length > 0) { + const nameInvalidError = e.errors.find((err) => err.status === 400); + if (nameInvalidError) { + errorMessage = nameInvalidError.detail; + } + } + + console.log('caught an error', e); this.notifications.add({ title: `Error saving ${this.path}`, - message: error, + message: errorMessage, color: 'critical', sticky: true, }); } else { - if (error.errors[0]?.detail) { - set(this, 'conflictingVariable', error.errors[0].detail); + if (e.errors[0]?.detail) { + set(this, 'conflictingVariable', e.errors[0].detail); } window.scrollTo(0, 0); // because the k/v list may be long, ensure the user is snapped to top to read error } diff --git a/ui/app/controllers/jobs/run/templates/new.js b/ui/app/controllers/jobs/run/templates/new.js index 65ad30d3ff7..59bce48f629 100644 --- a/ui/app/controllers/jobs/run/templates/new.js +++ b/ui/app/controllers/jobs/run/templates/new.js @@ -36,6 +36,11 @@ export default class JobsRunTemplatesNewController extends Controller { ); } + get hasInvalidName() { + let pathNameRegex = new RegExp('^[a-zA-Z0-9-_~/]{1,128}$'); + return !pathNameRegex.test(this.templateName); + } + @action updateKeyValue(key, value) { if (this.model.keyValues.find((kv) => kv.key === key)) { @@ -74,9 +79,19 @@ export default class JobsRunTemplatesNewController extends Controller { this.router.transitionTo('jobs.run.templates'); } catch (e) { + let errorMessage = + 'An unexpected error occurred when saving your Job template.'; + console.log('caught', e); + if (e.errors && e.errors.length > 0) { + const nameInvalidError = e.errors.find((err) => err.status === 400); + if (nameInvalidError) { + errorMessage = nameInvalidError.detail; + } + } + this.notifications.add({ title: 'Job template cannot be saved.', - message: e, + message: errorMessage, color: 'critical', }); } diff --git a/ui/app/templates/jobs/run/templates/new.hbs b/ui/app/templates/jobs/run/templates/new.hbs index 1a59f90e719..1ad146067f9 100644 --- a/ui/app/templates/jobs/run/templates/new.hbs +++ b/ui/app/templates/jobs/run/templates/new.hbs @@ -28,6 +28,11 @@ There is already a templated named {{this.templateName}}.

{{/if}} + {{#if this.hasInvalidName}} +

+ Template name must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length. +

+ {{/if}} {{#if this.system.shouldShowNamespaces}}