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

Feature: nested struct validation #1122

Merged
merged 5 commits into from Aug 6, 2023

Conversation

MysteriousPotato
Copy link
Contributor

@MysteriousPotato MysteriousPotato commented Jul 3, 2023

Fixes #367, #906

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

A test has been added for custom tags, however I was not brave enough to actually update the tests for all required/excluded tag variants before getting an initial feedback, but I'm willing to do so if this ever gets any further.

Same goes for documentation.

The implementation supports both struct and struct pointer validations for custom tags and all required/excluded tag variants.

Struct validity is evaluated first and fields are evaluated only if the struct is valid, though I'm not sure if this is the desired behavior.

@go-playground/validator-maintainers

@coveralls
Copy link

coveralls commented Jul 3, 2023

Coverage Status

coverage: 73.987% (+0.02%) from 73.972% when pulling b43cc52 on MysteriousPotato:master into bd1113d on go-playground:master.

…g variants on structs containing uncomparable types such as maps or slices

Also added a test to (partially) cover this case.
@MysteriousPotato MysteriousPotato marked this pull request as ready for review July 4, 2023 20:44
@MysteriousPotato MysteriousPotato requested a review from a team as a code owner July 4, 2023 20:44
@deankarn
Copy link
Contributor

Thanks for the PR @MysteriousPotato!

I like the approach here and yes fields after valid struct is the order that's required to not cause breaking changes. Going to let it sit with me for a big to digest.

@deankarn
Copy link
Contributor

@MysteriousPotato I really like this PR!

If you can update the tests I'd be happy to merge this :)

@MysteriousPotato
Copy link
Contributor Author

@deankarn Great!

There are now tests for all tags that are compatible with this implementation of nested struct validation. This inlcudes the tags below:

  • omitempty
  • required
  • required_if
  • required_unless
  • required_with
  • required_without
  • required_with_all
  • required_without_all
  • excluded_if
  • excluded_unless
  • excluded_with
  • excluded_without
  • excluded_with_all
  • excluded_without_all
  • nostructlevel
  • structonly
  • skip_unless
  • custom tags

I did not make a lot of changes to the documentation except a few places where the behavior is specified for each type. Let me know if you want the documentation to be more explicit about the behavior.

@deankarn
Copy link
Contributor

Awesome @MysteriousPotato!

I will try and review early this week :)

@deankarn
Copy link
Contributor

deankarn commented Aug 3, 2023

Sorry I haven’t forgotten about this, just been a heck of a week thus far, still high on my list :)

Copy link
Contributor

@deankarn deankarn left a comment

Choose a reason for hiding this comment

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

Amazing work, will be merging it sometime this weekend :)

@deankarn deankarn merged commit b293a5c into go-playground:master Aug 6, 2023
6 checks passed
deankarn added a commit that referenced this pull request Aug 22, 2023
@deankarn deankarn mentioned this pull request Aug 22, 2023
deankarn added a commit that referenced this pull request Aug 29, 2023
## PR
This PR does the following:
- Reverts #1122 
- Re-implements struct level validations in a different way which also
support `or`s etc.. which previous implementation did not.
- Adds special case to ignore `required` validation on non-pointer
structs to preserve pre-struct level tag validation support.
- Added new `WithRequiredStructEnabled` option to opt-in to this new
behaviour, that will become the default in the next major version.
@deankarn deankarn mentioned this pull request Aug 31, 2023
2 tasks
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.

Custom validation tag is ignored when validating a field that is of type struct
3 participants