These are chat archives for dry-rb/chat

1st
Apr 2016
Fran Worley
@fran-worley
Apr 01 2016 09:16

@solnic I've been reading: apotonick/reform#347

Is there any point in me upgrading dry-v to 0.7 or will I be unable to use it with reform?

Michał Pietrus
@blelump
Apr 01 2016 09:35
@fran-worley currently you won't be able to use 0.7 DSL with Reform
I've opened a PR for that , but it's not ready to use, however if you enjoy the new dry-v syntax, you might use my branch
Piotr Solnica
@solnic
Apr 01 2016 09:38
@fran-worley this ^
Fran Worley
@fran-worley
Apr 01 2016 09:42
@blelump @solnic Thanks. I've been reading the thread.
Piotr Solnica
@solnic
Apr 01 2016 09:50
@fran-worley @blelump it should be easy to do the upgrade and keep current behavior, we had other considerations wrt automatic coercions in dry-v vs reform behavior etc
Fran Worley
@fran-worley
Apr 01 2016 09:50
@solnic That is just what I was about to say...
To fundamentally redesign Reform and when such a large user base use AMV will take forever. I'd rather see a fix to enable the new API and keep the existing behaviour now, and leave the debates about rewriting Reform for another time...
Michał Pietrus
@blelump
Apr 01 2016 09:54
@fran-worley , so far we don't have a plan to do so, this post https://github.com/apotonick/reform/pull/347#issuecomment-200368229 summarizes TODOs
and essentially what has to be done is error handling for nested schemas
Fran Worley
@fran-worley
Apr 01 2016 09:57
@blelump as ever, things are far more complex than they appear on the surface...
Thank you guys for all of your hardwork, I'm just looking forward to being able to use the new API on things like this...
validation :default do
  configure { |config| config.messages_file = 'config/error_messages.yml' }
  key(:name, &:filled?)
  key(:quantity) { |quantity| quantity.optional? | (quantity.filled? & quantity.int?) }
  key(:time_unit) { |time_unit| time_unit.optional? | (time_unit.filled? & time_unit.inclusion?(TIME_UNITS.map(&:to_s))) }

  def optional?(value)
    !(form.quantity.present? || form.time_unit.present?)
  end
end
Alexander Chernik
@achernik
Apr 01 2016 13:26

I’m trying to figure out how to fit dry-v into use case where you don’t want to validate data coming form user directly, but check if an object is valid for some operation and provide error messages.

say user wants to confirm previously constructed order.
then i'd use something like this pseudocode:

OrderConfirmationSchema = Dry::Validation::ObjectContract do
  attr(:line_items).each { in_stock? } 
  attr(:start_time) { still_available? }
end

