Skip to content

Consider numbers in the required validator #92

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Consider numbers in the required validator #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/src/rules/required.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const required: FieldValidationFunction = (value, vm, customParams: Requi
function isValidField(value, trim: boolean): boolean {
return typeof value === 'string' ?
isStringValid(value, trim) :
value === true;
value === true || typeof value === "number";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract value === true || typeof value === "number" in a function like isNonStringValid to check non string values. That is, is different than undefined or null

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evaluate if value is different than null or undefined collides with the "When validating a non string value should return false if value is false" test. What do you consider for a boolean with false value?

}

function isStringValid(value: string, trim: boolean): boolean {
Expand Down
14 changes: 14 additions & 0 deletions lib/src/rules/spec/required.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ describe('[required] validation rule tests =>', () => {
expect(validationResult.errorMessage).to.be.empty;
});

it('should return true if typeof value is number', () => {
// Arrange
const value = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the validation if value is 0 or negative?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talking with @brauliodiez comments that we should not consider negative values, nor zero. That is, the validation fail in these cases:

  • Value equals undefined
  • Value equals null
  • Value equals ' '

For another value, should pass validation:

  • 'some text'
  • 0
  • 1
  • -1
  • {}
  • []
  • { property: 'value' }
    etc.

@v-borrego, what do you think? Could you add some unit tests for these cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will add the tests in the next pull request

const vm = undefined;
const customParams: RequiredParams = undefined;

// Act
const validationResult = required(value, vm, customParams) as FieldValidationResult;

// Assert
expect(validationResult.succeeded).to.be.true;
expect(validationResult.type).to.be.equals('REQUIRED');
expect(validationResult.errorMessage).to.be.empty;
});
});

describe('When validating a string value', () => {
Expand Down