Open ianwhite opened 4 years ago
Feel free to open a draft PR early if you'd like to get some feedback sooner.
Will do, when the next item (simplify ResultSet) is checked off.
Closing in favour of #595
I think this issue should stay open until it's resolved, because I believe other people would also like to get notified whenever it gets solved and if it's open it won't be forgotten (hopefully).
Hi, I'm happy to do that. I'm hoping to get some time to work on the new plan outlined in #595 over the Xmas break.
Hi @ianwhite and @solnic! First I just wanted to take a moment and thank you and the rest of the devs involved in this project for all of your efforts. It's really appreciated, and I know how hard it is to maintain projects like these.
That said I was hoping to find out what the state of this feature request is? I see from the threads that some of the work has shifted to dry-struct and dry-schema, and that work appears to have been completed, at least in part with the changes from https://github.com/dry-rb/dry-struct/pull/139 and https://github.com/dry-rb/dry-schema/pull/256. I also see that https://github.com/dry-rb/dry-validation/pull/595 has been closed, presumably with the intention of extracting some of the logic from dry-schema and dry-struct. Is that still the plan, and is this feature still viable?
I'd be willing to take a look and see what needs to be be changed, but I might need some direction. Let me know when you get a chance and thanks again for your work here!
@jameskbride hey 😄 this is on hold but I think @ianwhite is still interested in working on this, so please try to sync up with him first.
Hey, I’ve had a bunch of life issues recently interfering with my available time, but am still keen to work on this. I also want to grok the recent changes to dry-schema to make sure the approach still holds. Happy to discuss @jameskbride
On 26 Mar 2020, at 20:26, Piotr Solnica notifications@github.com wrote:
@jameskbride https://github.com/jameskbride hey 😄 this is on hold but I think @ianwhite https://github.com/ianwhite is still interested in working on this, so please try to sync up with him first.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dry-rb/dry-validation/issues/593#issuecomment-604321050, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABOXB3ADBLP4XF7A7YEMTRJMNUPANCNFSM4I55QWAA.
@ianwhite @jameskbride I'm really interested in this feature as well and probably will have some bandwidth to help on this!
I am composing contracts in this way. What do you think?
class EmployeeContract < Dry::Validation::Contract
json do
required(:concepts).array(:hash)
end
rule(:concepts).each do
result = ConceptContract.new.call(value)
unless result.success?
meta_hash = { text: 'invalid concept' }.merge(result.errors.to_h)
key.failure(meta_hash)
end
end
end
@anmacagno I think it's perfectly fine to do it like that for the time being, but eventually we definitely want a dedicated feature that would make it easier
I’m going to have some time to work on this tomorrow (Australian time). I’ll jump on Zulip from about 9am Australian Eastern Time.
On 21 May 2020, at 19:12, Piotr Solnica notifications@github.com wrote:
@anmacagno https://github.com/anmacagno I think it's perfectly fine to do it like that for the time being, but eventually we definitely want a dedicated feature that would make it easier
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dry-rb/dry-validation/issues/593#issuecomment-631977236, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABOXB3K4LHQLP5BNH5ESTRSTV6FANCNFSM4I55QWAA.
@anmacagno thx for ur snippet. Just a short extension, when using array(:hash)
any key coming inside concepts
json will go through. Therefore we use the schema of the child contract in the parent contract schema.
class EmployeeContract < Dry::Validation::Contract
json do
required(:concepts).array(ConceptContract.schema)
end
rule(:concepts).each do
result = ConceptContract.new.call(value)
unless result.success?
meta_hash = { text: 'invalid concept' }.merge(result.errors.to_h)
key.failure(meta_hash)
end
end
end
@solnic This feature would much improve this already great library!
@tbsvttr ah thanks, I know it's long-time coming but we'll get to implement it eventually 🙂
@tak1n While your solution works very well it has the downside that now each error hash has this text key value pair added which neither adds very useful additional information and also could lead to the confusion that the validated field in a hash has a field named text. Is there a way to get rid of it?
Hi- are we getting closer to allowing contracts to be reused within another contract?
@jacobtani not really :( currently it doesn't seem like there's anybody who'd have time to work on this feature. Like I mentioned, we'll get to this eventually. It's on my radar and it's going to be my priority after we're done with Hanami 2.0 (FWIW).
This is my improvement to @tak1n's and @anmacagno's solutions:
Dry::Validation.register_macro(:contract) do |macro:|
contract_instance = macro.args[0]
contract_result = contract_instance.(value)
unless contract_result.success?
# Generate a message string instead of adding a
# dummy 'text' value. I don't love this, and I
# wonder if it's possible to improve this.
msg = flattened_error_messages(
contract_result.errors.to_h
).join(', ')
key.failure(msg)
end
end
def flattened_error_messages(errors, path = [])
errors.each_with_object([]) do |(key, value), msgs|
case value
when Array
value.each { |v| msgs << "#{(path + [key]).join('.')} #{v}" }
when Hash
msgs.concat(flattened_error_messages(value, path + [key]))
end
end
end
# ...
class MyContract < Dry::Validation::Contract
params do
# hash:
required(:a).hash(A.schema)
# array of hashes:
required(:b).array(:hash, B.schema)
end
rule(:a).validate(contract: A)
rule(:b).each(contract: B)
end
Updated variant, now with proper error message nesting. Full solution:
# Use it like this:
#
# MyContract = Dry::Validation.Contract do
# params do
# # for a hash:
# required(:a).hash(OtherContract.schema)
#
# # for an array of hashes:
# required(:b).array(:hash, OtherContract.schema)
# end
#
# rule(:a).validate(contract: OtherContract)
# rule(:b).each(contract: OtherContract)
# end
#
Dry::Validation.register_macro(:contract) do |macro:|
contract_instance = macro.args[0]
contract_result = contract_instance.(value)
unless contract_result.success?
errors = contract_result.errors
errors.each do |error|
key(key.path.to_a + error.path).failure(error.text)
end
end
end
If you're looking to just merge schemas at the top level like myself, looks like you can do this:
class MyContract < Dry::Validation::Contract
params(MyOtherContract.schema, MyThirdContract.new(args).schema) do
required(:a).value(:string)
end
end
This is in the docs, but I had missed it.
If you need the rules you can copy them over too:
my_contract.rules = my_other_contract.rules + my_third_contract.rules
Updated variant, now with proper error message nesting. Full solution:
# Use it like this: # # MyContract = Dry::Validation.Contract do # params do # # for a hash: # required(:a).hash(OtherContract.schema) # # # for an array of hashes: # required(:b).array(:hash, OtherContract.schema) # end # # rule(:a).validate(contract: OtherContract) # rule(:b).each(contract: OtherContract) # end # Dry::Validation.register_macro(:contract) do |macro:| contract_instance = macro.args[0] contract_result = contract_instance.(value) unless contract_result.success? errors = contract_result.errors errors.each do |error| key(key.path.to_a + error.path).failure(error.text) end end end
This should be a built-in feature
This feature will allow contracts to be composed of other contracts, optionally mounted at input paths.
Example
Implementation
Initial on branch composable-contracts
Future work
Resources