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

False-positive when validating a nested datetime in an array of hashes #732

Open
colszowka opened this issue Oct 18, 2023 · 2 comments
Open

Comments

@colszowka
Copy link

colszowka commented Oct 18, 2023

Hey all, first of all thanks for the great library, we're using this gem a lot (alongside a variety of other tools from the dry-rb ecosystem) and find it super-useful 🎉 . Thank you all for the work you're putting into these great tools!

Describe the bug

I have encountered a very strange behaviour when dealing with validations on hashes inside an array, where the nested datetime fails to validate, whereas the same rule at the top level works just fine.

To Reproduce

require "pp"
require "dry-validation"
module Types
  include Dry.Types(default: :nominal)
end

class Hmm < Dry::Validation::Contract
  params do
    optional(:works_at).filled(Types::Params::DateTime)

    optional(:widgets).maybe(:array).each do
      hash do
        optional(:fails_at).filled(Types::Params::DateTime)
      end
    end
  end
end

pp Hmm.new.call(works_at: Time.now.iso8601, widgets: [{fails_at: Time.now.iso8601}]).errors.to_h
# => {:widgets=>{0=>{:fails_at=>["must be a date time"]}}}

Expected behavior

The example above should be valid, but is not. I also tried experimenting with a variety of other types but all have the same issue.

My environment

  • Affects my production application: YES (such that I cannot make valid parameters pass through my contract)
  • Ruby version: 3.2.2
  • OS: Linux
  • dry-validation 1.10.0 (latest)
  • dry-schema 1.13.3 (latest)
  • dry-types 1.7.1 (latest)
@colszowka
Copy link
Author

On another note, I attempted to make a workaround using a complex dry-type instead of the contracts DSL, but this actually runs into the same issue as well

class Hmm < Dry::Validation::Contract
  params do
    optional(:works_at).filled(Types::Params::DateTime)

    TYPE = Types::Strict::Array.of(Types::Hash.schema(fails_at: Types::Params::DateTime))

    optional(:widgets).maybe(TYPE)
  end
end

pp Hmm.new.call(works_at: Time.now.iso8601, widgets: [{fails_at: Time.now.iso8601}]).errors.to_h
# => {:widgets=>{0=>{:fails_at=>["must be a date time"]}}}

The only way I can get this to work is to skip the iso8601 parsing part and use Types::Params::Time and pass Time.now instead, but that's kinda missing the point because I want to return structured errors when a user submits an invalid timestamp

@colszowka
Copy link
Author

I came up with a way how to work around this issue, by making my nested date time a string on the contract and then performing the type casting and param validation myself in a rule. This then yields the desired result:

require "pp"
require "dry-validation"
module Types
  include Dry.Types(default: :strict)
end

class Hmm < Dry::Validation::Contract
  params do
    optional(:works_at).filled(Types::Params::DateTime)

    optional(:widgets).maybe(:array).each do
      hash do
        optional(:fails_at).filled(Types::Strict::String)
      end
    end
  end

  rule(:widgets) do
    value.each.with_index do |entry, i|
      next unless entry[:fails_at]
      value[i][:fails_at] = Types::Params::DateTime[entry.fetch(:fails_at)]
    rescue => err
      key([:widgets, i, :fails_at]).failure "is invalid"
    end
  end
end

pp Hmm.new.call(works_at: Time.now.iso8601, widgets: [{fails_at: Time.now.iso8601}]).errors.to_h
# => {}
pp Hmm.new.call(works_at: Time.now.iso8601, widgets: [{fails_at: "this is not valid"}]).errors.to_h
# => {:widgets=>{0=>{:fails_at=>["is invalid"]}}}

However, this is not super great so I think it would be nice to get this bug fixed regardless of a workaround being available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant