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

Validations #4

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Validations #4

wants to merge 8 commits into from

Conversation

atlantis
Copy link
Contributor

@atlantis atlantis commented Oct 27, 2020

First shot at validations - it's a bit more complex than ActiveRecord because I've always wanted the ability to have the backend generate warnings that don't prevent the model saving but can be displayed to the user/cause some other action.

  • Supports a few common built-in validations: validate :field, :present
  • Or custom validations validate :field, "must be capitalized" { |model| ... true|false|nil }
  • Or fully custom validations validate :field {|model, problems| ... problems << MySpecialError.new("Stuff") ... }
  • Nils cause further validations to run... false stops the chain
  • Adds a save method that only inserts/updates upon successful validation - insert and update behavior are unaffected.

One side-effect of the way I wrote this is that you can't have a model class inherit from another non-abstract model class... that seems lame but I couldn't figure out a way around it so far 🤷

@elbywan
Copy link
Owner

elbywan commented Nov 1, 2020

Hey @atlantis, thanks for the PR this is a great addition ❤️ !

Adds a save method that only inserts/updates upon successful validation - insert and update behavior are unaffected.

Nice, and if users want to add validation with the regular insert/update methods they can use hooks (before_insert, before_update) anyway.

One side-effect of the way I wrote this is that you can't have a model class inherit from another non-abstract model class...

I think we can work around that by dropping the inherited macro. The argument of the block passed to the validate methods will always typed as the superclass+, but it is actually correct since the validators can be set from any class in the hierarchy. They can be force casted by the user anyway with .as if needed.

I would also like to make Validation optional (like the Versioning module) so people using Moongoon can pick another validation library if they want to.

I have made some changes based on what I wrote above on a separate branch (I don't know if there is better way to contribute to an external PR, maybe this?)

If you are cool with the changes then we'll also need to document everything properly at some point 😉.

make validation optional and allow inheritance
@atlantis
Copy link
Contributor Author

atlantis commented Nov 3, 2020

Great! Looks like there's a conflict so I may let you resolve that if that's OK to make sure the correct parts get kept...

@atlantis
Copy link
Contributor Author

atlantis commented Nov 3, 2020

And I'll work on documentation in the next few days - do you want it on the main Readme or just documented in the API docs?

@elbywan
Copy link
Owner

elbywan commented Nov 5, 2020

Looks like there's a conflict so I may let you resolve that if that's OK to make sure the correct parts get kept

Sure, I'll look into it!

do you want it on the main Readme or just documented in the API docs

I you could handle the API docs it would be great, and as a follow up I'll add a paragraph in the readme.

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