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

Fixed support of negative numbers in numeric rule #52

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

maoueh
Copy link
Contributor

@maoueh maoueh commented Jan 10, 2019

The numeric rule was not handling negative numbers correctly as the Numeric regexp was not accepting negative numbers which is valid.

Fixed that and added a bunch of test cases around negative numbers for numeric, numeric_between, min and max.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 6ce2f8a on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 6ce2f8a on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 6ce2f8a on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 6ce2f8a on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 6ce2f8a on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

@coveralls
Copy link

coveralls commented Jan 10, 2019

Coverage Status

Coverage increased (+0.1%) to 88.985% when pulling 8bcee46 on maoueh:fix/numeric-negative into 8625cae on thedevsaddam:master.

@thedevsaddam
Copy link
Owner

Can you please send the PR to dev branch?

@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2019

Yeah sure.

The `numeric` rule was not handling negative numbers correctly as the
`Numeric` regexp was not accepting negative numbers which is valid.

Fixed that and added a bunch of test cases around negative numbers for
`numeric`, `numeric_between`, `min` and `max`.
@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2019

Rebased & updated.

@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2019

While you are not far, I tried validating a query parameters with numeric, min:1, however, the since query param is a string, it validates against the length of string instead of its value.

I see two possibilities to workaround that:

  • Update numeric_between to accept unbound max/min (numeric_between:1, and numeric_between:,10.
  • Add numeric_min and numeric_max

What do you think? Other ideas?

@maoueh maoueh changed the base branch from master to dev January 10, 2019 05:31
@thedevsaddam thedevsaddam merged commit e54c6c0 into thedevsaddam:dev Jan 10, 2019
@maoueh maoueh deleted the fix/numeric-negative branch January 10, 2019 05:38
@maoueh
Copy link
Contributor Author

maoueh commented Jan 10, 2019

I went with numeric_between way, see #53

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants