One to many: undefined method `from_hash' for nil:NilClass on validate

[AxisOfEval]

Validate seems to run fine if I have no nested models. But if I have a has_many relation with a appropriate collection call in the form I get the above error.

My form code:

class ProjectForm < Reform::Form
  model      :project, on: :project

  properties [:name, :number_of_buildings, :number_of_parking_slots], on: :project

  validates :name,
            presence:     true
  validates :number_of_buildings,
            presence:     true
  validates :number_of_parking_slots,
            presence:     true

  collection :buildings do
    model    :building, on: :building

    properties [:name, :number_of_floors, :number_of_apartments], on: :building

    validates :name,
              presence:     true
    validates :number_of_floors
              presence:     true
    validates :number_of_apartments,
              presence:     true
  end
end

And of course, a Project has_many Buildings. I am using Reform 0.2.0.

Am I doing something I am not supposed to, or did I stumble on a bug?

[apotonick]

Can you remove all the on: options? You don't need this anymore since you're not including the Composition module.

[elvanja]

+1 for the issue

Form definition:

class BusinessProfileForm < Reform::Form
  include Reform::Form::ActiveRecord

  model :business_profile

  collection :business_profile_categories do
    property :places_category_name, getter: ->(data) { BusinessCategory.find_by_id(places_category_id).try(:name) }
    property :places_category_id
    validates :places_category_id, business_category: true
  end
end

Rendering the form:

-# TODO model is not yet available in reform, should use @form.model.new_record?
= form_for(@form, as: :business_profile) do |f|
  = f.fields_for :business_profile_categories do |ff|
    .form-group
      = ff.label :category, class: "col-lg-2 control-label"
      .form-inline.col-lg-10
        = ff.hidden_field :id, value: f.object.id
        = ff.hidden_field :places_category_id
        = ff.text_field :places_category_name, type: "text", placeholder: "category"

There are other fields in the mix, but all seems to be working until I have more than one item in the collection.
And interestingly, I can have as many as I like if they have been loaded from DB, but once I start adding new items, then it gets interesting.

The issue arises here https://github.com/apotonick/reform/blob/master/lib/reform/form.rb#L101
I've added some more details to the console and when form.validate is called, this is the sequence:

This is what enters the validate method:

{
"name"=>"Pizza Hut",
"username_attributes"=>{"username"=>"pizzahut", "id"=>"2"},
"business_profile_categories_attributes"=> {
  "0"=>{"id"=>"1", "places_category_id"=>"362", "places_category_name"=>"Pizza"},
  "1"=>{"id"=>"1", "places_category_id"=>"148", "places_category_name"=>"Food and Beverage"}
},
"business_profile_categories"=>[
  {"id"=>"1", "places_category_id"=>"362", "places_category_name"=>"Pizza"},
  {"id"=>"1", "places_category_id"=>"148", "places_category_name"=>"Food and Beverage"}
]}

The from_hash method gets the same params as above.
But then, from_hash is called two more times.
First with these params:

{"username"=>"pizzahut", "id"=>"2"}

And second time with these:

{"id"=>"1", "places_category_id"=>"362", "places_category_name"=>"Pizza"}

I am guessing that it is trying to run validations recursively for each nested set.
But, for some reason, this is where it breaks with this stack:

NoMethodError - undefined method `from_hash' for nil:NilClass:
  representable (1.7.1) lib/representable/deserializer.rb:50:in `deserialize'
  representable (1.7.1) lib/representable/deserializer.rb:44:in `call'
  representable (1.7.1) lib/representable/deserializer.rb:18:in `block in deserialize'
  representable (1.7.1) lib/representable/deserializer.rb:15:in `deserialize'
  representable (1.7.1) lib/representable/bindings/hash_bindings.rb:45:in `deserialize'
  representable (1.7.1) lib/representable/bindings/hash_bindings.rb:26:in `deserialize_from'
  representable (1.7.1) lib/representable/hash/collection.rb:27:in `update_properties_from'
  representable (1.7.1) lib/representable/hash.rb:34:in `from_hash'
  representable (1.7.1) lib/representable/deserializer.rb:50:in `deserialize'
  representable (1.7.1) lib/representable/deserializer.rb:44:in `call'
  representable (1.7.1) lib/representable/binding.rb:117:in `deserialize'
  representable (1.7.1) lib/representable/bindings/hash_bindings.rb:18:in `read'
  representable (1.7.1) lib/representable/binding.rb:65:in `read_fragment_for'
  representable (1.7.1) lib/representable/binding.rb:54:in `read_fragment'
  representable (1.7.1) lib/representable/binding.rb:36:in `block in uncompile_fragment'
  representable (1.7.1) lib/representable/binding.rb:88:in `represented_exec_for'
  representable (1.7.1) lib/representable/binding.rb:35:in `uncompile_fragment'
  representable (1.7.1) lib/representable/mapper.rb:71:in `uncompile_fragment'
  representable (1.7.1) lib/representable/mapper.rb:40:in `deserialize_property'
  representable (1.7.1) lib/representable/feature/readable_writeable.rb:6:in `deserialize_property'
  representable (1.7.1) lib/representable/mapper.rb:18:in `block in deserialize'
  representable (1.7.1) lib/representable/mapper.rb:17:in `deserialize'
  representable (1.7.1) lib/representable.rb:23:in `update_properties_from'
  representable (1.7.1) lib/representable/hash.rb:34:in `from_hash'
  reform (0.2.0) lib/reform/form.rb:105:in `from_hash'
  reform (0.2.0) lib/reform/form.rb:64:in `validate'
  reform (0.2.0) lib/reform/form/active_model.rb:34:in `validate'

If that helps, this is how the models looks:

class BusinessProfile < ActiveRecord::Base
  self.table_name = 'places_profile'
  self.primary_key = 'places_id'
  alias_attribute :id, :places_id
  has_many :business_profile_categories, -> { order 'category_type' }, foreign_key: 'places_id'
  has_many :business_categories, through: :business_profile_categories
end

class BusinessProfileCategory < ActiveRecord::Base
  self.table_name = 'places_profile_categories'
  belongs_to :business_profile, foreign_key: 'places_id'
  belongs_to :business_category, foreign_key: 'places_category_id'
end

Also, if I don't touch the business_profile_categories in the form (don't add new ones), regardless of how many I previously have, nothing breaks. Just breaks with new items.

Tried debugging, but got me nowhere.
Any help appreciated!
Also, if you need more details, just let me know!

[elvanja]

An interesting situation:

• with X (e.g. 3) categories in the join table already present
• form / partial is for drawn for each
• when I hit save
• all categories are normally saved / updated

Only when new categories are added it breaks with the already mentioned undefined method from_hash' for nil:NilClass`` error.

[apotonick]

The problem is that reform (actually, representable) can only "sync": Say you have the collection categories, each form in the main form must map to an existing Category instance in the profile.categories collection.

As soon as there's a form that doesn't have a corresponding category object the error will occur.

This "bug" is per design, as reform/representable don't know (yet) how to build a new category object for that collection if it is not existing.

Please, post profile.categories.inspect before rendering the form and the same before updating the form. Also, the params hash you use in form.validate would be helpful to find a generic solution/strategy for that issue.

[elvanja]

Instead of stack traces, I made the example you can use to play with this idea: https://github.com/elvanja/reform_example/tree/feature/dynamically_adding_songs

That is the known reform example project, I just added the ability to dynamically add new songs to an album. In this branch there is also a workaround. Basically, just ripped @gogogarrett's idea and before creating the form for an album, new songs are built, as many as there are in submitted params. I've denoted the workaround with # dynamically added songs workaround.

If you remove the few workaround lines, you can get the described behavior and the undefined method from_hash' for nil:NilClass on validate` error.

