Implementation of support for I18n and custom messages for validation errors

[afuno]

Your gem is a good solution with a fresh perspective on some things. Thank you.

But I18n support for adding translations for different languages is very much lacking. Also, there is a lack of specifying custom text bypassing I18n. In general, this is how the ActiveModel works.

Can you please tell me if you plan to add support for this in the near future?

[sunny]

Hey afuno! Thank you for bringing this up.

I don’t mind adding I18n, but until now I didn’t have the need for it. The way I currently handle I18n is something like this:

class Pay < Actor
  input :amount

  def call
    if error?
      fail!(error: I18n.t("pay.errors.payment_processor_failure"))
    end

    # …
  end

  private

  def error?
    true
  end
end

What kind of syntax or need did you have in mind?

[afuno]

I rather meant something like this:

input :amount, allow_nil: { message: "The amount is required" }
# or
input :amount, allow_nil: { message: :amount_required }
# or
input :amount, allow_nil: { message: :required } # <= Here the argument name is automatically determined

Based on this example: https://guides.rubyonrails.org/active_record_validations.html#message

[afuno]

Would like to avoid using the I18n class directly by writing keys like this: amount_required. And so that under the hood I18n recognizes the class of service (pay) and substitutes the specified key - pay.amount_required.

[sunny]

Sounds good! I’d be happy to accept a Pull-Request with this syntax for inputs 👍🏻

I’d expect keys to be set actor.inputs.my_namespace/my_actor.input.be_admin with variables %{input_key} and %{must_key} and %{value}`.

[afuno]

In addition to what I wrote above, I want to consider one example, which I consider a vulnerability.

This is the code from the README:

input :user,
  must: {
    be_an_admin: -> user { user.admin? }
  }

If it fails, then the error text will look like this:

"Input #{key} must #{name} but was #{value.inspect}"

That is, all the data of the record will be displayed in the error text. This is very bad. If use .call instead of .result, then this can get out of the way when developing an application in API mode.

It seems to me that the presence of I18n in your gem is very necessary.

[afuno]

In a Rails application in API mode, projects have a mechanism that catches all errors in the application and renders them in JSON for the frontend.

Currently your gem is returning error texts from input which cannot be rendered elegantly for the frontend. These texts cannot be processed.

[sunny]

Security-wise in Ruby applications you should not catch all exceptions and show the error message. This can bring up a lot of unintended data as there are a lot of possible exceptions.

I’d encourage you to specifically show only messages from exceptions you know are safe to show.

In this gem the input handling was never meant to be user-facing, but with your suggestions we can find ways to make it safe to show to users 👍🏻

[sunny]

Perhaps we could change the default message from:

Input #{input_key} must #{must_key} but was #{user.inspect}

To

Input #{input_key} must #{must_key}

Even though this is not as easy to understand for developers, there is less risk of leaking unintended data to users when the errors are pushed to users.

[afuno]

What do you think about such an implementation as an alternative? I mean an implementation that keeps the current version and also supports a more advanced one?

input :user,
      type: User,
      allow_nil: false,
      # must: {
      #   be_admin: ->(user) { user.admin? }
      # }
      must: {
        be_admin: {
          call: ->(user) { user.admin? }, # Alternative names for `call`: state, condition
          message: (lambda do |input_key, must_key, user| # Here are all the values that are described in the current error texts
            # "Input #{input_key} must #{must_key} but was #{user.inspect}" # <= current error text
            "Input #{input_key} must #{must_key} but was #{user.kind}" # <= do not show an object with sensitive data
            # or simply
            # "The user must be an admin" # <= any other text at the request of the developer inside the Rails application
          end)
        }
      }

It seems to me that this is easier to implement than the I18n integration. In such a case, I18n becomes optional and will be decided within the Rails application.

[sunny]

Indeed, that’s a very flexible way of handling messages, which doesn’t add a dependency to I18n. This also fixes the fact that I18n accepts strings as variables but not larger objects such as user.

I’d also be happy to accept a Pull-Request with this, if you prefer.

[afuno]

@sunny I will try to prepare a draft version as soon as possible. In that case, I'll focus on this option, which doesn't require an I18n dependency in the gem.

[afuno]

Draft: #82

[sunny]

Added to v3.3.0 🎉