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

Custom errors and valid property on Operations #534

Merged
merged 6 commits into from
Nov 14, 2020
Merged

Conversation

jwoertink
Copy link
Member

@jwoertink jwoertink commented Nov 13, 2020

Fixes #529

This allows you to set custom errors not related to attributes. When you set an error message, the whole operation is no longer valid.

The main use case is when your operations (or save operations) need some external object, but that object itself may also not be valid.

class SaveMessage < Message::SaveOperation
  needs user : User

  before_save do
    if user.is_not_allowed_to_save_message?
       add_error(:user_not_welcome, "You don messed up A A ron.")
    end
  end
end

SaveMessage.create(user: current_user) do |op, _|
  op.errors[:user_not_welcome] == ["You don messed up A A ron"]
end

…od. Also added in a new add_error method for custom error messages not attached directly to attributes
@matthewmcgarvey
Copy link
Member

@jwoertink Calling add_error does not make the operation invalid, is that intentional?

This is something that has tripped me up in rails before, and this PR implements the same thing.

@jwoertink
Copy link
Member Author

jwoertink commented Nov 13, 2020

hmm 🤔 I guess I assumed you'd never call add_error unless it was invalid... BUT.... you do make a good point. I think the ruby mutations gem (lol) does that where you just add an error and the whole thing fails. Do you think that would be a better way? So like, instead of setting self.valid = false, you just add an error message, and it knows it's invalid at that point.... I'm thinking maybe that is a better UX.

def valid?
  return false if any_errors_detected?

   #... other stuff it does
end

@matthewmcgarvey
Copy link
Member

maybe the valid? check would just see if any errors are present?

@jwoertink
Copy link
Member Author

Yup, exactly. Ok. I'll make that change and push it up.

…se for you. If you're setting an error, it's not valid.
…tch the valid property. This is less of a short circuit and more of just error detection
@jwoertink
Copy link
Member Author

jwoertink commented Nov 13, 2020

Alright, so this doesn't really fix my original thought behind #529, but it does allow me to do some of what I intended by setting a custom error on operations.

I'm thinking maybe in a future PR we could have some way to "halt" the entire chain of callbacks and such. Something that says "don't run any more callbacks, don't run any other validations... nothing... just stop and exit the operation". For now, this will be a good improvement.

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.

Allow for custom errors on operations
2 participants