Also, if stack traces would help better, just let me know and I'll paste some.

P.S. Maybe the same idea can be used to solve the issue generically? For AR at least it should be easy.

[zoer]

I have the same problem with @obj = nil on collection with error message: undefined method from_hash' for nil:NilClass`

[miros]

This "bug" is per design, as reform/representable don't know (yet) how to build a new category object for
that collection if it is not existing.

But this makes reform pretty much unusable. Your just can not create a form that adds dynamically new objects to 1-n Relation? That is insane, is not it?

[apotonick]

Haha, I am insane, so what????

The initial thought is that you have to setup your model in the controller and pass it to the form. This clearly moves responsibility and knowledge about how to setup your data model into your domain - this is called abstraction and is the opposite of insanity. "Magic" and "smart" code is insane, where you don't know what's happening because of the code making assumptions you probably don't want.

I do see your point that this is inconvenient which is why we'll add a strategy on top of the insane code which helps you. Cool?

[RKushnir]

This is also a big deal for me. What's the best we can do at the moment? Any better options than just going through all the nested attributes and assigning them manually one by one?
At the very least, it should not fail in such a way, because this let's any user break your app.

[apotonick]

Right now, you have to do the following:

album = Album.new

params[:album][:songs].each do |s|
  album.songs << Song.new
end

AlbumForm.new(album)

It's not as easy to solve in reform/representable. The reason for that is that some users will want reform to skip nested forms when the pendant object doesn't exist, some users will expect reform to create the missing object, some users will want to find the object, and so on, so I can't just push "your" fix, we need a sustainable set of strategies. This is discussed here: trailblazer/roar#85

[tomash]

@elvanja i've cloned your repository (feature branch) and updating actually does not work as expected. trying to rename existing songs leaves them unchanged. only adding new songs in update works.
cc @apotonick

I've worked around this by the ugliest piece of code written in 2014 so far:

    album = Album.find(params[:id])
    (params[:album][:songs_attributes]).each do |k,s|
       song = Song.where(:id => s[:id]) || Song.new
       song.name = s[:name]
       song.save 
    end

    @form = Forms::AlbumForm.new(album)

But I wanted to use Reform to have my code cleaner, not uglier.

(and I stumbled upon it when investigating a similar issue -- nested objects not being updated on form save in update)

[apotonick]

The new representable will give you something like that (not ugly!).

collection :songs, find_strategy: lamdba { |hash| Song.where(:id => hash[:id]) || Song.new }

I'm still working out the details but this is roughly how it's gonna be. You will be able to customize the default behaviour of reform/representable when it comes to finding/deserializing/populating properties.

Popular strategies will then be abstracted to "pre-sets", e.g.

collection :songs, parse_strategy: :find_or_create

[tomash]

All right, but my current problem (lack of updates) does not come from find strategy. The attributes of nested objects are defined and passed all right, but not updated when neither form.save nor album.save is called.

[apotonick]

That is per design, Reform just goes and calls model.save, it's up to you (currently) to propagate that to attached models.

[tomash]

All right. Where should the code for updating belonging models reside? Currenly I have it in the controller's update method, but this feels wrong:

  def update
    @album = @company.albums.find(params[:id])

    # FIXME: this is wrong and should not be here at all
    params[:album][:songs_attributes].each do |k, sa|
      sng = @album.songs.find_or_initialize_by(:song_id => sa[:song_id])
      sng.title = sa[:title]
      sng.save
    end

    @form = AlbumForm.new(@album)
    ...

(code renamed to use album and songs, the actual code in actual application deals with different entities)

[apotonick]

You're mixing up two things: One problem is how to find/initialize the nested objects. You solved that using find_or_initialize_by (which I didn't even know it exists). Next, you have the #save issue, which should be handled by AR.

The saving could happen by overriding AlbumForm#save. The initialization stuff: leave it in the controller and add a # TODO: nick promised me to fix this. 😁

[elvanja]

A bit late to the party, but here goes anyway!

@tomash, I've created a working solution in reform_example. It now uses workflow class for handling form submits, admin manager (a sort of mapper) for storing albums and in controller there is a fix for dynamically added songs. It works both for adding songs to new or existing albums.

@tomash and @apotonick, maybe there are still a few places that may benefit with reform playing nicer with nested values. E.g. the "# dynamically added songs workaround" fix is now in two places. In controller and in form itself.

Also, there are a few things that seem a bit confusing. E.g. I had to access forms model in a strange way, and map yielded in form.save block has no ID's that are present in the original params. And, don't know if this is a bug, but when I try to add a new album without any songs, I get undefined method values' for nil:NilClassfromform.validate(params)`.

Hope this helps...

[elvanja]

I guess the ideal pseudo code would be:

class AlbumForm < Reform::Form
  model :album

  property :title
  validates :title, presence: true

  collection :songs, new_instance: lambda { model.songs.build } do
    property :name
    validates :name, presence: true
  end
end

params = {album:{
  title: 'Arrival',
  songs: [
    {id: 100, title: 'Money, Money, Money'},
    {id: 101, title: 'Dancing Queen'}
  ]
}}

new_album = Album.new
new_album_form = AlbumForm.new(@new_album)
new_album_form.validate(params)
saved_album = new_album_form.save { ... }

expect(saved_album.id).not_to be_nil
expect(saved_album.title).to eq('Arrival')
expect(saved_album.songs.size).to eq(2)

@apotonick do you think this is possible (in a clean way)?

[apotonick]

Yepp, I am working on that in representable.

[elvanja]

👍

[tomash]

@elvanja thanks, most helpful.

[TylerRick]

@elvanja, I like your idea!

collection :songs, new_instance: lambda { model.songs.build } do
  …

I hate that we have to manually do stuff (ugly hacks) like this in the controller:

    params[:album][:songs_attributes].size.times do
      album.songs.build
    end

@apotonick, I'm looking forward to seeing your fix in representable.

[midas]

@apotonick

I am having trouble with getting this to work. I have read all of the issues that seem close to applicable and still cannot resolve my issue. Probably user error, but any guidance is appreciated.

class Subject < ActiveRecord::Base
  has_many :interests
end

class Interest < ActiveRecord::Base
  belongs_to :installation
  belongs_to :subject
end

class SubjectAndInterestForm < Reform::Form
  property :name_last
  collection :interests do
    property :installation_id
    validates :installation_id, presence: true

    property :subject_id
    validates :subject_id, presence: true
  end
end

When I execute:

subject = Subject.new
subject.interests << Interest.new
form = SubjectAndInterestForm.new( subject )

form.validate( "name_last" => 'Harrelson', 
               "interests_attributes[0]" => { 
                 "installation_id" => 1, 
                 "subject_id" => 1 
               } ) # => false
form.fields #=> #<Reform::Fields name_last="Harrelson", 
                         #       interests=
                         #         [#<#<Class:0x000001026289c0>:0x000001025f1448 
                         #            @model=#<Interest id: nil, subject_id: nil, installation_id: nil, vetting_cycle_in_days: nil, created_at: nil, updated_at: nil>, 
                         #            @fields=#<Reform::Fields 
                         #                       installation_id=nil, 
                         #                       subject_id=nil>,                        
                         #             @validation_context=nil                                          
                         #             @errors=#<Reform::Form::Errors:0x00000102599978 @base=#<#<Class:0x000001026289c0>:0x000001025f1448 ...>, @messages={:installation_id=>["can't be blank"], :subject_id=>["can't be blank"]}>>
                         #         ]>

This appears to be due to the nested form not getting the attributes. I apparently do not fully understand how to use the nested form.

Also, if I do not want to use the block form to save the nested form elements, what is the suggested technique for overriding the #save method in order to achieve the nested save(s)? Can I get access to the same nested attributes that the block form yields?

Thanks!

[apotonick]

That all works as expected! Since your Interest instance is emtpy (not populated with attributes), the nested form will be look "empty" as well.

[apotonick]

Goodness - re-reading helps. Your validate call is wrong.

It should be

validate(
  "name_last" => 'Harrelson', 
  "interests_attributes" => [ 
    "installation_id" => 1, 
    "subject_id" => 1
  ]
}

When using with form builder, the incoming hash looks a bit different but Reform can handle both formats.

[midas]

When I do this, as you suggested (which is how I thought it should work a la Rails form builders):

form.validate( "name_last" => 'Harrelson', 
               "interests_attributes" => [
                 { 
                   "installation_id" => 1, 
                   "subject_id" => 1 
                 }
                ] )

I get the exception:

NoMethodError: undefined method `values' for [{"installation_id"=>1, "subject_id"=>1}]:Array
from /Users/cjharrelson/.rvm/gems/ruby-2.1.0@ncite_vetting_server/gems/reform-0.2.6/lib/reform/form/active_model.rb:39:in `rename_nested_param_for!'

[apotonick]

Sorry, it's early. You wanna do

form.validate( "name_last" => 'Harrelson', 
               "interests" => [
                 { 
                   "installation_id" => 1, 
                   "subject_id" => 1 
                 }
                ] )

The weird interests_attributes hash was introduced from Rails' nested_attributes. Reform exposes a way more straight-forward input hash format but, as I said, it works with both formats to allows compatiblity with Rails oddness. The implementation is here.

[midas]

OK. Works fine now. Thanks!

Now for the 2nd question from my original post that was obviously hidden within all of the details for the first question (sorry):

If I do not want to use the block form to save the nested form elements, what is the suggested technique for overriding the #save method in order to achieve the nested save(s)? Can I get access to the same nested attributes that the block form yields?

[apotonick]

You wanna call save on all nested models yourself but still want Reform to set all the properties on nested models?

[midas]

I just want to be able to create the entire object graph of records in a single transaction. I want to be able to do it without using the block syntax in a controller/workflow/command/service/etc. Is there an accepted best practice for this with reform?

Thanks!

[apotonick]

You don't need to put that block in a service. The block form of #save is the only official way to read the nested hash. Give me a piece of pseudo-code what you would do if you already had the nested hash.

[midas]

I know you do not have to use a workflow/service/etc to save a record. I am asserting that if the block syntax is the only way to save nested records, then I am forced reimplement it in every controller/workflow/service/etc that I use it in, thus not being DRY.

Here is the pseudo code for what I am thinking I may be able to do. However, I cannot figure out how to do it. Essentially I am trying to make a form that knows how to save a specific object graph of attributes.

class ProjectWithTasksForm < Reform::Form
  property :name

  collection :tasks do
    property :description
    property :due_on
  end

  def save
    Project.transaction do
      project = Project.create!( attributes.slice( :name ))
      attributes[:tasks].each do |task_attributes|
        project.tasks.create!( task_attributes )
      end 
    end
  end
end

# set up form ...
form.save if form.validate( params[:project] )

[apotonick]

Consider overriding Form#save and use that: https://github.com/apotonick/reform/blob/7588a3ce7d160a3b126cf3e2da52ea13df4024da/lib/reform/form.rb#L111

BTW, a general note, I'm on codementor now, a great way for paid first-class support and thus supporting open-source authors.

[apotonick]

Implemented :populate_if_empty in 1.0: https://github.com/apotonick/reform#populating-forms-for-validation

[arnvald]

Thank you @apotonick!

[elvanja]

Hey @apotonick, I just tried this new feature using latest Reform master.
Latest code is available here: https://github.com/elvanja/reform_example/tree/feature/using_populate_if_emtpy_to_dynamically_add_songs

I've removed the previously mentioned fix, and used the new feature like this:

class AlbumForm < Reform::Form # FIXME: sub forms don't inherit FBM.
  model :album
  collection :songs, populate_if_emtpy: lambda { |fragment, args| model.songs.build } do
    property :name
    validates :name, presence: true
  end
end

When I submit with these parameters:

{
  "album"=>{
    "title"=>"Test",
    "songs_attributes"=>{
      "0"=>{"name"=>"Existing Song", "id"=>"34"},
      "1"=>{"name"=>"Newly Added Song !!!", "id"=>""}
    },
}

I get this: undefined method validate!' for {"name"=>"Newly Added Song !!!", "id"=>""}:ActionController::Parameters`

[apotonick]

Thanks for testing!

Look, I do it here as well: https://github.com/apotonick/reform_example/blob/master/app/controllers/albums_controller.rb#L29

[apotonick]

@elvanja Did you run it with the latest master? I fixed a bug with FormBuilder yesterday.

[apotonick]

@elvanja All good now.

First, you had a typo, it's :populate_if_empty. Check the dictionary for "empty" (yes, you won't find "emtpy" in there). 😁

