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.

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

Model Validation Warnings #19913

Merged
merged 4 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/19913.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Adds whitespace warning to secrets engine and auth method path inputs
```
12 changes: 12 additions & 0 deletions ui/app/components/mount-backend-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down Expand Up @@ -141,6 +152,7 @@ export default class MountBackendForm extends Component {
@action
onKeyUp(name, value) {
this.args.mountModel[name] = value;
this.checkModelWarnings();
}

@action
Expand Down
35 changes: 23 additions & 12 deletions ui/app/decorators/model-validations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'}}"
*/
Expand Down Expand Up @@ -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];
Expand All @@ -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;
}
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion ui/app/models/auth-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion ui/app/models/secret-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.' },
Expand Down
8 changes: 8 additions & 0 deletions ui/lib/core/addon/components/form-field.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@
{{#if this.validationError}}
<AlertInline @type="danger" @message={{this.validationError}} @paddingTop={{true}} />
{{/if}}
{{#if this.validationWarning}}
<AlertInline
@type="warning"
@message={{this.validationWarning}}
@paddingTop={{true}}
data-test-validation-warning
/>
{{/if}}
{{/if}}
</div>
{{else if (eq @attr.type "boolean")}}
Expand Down
5 changes: 5 additions & 0 deletions ui/lib/core/addon/components/form-field.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions ui/tests/integration/components/form-field-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`<FormField @attr={{this.attr}} @model={{this.model}} @modelValidations={{this.validations}} @onChange={{this.onChange}} />`
);
assert.dom('[data-test-validation-warning]').exists('Validation warning renders');
});
});
22 changes: 20 additions & 2 deletions ui/tests/unit/decorators/model-validations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);

Expand All @@ -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'
);
});
Expand Down Expand Up @@ -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');
});
});