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

present: true not working as expected on embedded values #25

Open
jits opened this issue Dec 12, 2018 · 3 comments
Open

present: true not working as expected on embedded values #25

jits opened this issue Dec 12, 2018 · 3 comments

Comments

@jits
Copy link

jits commented Dec 12, 2018

Hi – firstly, thank you for this library!

I've noticed that using present: true on embedded values throws an error (seemingly every time).

To reproduce: if you add , present: true to

attribute :name, String
the test will fail with error messages like:

ShallowAttributes::MissingAttributeError: Mandatory attribute "name" was not provided
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:214:in `block in define_mandatory_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:212:in `each'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:212:in `define_mandatory_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:40:in `initialize'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type.rb:80:in `new'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type.rb:80:in `type_instance'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type.rb:57:in `coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/class_methods.rb:117:in `city='
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:227:in `block in define_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:226:in `each'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:226:in `define_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:93:in `attributes='
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:142:in `coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type.rb:57:in `coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type/array.rb:31:in `block in coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type/array.rb:30:in `map!'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type/array.rb:30:in `coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/type.rb:57:in `coerce'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/class_methods.rb:117:in `addresses='
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:227:in `block in define_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:226:in `each'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:226:in `define_attributes'
    /Users/jits/Src/shallow_attributes/lib/shallow_attributes/instance_methods.rb:38:in `initialize'
    /Users/jits/Src/shallow_attributes/test/custom_types_test.rb:62:in `new'
    /Users/jits/Src/shallow_attributes/test/custom_types_test.rb:62:in `block (4 levels) in <top (required)>'

I may be able to take a closer look and work on a PR when I have more time; any pointers are gladly welcome.

@davydovanton
Copy link
Owner

Hey, thanks for report. Will check it when will have time (hope on new years holidays) 👍

@X1ting
Copy link

X1ting commented Jul 19, 2019

So, I found the root cause of the issue.
We are defining all mandatory attributes and only after it we are creating nested classes.
So, in this step ->
https://github.com/davydovanton/shallow_attributes/blob/master/lib/shallow_attributes/type.rb#L80
we have mandatory attributes for User, but we call .new without args and it threw an exception.
I haven't ideas how to fix it properly, just thoughts.
We have two ways:

  1. Define nested mandatory attributes later
  2. Call nested Class.new with params
    @jits @davydovanton WDYT ?

@jits
Copy link
Author

jits commented Jul 23, 2019

@X1ting – nice find. To be honest, I'm not sure I fully understand the impact, but your option (2) sounds like it makes the most sense and is in line with how shallow_attributes expects objects to be initialised (i.e. all required data must be provided at object initialisation). Having the nested objects initialised in the same way makes a lot of sense.

Let me know if there's something I can help further with.

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

3 participants