Skip to content

Add support for if/unless validator options#239

Merged
dblock merged 1 commit intomongoid:masterfrom
knovoselic:feature/conditional_validators_support
Oct 6, 2021
Merged

Add support for if/unless validator options#239
dblock merged 1 commit intomongoid:masterfrom
knovoselic:feature/conditional_validators_support

Conversation

@knovoselic
Copy link
Copy Markdown
Contributor

This PR adds support for testing validators which are triggered only under a certain condition. The change supports if/unless options with symbol or lambda parameters as the conditions.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Oct 5, 2021

Definitely need to fix CI first.

@dblock dblock merged commit 6f0ac2f into mongoid:master Oct 6, 2021
@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Oct 6, 2021

@knovoselic interested in helping out with maintaining this library? If @rodrigopinto doesn't object I can add you here and rubygems.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Oct 6, 2021

Maybe you can finish #216 too? :)

@knovoselic knovoselic deleted the feature/conditional_validators_support branch October 7, 2021 13:43
@knovoselic
Copy link
Copy Markdown
Contributor Author

@dblock thanks! I'm not sure if I'll have enough time to help with maintenance of the library. Let's see if I can find some time next week to take care of #216.

@johnnyshields
Copy link
Copy Markdown
Member

FYI, this PR broke semver of this gem as it was released as 4.2.0 (tiny version bump)--it caused many of my existing test cases to fail.

I have merged a PR here: #249 which now gates it behind a .check_conditions method.

I am lukewarm on the idea of making this "check_conditions" logic to be the default behavior. If we were to do it, it would need to (a) be done in a major release, i.e. 5.0.0, and (b) we would have to handle it very carefully. Read this comment for details:
#249 (comment)

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented May 4, 2026

Agreed @johnnyshields I think a 5.0 with default behavior makes sense.

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.

3 participants