From efe31ae32e401f54118ced03ac9f39bfc05ea4e4 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Mon, 3 Apr 2023 15:24:58 -0600 Subject: [PATCH] Model Validation Warnings (#19913) * updates model validations to support warnings and adds alert to form field * adds changelog entry * updates model validations tests --- changelog/19913.txt | 3 ++ ui/app/components/mount-backend-form.js | 12 +++++++ ui/app/decorators/model-validations.js | 35 ++++++++++++------- ui/app/models/auth-method.js | 10 +++++- ui/app/models/secret-engine.js | 10 +++++- ui/lib/core/addon/components/form-field.hbs | 8 +++++ ui/lib/core/addon/components/form-field.js | 5 +++ .../integration/components/form-field-test.js | 16 +++++++++ .../unit/decorators/model-validations-test.js | 22 ++++++++++-- 9 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 changelog/19913.txt diff --git a/changelog/19913.txt b/changelog/19913.txt new file mode 100644 index 000000000..eccdec653 --- /dev/null +++ b/changelog/19913.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Adds whitespace warning to secrets engine and auth method path inputs +``` \ No newline at end of file diff --git a/ui/app/components/mount-backend-form.js b/ui/app/components/mount-backend-form.js index e33b89ece..6fa141265 100644 --- a/ui/app/components/mount-backend-form.js +++ b/ui/app/components/mount-backend-form.js @@ -63,6 +63,17 @@ export default class MountBackendForm extends Component { return isValid; } + checkModelWarnings() { + // check for warnings on change + // since we only show errors on submit we need to clear those out and only send warning state + const { state } = this.args.mountModel.validate(); + for (const key in state) { + state[key].errors = []; + } + this.modelValidations = state; + this.invalidFormAlert = null; + } + async showWarningsForKvv2() { try { const capabilities = await this.store.findRecord('capabilities', `${this.args.mountModel.path}/config`); @@ -141,6 +152,7 @@ export default class MountBackendForm extends Component { @action onKeyUp(name, value) { this.args.mountModel[name] = value; + this.checkModelWarnings(); } @action diff --git a/ui/app/decorators/model-validations.js b/ui/app/decorators/model-validations.js index 800ce32ed..5f598a571 100644 --- a/ui/app/decorators/model-validations.js +++ b/ui/app/decorators/model-validations.js @@ -11,14 +11,20 @@ import { get } from '@ember/object'; * used to validate properties on a class * * decorator expects validations object with the following shape: - * { [propertyKeyName]: [{ type, options, message, validator }] } + * { [propertyKeyName]: [{ type, options, message, level, validator }] } * each key in the validations object should refer to the property on the class to apply the validation to - * type refers to the type of validation to apply -- must be exported from validators util for lookup - * options is an optional object for given validator -- min, max, nullable etc. -- see validators in util - * message is added to the errors array and returned from the validate method if validation fails - * validator may be used in place of type to provide a function that gets executed in the validate method - * validator is useful when specific validations are needed (dependent on other class properties etc.) - * validator must be passed as function that takes the class context (this) as the only argument and returns true or false + * + * type - string referring to the type of validation to apply -- must be exported from validators util for lookup + * + * options - an optional object for given validator -- min, max, nullable etc. -- see validators in util + * + * message - string added to the errors array and returned from the validate method if validation fails + * + * level - optional string that defaults to 'error'. Currently the only other accepted value is 'warn' + * + * validator - function that may be used in place of type that is invoked in the validate method + * useful when specific validations are needed (dependent on other class properties etc.) + * must be passed as function that takes the class context (this) as the only argument and returns true or false * each property supports multiple validations provided as an array -- for example, presence and length for string * * validations must be invoked using the validate method which is added directly to the decorated class @@ -59,6 +65,7 @@ import { get } from '@ember/object'; * -> state.foo.errors = ['foo is required if bar includes test.']; * * *** example adding class in hbs file + * * all form-validations need to have a red border around them. Add this by adding a conditional class 'has-error-border' * class="input field {{if this.errors.name.errors 'has-error-border'}}" */ @@ -91,10 +98,10 @@ export function withModelValidations(validations) { continue; } - state[key] = { errors: [] }; + state[key] = { errors: [], warnings: [] }; for (const rule of rules) { - const { type, options, message, validator: customValidator } = rule; + const { type, options, level, message, validator: customValidator } = rule; // check for custom validator or lookup in validators util by type const useCustomValidator = typeof customValidator === 'function'; const validator = useCustomValidator ? customValidator : validators[type]; @@ -115,9 +122,13 @@ export function withModelValidations(validations) { if (!passedValidation) { // consider setting a prop like validationErrors directly on the model // for now return an errors object - state[key].errors.push(message); - if (isValid) { - isValid = false; + if (level === 'warn') { + state[key].warnings.push(message); + } else { + state[key].errors.push(message); + if (isValid) { + isValid = false; + } } } } diff --git a/ui/app/models/auth-method.js b/ui/app/models/auth-method.js index 0a7288771..9cc365476 100644 --- a/ui/app/models/auth-method.js +++ b/ui/app/models/auth-method.js @@ -13,7 +13,15 @@ import attachCapabilities from 'vault/lib/attach-capabilities'; import { withModelValidations } from 'vault/decorators/model-validations'; const validations = { - path: [{ type: 'presence', message: "Path can't be blank." }], + path: [ + { type: 'presence', message: "Path can't be blank." }, + { + type: 'containsWhiteSpace', + message: + "Path contains whitespace. If this is desired, you'll need to encode it with %20 in API requests.", + level: 'warn', + }, + ], }; // unsure if ember-api-actions will work on native JS class model diff --git a/ui/app/models/secret-engine.js b/ui/app/models/secret-engine.js index f1db0420a..973429908 100644 --- a/ui/app/models/secret-engine.js +++ b/ui/app/models/secret-engine.js @@ -14,7 +14,15 @@ import { withExpandedAttributes } from 'vault/decorators/model-expanded-attribut const LIST_EXCLUDED_BACKENDS = ['system', 'identity']; const validations = { - path: [{ type: 'presence', message: "Path can't be blank." }], + path: [ + { type: 'presence', message: "Path can't be blank." }, + { + type: 'containsWhiteSpace', + message: + "Path contains whitespace. If this is desired, you'll need to encode it with %20 in API requests.", + level: 'warn', + }, + ], maxVersions: [ { type: 'number', message: 'Maximum versions must be a number.' }, { type: 'length', options: { min: 1, max: 16 }, message: 'You cannot go over 16 characters.' }, diff --git a/ui/lib/core/addon/components/form-field.hbs b/ui/lib/core/addon/components/form-field.hbs index b3a405fa8..895cd019c 100644 --- a/ui/lib/core/addon/components/form-field.hbs +++ b/ui/lib/core/addon/components/form-field.hbs @@ -318,6 +318,14 @@ {{#if this.validationError}} {{/if}} + {{#if this.validationWarning}} + + {{/if}} {{/if}} {{else if (eq @attr.type "boolean")}} diff --git a/ui/lib/core/addon/components/form-field.js b/ui/lib/core/addon/components/form-field.js index 18042d418..3473cae58 100644 --- a/ui/lib/core/addon/components/form-field.js +++ b/ui/lib/core/addon/components/form-field.js @@ -117,6 +117,11 @@ export default class FormFieldComponent extends Component { const state = validations[this.valuePath]; return state && !state.isValid ? state.errors.join(' ') : null; } + get validationWarning() { + const validations = this.args.modelValidations || {}; + const state = validations[this.valuePath]; + return state?.warnings?.length ? state.warnings.join(' ') : null; + } onChange() { if (this.args.onChange) { diff --git a/ui/tests/integration/components/form-field-test.js b/ui/tests/integration/components/form-field-test.js index 68e765140..641889f4c 100644 --- a/ui/tests/integration/components/form-field-test.js +++ b/ui/tests/integration/components/form-field-test.js @@ -210,4 +210,20 @@ module('Integration | Component | form field', function (hooks) { assert.dom('[data-test-toggle-input="Foo"]').isChecked('Toggle is initially checked when given value'); assert.dom('[data-test-ttl-value="Foo"]').hasValue('1', 'Ttl input displays with correct value'); }); + + test('it should show validation warning', async function (assert) { + const model = this.owner.lookup('service:store').createRecord('auth-method'); + model.path = 'foo bar'; + this.validations = model.validate().state; + this.setProperties({ + model, + attr: createAttr('path', 'string'), + onChange: () => {}, + }); + + await render( + hbs`` + ); + assert.dom('[data-test-validation-warning]').exists('Validation warning renders'); + }); }); diff --git a/ui/tests/unit/decorators/model-validations-test.js b/ui/tests/unit/decorators/model-validations-test.js index 5254c1d02..4de890380 100644 --- a/ui/tests/unit/decorators/model-validations-test.js +++ b/ui/tests/unit/decorators/model-validations-test.js @@ -74,7 +74,7 @@ module('Unit | Decorators | ModelValidations', function (hooks) { assert.false(v1.isValid, 'isValid state is correct when errors exist'); assert.deepEqual( v1.state, - { foo: { isValid: false, errors: [message] } }, + { foo: { isValid: false, errors: [message], warnings: [] } }, 'Correct state returned when property is invalid' ); @@ -83,7 +83,7 @@ module('Unit | Decorators | ModelValidations', function (hooks) { assert.true(v2.isValid, 'isValid state is correct when no errors exist'); assert.deepEqual( v2.state, - { foo: { isValid: true, errors: [] } }, + { foo: { isValid: true, errors: [], warnings: [] } }, 'Correct state returned when property is valid' ); }); @@ -115,4 +115,22 @@ module('Unit | Decorators | ModelValidations', function (hooks) { const v3 = fooClass.validate(); assert.strictEqual(v3.invalidFormMessage, null, 'invalidFormMessage is null when form is valid'); }); + + test('it should validate warnings', function (assert) { + const message = 'Value contains whitespace.'; + const validations = { + foo: [ + { + type: 'containsWhiteSpace', + message, + level: 'warn', + }, + ], + }; + const fooClass = createClass(validations); + fooClass.foo = 'foo bar'; + const { state, isValid } = fooClass.validate(); + assert.true(isValid, 'Model is considered valid when there are only warnings'); + assert.strictEqual(state.foo.warnings.join(' '), message, 'Warnings are returned'); + }); });