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

Problems between checks within the same input #95

Closed
afuno opened this issue Oct 19, 2022 · 8 comments
Closed

Problems between checks within the same input #95

afuno opened this issue Oct 19, 2022 · 8 comments

Comments

@afuno
Copy link
Contributor

afuno commented Oct 19, 2022

There is this code in the application:

class ApplicationService
  include ServiceActor::Base
end
class CurrencyService < ApplicationService
  input :currency,
        type: String,
        inclusion: {
          in: %w[USD EUR],
          message: ->(value:, **) { "Incorrect currency: #{value}" }
        }
end

When using it:

CurrencyService.call(currency: :GBP)

Gives an error:

ServiceActor::ArgumentError: Incorrect currency: GBP

That is, the check for the type of the input value is ignored.

@sunny
Copy link
Owner

sunny commented Oct 20, 2022

Perhaps this is an ordering issue. It looks like it would make sense to always have type be called before inclusion.

Would changing the order of the checks solve this for you?

@afuno
Copy link
Contributor Author

afuno commented Oct 20, 2022

In theory, it is correct to take the first check and fall immediately on it. If the first check passes, then take the second and fall on it. And so on until the end.

@afuno
Copy link
Contributor Author

afuno commented Oct 20, 2022

That is, there is not a problem in ordering, but a problem in the positioning of these checks. That is, first need to take the first rule and it doesn’t matter what it is - type or inclusion.

@afuno
Copy link
Contributor Author

afuno commented Oct 20, 2022

In theory need to add all the checks to an array and run them in a loop. But I'm not sure. 😬 Need to discuss the approach.

@sunny
Copy link
Owner

sunny commented Oct 20, 2022

Oh, I see, you mean that input :currency, type: "String", inclusion: %w[A B] would act differently to input :currency, inclusion: %w[A B], type: "String".

That would make total sense! However it would require rewriting how conditions are applied internally. I’d be happy to accept a pull-request going in this direction.

Or else, a smaller change that would fix this specific problem here would be to place base.include(ServiceActor::Conditionable) before TypeCheckable in lib/service_actor/base.rb.

@afuno
Copy link
Contributor Author

afuno commented Oct 20, 2022

Here we are talking about all the checks, and not just about these two. Those two are just examples. In an amicable way it is necessary to introduce new logic. I'll try to provide a draft as soon as possible.

@afuno
Copy link
Contributor Author

afuno commented Oct 23, 2022

@sunny Draft: #97

@sunny
Copy link
Owner

sunny commented Dec 29, 2022

Released in v3.6.0 🚀

@sunny sunny closed this as completed Dec 29, 2022
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

No branches or pull requests

2 participants