Skip to content

Commit

Permalink
FIX: fields with boolean values should be considered empty only if no…
Browse files Browse the repository at this point in the history
… value is set
  • Loading branch information
royriojas committed Jan 21, 2021
1 parent 5242360 commit 98112c6
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 16 deletions.
31 changes: 23 additions & 8 deletions src/Field.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { observable, computed, action, makeObservable } from 'mobx';
import debounce from 'debouncy';
import trim from 'jq-trim';

const isNullishOrEmpty = value => typeof value === 'undefined' || value === null || value === '';

/**
* Field class provides abstract the validation of a single field
Expand Down Expand Up @@ -38,7 +41,8 @@ export default class Field {
if (Array.isArray(this.value)) {
return this.value.length > 0;
}
return !!this.value;

return !isNullishOrEmpty(this.value);
}

/**
Expand Down Expand Up @@ -157,7 +161,7 @@ export default class Field {
setValue(value, { resetInteractedFlag, commit } = {}) {
if (resetInteractedFlag) {
this._setValueOnly(value);
this.errorMessage = '';
this.errorMessage = undefined;
this._interacted = false;
} else {
this._setValue(value);
Expand Down Expand Up @@ -185,7 +189,7 @@ export default class Field {
* considered valid if the errorMessage is not empty
*/
clearValidation() {
this.errorMessage = '';
this.errorMessage = undefined;
}

/**
Expand Down Expand Up @@ -268,7 +272,7 @@ export default class Field {
// if we're not forcing the validation
// and we haven't interacted with the field
// we asume this field pass the validation status
this.errorMessage = '';
this.errorMessage = undefined;
return;
}
} else {
Expand All @@ -283,7 +287,7 @@ export default class Field {
this.errorMessage = typeof this._required === 'string' ? this._required : `Field: "${this.name}" is required`;
return;
}
this.errorMessage = '';
this.errorMessage = undefined;
}

const res = this._doValidate();
Expand All @@ -295,7 +299,7 @@ export default class Field {
// if the function returned a boolean we assume it is
// the flag for the valid state
if (typeof res_ === 'boolean') {
this.errorMessage = res_ ? '' : this.originalErrorMessage;
this.errorMessage = res_ ? undefined : this.originalErrorMessage;
resolve();
return;
}
Expand All @@ -306,12 +310,19 @@ export default class Field {
return;
}

this.errorMessage = '';
this.errorMessage = undefined;
resolve(); // we use this to chain validators
}),
action((errorArg = {}) => {
const { error, message } = errorArg;
this.errorMessage = (error || message || '').trim() || this.originalErrorMessage;

let errorMessageToSet = (error || message || '').trim() || this.originalErrorMessage;

if (errorMessageToSet === '') {
errorMessageToSet = undefined; // empty string is not longer a valid value for error message
}

this.errorMessage = errorMessageToSet;
resolve(); // we use this to chain validators
}),
);
Expand All @@ -323,6 +334,10 @@ export default class Field {
};

setErrorMessage(msg) {
if (trim(msg) === '') {
msg = undefined;
}

this.errorMessage = msg;
}

Expand Down
3 changes: 1 addition & 2 deletions tests/FormModel.old.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ describe('form-model', () => {
fields: model.fields,
};
},
({ fields, name }) => {
console.log('name', name);
({ fields }) => {
fields.lastName.setRequired(!!fields.name.value && 'lastName is required');
fields.email.setRequired(fields.lastName.value === 'Doo' && 'Email is required for all Doos');
},
Expand Down
6 changes: 3 additions & 3 deletions tests/FormModel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ describe('FormModel', () => {
await model.validate();

expect(model.valid).toEqual(true);
expect(model.fields.email.errorMessage).toEqual('');
expect(model.fields.email.errorMessage).toEqual(undefined);
});

it('The validation function can return an error object to describe the error', async () => {
Expand Down Expand Up @@ -920,7 +920,7 @@ describe('FormModel', () => {
await model.validate();

expect(model.valid).toEqual(true);
expect(model.fields.email.errorMessage).toEqual('');
expect(model.fields.email.errorMessage).toEqual(undefined);
});

it('error message can be cleared from the field and make the form to be considered valid again', async () => {
Expand Down Expand Up @@ -952,7 +952,7 @@ describe('FormModel', () => {
model.fields.email.clearValidation();

expect(model.valid).toEqual(true);
expect(model.fields.email.errorMessage).toEqual('');
expect(model.fields.email.errorMessage).toEqual(undefined);
});
});

Expand Down
22 changes: 22 additions & 0 deletions tests/booleanValues.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { createModel } from '../src/FormModel';

describe('FormModel with boolean fields', () => {
it('it should not complain value is required if value is set to a boolean', async () => {
const model = createModel({
descriptors: { name: {}, lastName: {}, isFunny: { required: true } },
initialState: { name: 'Snoopy', lastName: 'Brown' },
});

expect(model.fields.isFunny.value).toEqual(undefined);

await model.validate();

expect(model.fields.isFunny.errorMessage).toMatchInlineSnapshot('"Field: \\"isFunny\\" is required"');

model.fields.isFunny.setValue(false);

await model.validate();

expect(model.fields.isFunny.errorMessage).toEqual(undefined);
});
});
6 changes: 3 additions & 3 deletions tests/throwIfMissingField.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('FormModel.options.throwIfMissingField', () => {
});
});

describe('if throwIfMissingField is not specified it should behave as if it is true by default', () => {
it('should not throw if ', () => {
describe('if throwIfMissingField is not specified', () => {
it('it should behave as if it is true by default', () => {
expect(() => {
createModel({
descriptors: { name: {}, lastName: {} },
Expand All @@ -34,7 +34,7 @@ describe('FormModel.options.throwIfMissingField', () => {
});

describe('if throwIfMissingField is false', () => {
it('should not throw if ', () => {
it('should not throw if a field is passed that do not match the ones specified in the descriptors', () => {
let model;
expect(() => {
model = createModel({
Expand Down

0 comments on commit 98112c6

Please sign in to comment.