Second, that's my bad, the block wasn't executed in form context (you didn't even get that far haha). All fixed now.

Thanks again for your responsive testing!!!! c4f3922

Should we merge our reform_examples? Check out my master branch, I added a user form into the song form to add a composer to songs.

[elvanja]

@apotonick oh man, so much about Typo Driven Development :-) Anyway, I tried with your latest fix and now it works as expected, thank your very very much!

I don't have the time to merge reform examples today, but I'll be sure to do that during the week. Have to see what you did first.

And, I also propose a new discipline: Clipboard Driven Development! Just see https://github.com/apotonick/reform#populating-forms-for-validation and find the typo that got into my code :-)

[apotonick]

Hahaha, great! Hey man, YOU 're the one living on the edge, this is all pre-1.0 master branch code, all your fault if you use non-stable code.

[elvanja]

Hey @apotonick, compared your branch with mine and added support for composed song user.
Adding new users works fine, but I can't get it to work when I want to use an existing user. See https://github.com/elvanja/reform_example/blob/feature/using_populate_if_emtpy_to_dynamically_add_songs/app/forms/forms/album_form.rb. Any ideas?

[apotonick]

I tested and it works fine, I can edit an existing user. What are you after, do you wanna add an existing user? Can you make a form for that somehow? As soon as you send the id to your form, it should grab that user?!?!

[apotonick]

BTW Thanks for your work on the example project - makes it so much easier for me to test etc. And it looks good ❤️

Maybe I should link to your repository in the README as @gogogarrett doesn't do much but drinking beers and being a husband... ;)

[elvanja]

Ha ha :-) Do not underestimate the influence of Wife!

What I mean was that when a new song is created, when I enter the user's first and last name, I tried to detect in populate_if_empty if I already have a user with the given ID or names and use it instead of creating a new user each time a new song is added.

Example:

• let's say you already have a user named Roger Rabbit
• enter the new album form
• add 2 songs, both with the same Roger Rabbit user
• upon saving the album, there will be 2 new users with same name, instead of just the original Roger Rabbit

I know this mainly falls into logic after form processing. I mean, I can add typeahead or something similar to detect the existing user and grab it's ID so it can be added to the form submit parameters. I was just wondering if there was a way to use reform to do the job or prepare the way.

Currently I just use form for validating, and then still pass raw parameters to the rest of the system. For this user per song feature, I'd need to do processing again, parsing everything from params or tweaking them to be what I need. That sounds a bit silly. Is there a way to get object tree from the form itself? E.g. the album with all the songs and users, but with filled data from the UI form? (currently I get only the empty structure).

[apotonick]

It's not even his Wife, only Girlfriend!!!!

Is there a way to get object tree from the form itself? E.g. the album with all the songs and users, but with filled data from the UI form? (currently I get only the empty structure).

Sure!!! Either call

form.sync

which will populate all models with the form data. No saving happens!

Or, I'm not sure what you exactly want, use #save with a block.

form.save do |f, nested|
  # nested is nested object DATA graph, just a hash

Does that help? Should we discuss that on a separate thread?

In regards to your first "issue" - how do you tell the form (the web form) that you wanna "reuse" Roger Rabbit? If you make that work in the UI I'll make the code work 😁

[elvanja]

Great, the form.sync actually helped. That way I get the entire tree in form.model and then I can decide on how to save / update the tree.

I've updated the example code, so now it:

• updates first and last user name on album update
• finds a matching user by first and last name, so no user duplication occurs
• adds new users if not updating and no match found

Now there is no need for the form.save with a block, so album workflow is much simpler now.

The populate_if_empty solution with lambda that detected existing users was working OK for matching existing users, but it had a bug described in previous comment (2 new songs, both with the same new user, actually created 2 new users instead of just one). So, I moved the user detection code to album manager. The lambda solution is not used any more for that reason.

Anyway, there are probably some more bugs, but this is pretty much it I think. @apotonick what say you?