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

Add comparison rules #84

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

Conversation

System-Glitch
Copy link

@System-Glitch System-Glitch commented Sep 30, 2019

Comparison rules addded:

  • Rule functions have access to the entire form in order to be able to compare one field with another.
  • Can be used to add more complex rules such as "field_1 must be greater than field_2" or even "date_start must be before date_end"
  • Updated documentation and README to reflect the change

Caveats:

  • Breaks backward compatibility. But can be fixed simply by adding the following parameter to all custom rules: form map[string]interface{}
  • Slightly heavier when not using JSON in request since a copy of the form in a map[string]interface{} is made.

@coveralls
Copy link

coveralls commented Sep 30, 2019

Coverage Status

Coverage increased (+0.5%) to 89.813% when pulling 4890adf on System-Glitch:features/comparison into a8b0eb4 on thedevsaddam:dev.

@System-Glitch System-Glitch force-pushed the features/comparison branch 2 times, most recently from 4890adf to 52c8769 Compare October 21, 2019 11:37
@lsfiege
Copy link

lsfiege commented Nov 11, 2019

@thedevsaddam can you approve this PR? :)

@afloresaivo
Copy link

@thedevsaddam approve this PR plz, thanks my friend!

@thedevsaddam
Copy link
Owner

Guys, this is a breaking change. To merge the request we have to move to the next major release. Can you consider this issue #26, seems it'll resolve the current PR

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

5 participants