OrderConfirmationSchema.(order)
I'm sure I'm missing something, as this approach smells a bit like AM:V
Hannes Nevalainen
@kwando
Apr 01 2016 13:42
@solnic I want something like this to work
optional(:meta).required(:hash?)
I only wanna check that if meta is given it is a hash
Piotr Solnica
@solnic
Apr 01 2016 13:43
this should work
Hannes Nevalainen
@kwando
Apr 01 2016 13:44
It will reject empty hashes
```rubytoptional(:meta){ hash? }
Piotr Solnica
@solnic
Apr 01 2016 13:44
optional(:meta) { hash? }
maybe won’t work with hashes IIRC, but please try too
Hannes Nevalainen
@kwando
Apr 01 2016 13:45
maybe will do the wrong thing
I dont wanna allow nil
Piotr Solnica
@solnic
Apr 01 2016 13:46
ok so just use a block
Hannes Nevalainen
@kwando
Apr 01 2016 13:47
/Users/kwando/projects/work/qr-registry/vendor/bundle/ruby/2.2.0/gems/dry-validation-0.7.3/lib/dry/validation/schema/key.rb:27:in `instance_eval': wrong number of arguments (0 for 1..3) (ArgumentError)
    from /Users/kwando/projects/work/qr-registry/vendor/bundle/ruby/2.2.0/gems/dry-validation-0.7.3/lib/dry/validation/schema/key.rb:27:in `hash?'
this is for optional(:meta) { hash? }
Piotr Solnica
@solnic
Apr 01 2016 13:49
@kwando oh damn
Hannes Nevalainen
@kwando
Apr 01 2016 13:49
This is the only red test before I got another app ported over to the dry-* ecosystem =)
Piotr Solnica
@solnic
Apr 01 2016 13:50
I should add type macro
optional(:meta).type(:hash?)
block with a single predicate looks awful
Hannes Nevalainen
@kwando
Apr 01 2016 13:53
would allowing blocks to macros make sense? something like optional(:age).required{ gteq?(23) & lteq?(99) }
Piotr Solnica
@solnic
Apr 01 2016 13:53
yes, I plan to add this
Hannes Nevalainen
@kwando
Apr 01 2016 13:53
cool =)
FWIW I dont think blocks with a single predicate looks that bad
concratz on the latest release, really awesome =)
Piotr Solnica
@solnic
Apr 01 2016 13:56
thanks <3
Hannes Nevalainen
@kwando
Apr 01 2016 13:56
thank you
Piotr Solnica
@solnic
Apr 01 2016 13:58
:dancers:
Tim Cooper
@coop
Apr 01 2016 14:05
Morning @solnic
Piotr Solnica
@solnic
Apr 01 2016 14:06
hey @coop
Tim Cooper
@coop
Apr 01 2016 14:08

I’ve got 99% of the JSON schema done, just having an issue with this spec - https://github.com/dry-rb/dry-validation/pull/89/files#diff-82623613dc3ad9b87365908bf5bc4421R32

I don’t understand why the value being passed to age is changing the value passed to email. Would you mind having a look when you get a chance?

Piotr Solnica
@solnic
Apr 01 2016 14:23
@coop :age won’t be coerced to int
Tim Cooper
@coop
Apr 01 2016 14:24
I get that but why is that changing :email from an empty string to nil?
If :email is an empty string and a I pass :age as a string :email is being coerced to nil but if I pass :age as an int :email is left as an empty string.
Piotr Solnica
@solnic
Apr 01 2016 14:26
not sure if I follow
Tim Cooper
@coop
Apr 01 2016 14:27
Given the two specs starting on L32 (https://github.com/dry-rb/dry-validation/pull/89/files#diff-82623613dc3ad9b87365908bf5bc4421R32) I would think that the validation message for email should be the same.
Piotr Solnica
@solnic
Apr 01 2016 14:28
one sec
please rebase on top of master in your dry-types branch
that’s one thing
dry-rb/dry-validation@d2e4790 <= @coop
make sure your json branch in dry-types is up to date with master
Tim Cooper
@coop
Apr 01 2016 14:31
dry-types is up to date.
I don’t see why changing the input from 19 to 18 would change the validation message that email outputs.
Piotr Solnica
@solnic
Apr 01 2016 14:32
it doesn’t, I didn’t see this behavior
Tim Cooper
@coop
Apr 01 2016 14:32
oh
OH
I did something dumb.
Piotr Solnica
@solnic
Apr 01 2016 14:33
the mistake was that age was a string, json doesn’t coerce it
Tim Cooper
@coop
Apr 01 2016 14:34
Yeah that was expected - I was more interested in the email message changing. I believe that was because I was pointing my dry-types locally to my GH branch when I meant for it to be pointed locally.
Anyway, sorted now.
Tim Cooper
@coop
Apr 01 2016 14:41
How do you have your local dev configured so that you can build against dry-* gems locally but when you push to GitHub it uses the released versions?
Piotr Solnica
@solnic
Apr 01 2016 14:43
@coop re-run w/o cache on travis
Michał Pietrus
@blelump
Apr 01 2016 14:56
@fran-worley , you around ?
Ralf Schmitz Bongiolo
@mrbongiolo
Apr 01 2016 14:58
@solnic will you favor only nested schema instead of nested key on newer versions of dry-v?
Piotr Solnica
@solnic
Apr 01 2016 14:59
@mrbongiolo nested keys will be removed in 0.8.0
it’s already gone in the docs (hopefuly)
Ralf Schmitz Bongiolo
@mrbongiolo
Apr 01 2016 15:01
Ok, fine ;)
Michał Pietrus
@blelump
Apr 01 2016 15:01
@mrbongiolo , Hi, any spare time to implement dry-v errors handling for nested schemas in Reform? ;-)
Ralf Schmitz Bongiolo
@mrbongiolo
Apr 01 2016 15:02
I can make some, just gotta finish something here
Piotr Solnica
@solnic
Apr 01 2016 15:02
@mrbongiolo it’s gonna simplify implementation A LOT
Ralf Schmitz Bongiolo
@mrbongiolo
Apr 01 2016 15:03
@solnic I assume so
Tim Cooper
@coop
Apr 01 2016 15:05

@solnic

dry-types dry-rb/dry-types#61
dry-v dry-rb/dry-validation#89

For dry-v the last commit will need to be replaced with a bump in dry-types deps when it is released. I’m going to head to bed, let me know if there is anything you want changed.

Piotr Solnica
@solnic
Apr 01 2016 15:06
@coop thanks, I’ll check it out during the weekend
Tim Cooper
@coop
Apr 01 2016 15:06
No problems. Catch you later.
Fran Worley
@fran-worley
Apr 01 2016 15:20
@blelump I am now...
Michał Pietrus
@blelump
Apr 01 2016 15:30
@fran-worley would you like to help with that PR regarding new dry-v in Reform? A good start would be to make the build green
Fran Worley
@fran-worley
Apr 01 2016 15:58
@blelump Sure. Happy to take a look.
Fran Worley
@fran-worley
Apr 01 2016 16:34
This message was deleted
Fran Worley
@fran-worley
Apr 01 2016 17:35
@blelump Looks like we can't use to_nested_hash in its current form
It comes from active record so if you try to call it on a Struct, it will fail as the method doesn't exist.
Michał Pietrus
@blelump
Apr 01 2016 17:41
@fran-worley , disposable implements it for his own needs . its defined here
it's interesting it works locally
Fran Worley
@fran-worley
Apr 01 2016 17:44

@blelump at this point: https://github.com/blelump/reform/blob/new-dry-validation/lib/reform/validation.rb#L36

I get errors when a Struct is used as the model (these two tests: https://github.com/blelump/reform/blob/new-dry-validation/test/validate_test.rb#L36-L49)

The errors on these two go away if I replace the valid code with:

def valid?
    #bit of a hack, but to_nested_hash only works with AR objects so structs raise an error...
    hash = respond_to?(:to_nested_hash) ? to_nested_hash : @fields
    Groups::Result.new(self.class.validation_groups).(hash, errors, self)
end
With that and some changes to error messages config, I've got all the tests passing
Michał Pietrus
@blelump
Apr 01 2016 17:50
yup, but the problem is that we cant solve it with such approach . It occurs because that validation module is used by both dry-v and AM:V and crashes with AM:V
we need to move that logic to the dry-v piece
Fran Worley
@fran-worley
Apr 01 2016 17:51
Yeah I'm seeing the problem now. I wasn't really suggesting that as a fix, more just trying to prove to myself that that was the problem...
Michał Pietrus
@blelump
Apr 01 2016 17:51
:-)
sure
there's a ton of these validation methods and modules
Fran Worley
@fran-worley
Apr 01 2016 17:53
I'm getting the whole, can't we just remove AMV code and move on now.
Michał Pietrus
@blelump
Apr 01 2016 17:53
@fran-worley , gotta go, enough for today . Would you like an access to my fork ?
Fran Worley
@fran-worley
Apr 01 2016 17:54
That would be great if you wouldn't mind
This message was deleted
Michał Pietrus
@blelump
Apr 01 2016 17:54
@fran-worley , I dont think so . Plan for reform without AM:V is v3.0 ?
Fran Worley
@fran-worley
Apr 01 2016 17:54
I know we can't, it would just be alot easier!
Daniel Gollahon
@dgollahon
Apr 01 2016 17:55
Hi, I've just started using Dry Types. I was wondering, if I'm using default values is there a way to initialize a dry struct without providing the key that corresponds to the default value?
Michał Pietrus
@blelump
Apr 01 2016 17:55
@fran-worley :-) , I gave you access
Fran Worley
@fran-worley
Apr 01 2016 17:56
Thats to disposable. Don't I need reform?
Michał Pietrus
@blelump
Apr 01 2016 17:56
Yup, just added to Reform
Fran Worley
@fran-worley
Apr 01 2016 17:57
Thanks I'll see what I can do
Michał Pietrus
@blelump
Apr 01 2016 17:57
sure, thanks for looking at it !
Daniel Gollahon
@dgollahon
Apr 01 2016 17:59
for clarification, I'm looking to do something like
class Foo < Dry::Types::Struct
  attribute :bar, Types::Strict::String
  attribute :baz, Types::Strict::String.default('default!')
end

Foo.new(bar: 'test') # => Foo with bar == 'test', baz == 'default!'
Piotr Solnica
@solnic
Apr 01 2016 18:05
@dgollahon set constructor_type(:schema)
it’s not documented because I don’t like it ;)
srsly though, this should be documented
Daniel Gollahon
@dgollahon
Apr 01 2016 18:07
awesome! thanks :)
François Bernier
@fbernier
Apr 01 2016 18:36
@solnic What do you think about having an option to have the attributes of a Dry::Types::Structas writable ?
Piotr Solnica
@solnic
Apr 01 2016 18:50
@fbernier people can add an extension if they want it but I don’t want to have it in the main lib, in general we’re trying to promote objects that don’t expose public APIs to mutate them
structs are meant to be “simple” data objects, w/o any complex behaviors or APIs to mutate them
Piotr Solnica
@solnic
Apr 01 2016 18:56
dry-rb/dry-validation#90 <== any thoughts?
Piotr Solnica
@solnic
Apr 01 2016 19:55
@fran-worley thanks for fantastic feedback. I will chime in later today or tomorrow. I need to give this more thoughts.
Fran Worley
@fran-worley
Apr 01 2016 19:58
@solnic no worries. IMO any significant changes to the DSL should very carefully considered as a library that re-writes itself every week is not ideal for those of use having to recode our apps each time as a result!
Piotr Solnica
@solnic
Apr 01 2016 20:24
Yep. Alhough in thus updating would be simple
Daniel Gollahon
@dgollahon
Apr 01 2016 20:25
can you inherit from a class that has inherited from the dry type struct? i ran into some unexpected behavior
require 'dry-types'

module Types
  include Dry::Types.module
end

class Foo < Dry::Types::Struct
  attribute :thing, Types::Strict::String
end

Foo.new(thing: 'hello, world!') #  => #<Foo thing="hello, world!">
Foo.new({}) # => Dry::Types::StructError: [Foo.new] :thing is missing in Hash input
# hurray^

class Bar < Foo
end

hmm = Bar.new(thing: 'hello, world!') #  => #<Bar>
hmm.thing # => 'hello, world!'
# seems ok

wait_wat = Bar.new({}) # => #<Bar>
# woah, definitely not hurray^
wait_wat.thing # => nil
# huh? ^
# :(
Daniel Gollahon
@dgollahon
Apr 01 2016 21:19
On another note, have you guys considered erroring on unexpected keys in the constructor (like the anima gem does)?
require 'dry-types'
require 'anima'

module Types
  include Dry::Types.module
end

class Foo < Dry::Types::Struct
  attribute :thing, Types::Strict::String
end

Foo.new(thing: 'hello, world!', other_thing: 'goodbye, cruel world!') #  => #<Foo thing="hello, world!">
# other_thing is swallowed without my notice^

class Bar
  include Anima.new(:thing)
end

Bar.new(thing: 'hello, world!', other_thing: 'goodbye, cruel world!') #  => Anima::Error: Bar attributes missing: [], unknown: [:other_thing]
# ah! i probably did not mean to do that^
Piotr Solnica
@solnic
Apr 01 2016 21:49
@dgollahon I don't like this type of strictness because it doesn't work well with building structs from data from the db where often you may have additional keys for stuff like joins but you don't want to have them as part of struct state. BUT we could have a constructor which has this behaviour as in some contexts it could be useful.
Daniel Gollahon
@dgollahon
Apr 01 2016 23:37
Ah, yeah, I think that's reasonable. From my perspective it would definitely be useful as an option to enforce additional strictness.