trailblazer / reform-rails

Automatically load and include all common Rails form features.
MIT License
99 stars 49 forks source link

undefined method `persisted?' (and `to_key`) for #<Hash> when using Composition and `form_for` #63

Open TylerRick opened 6 years ago

TylerRick commented 6 years ago

Without Composition, persisted? works fine, but with Composition it gives this error apparently because it delegates to its model which is a Hash of multiple models.

Steps to reproduce

class MyForm < Reform::Form
  include Composition
  model :a

  property :prop_1, on: :a
  property :prop_2, on: :b
end

form = MyForm.new(a: OpenStruct.new, b: OpenStruct.new)
form.persisted?  # undefined method `persisted?' for {:a=>#<OpenStruct>, :b=>#<OpenStruct>}:Hash

Using it in a form with form_for @form also generates the error, since form_for calls persisted?.

Workaround

An easy workaround is to simply add this to form class:

  def persisted?
    false
  end
TylerRick commented 6 years ago

Also ran into this error:

undefined method `to_key' for #<Hash:0x00560344c3a328>
actionview (4.2.10) lib/action_view/record_identifier.rb:80:in `record_key_for_dom_id'
actionview (4.2.10) lib/action_view/record_identifier.rb:62:in `dom_id'
actionview (4.2.10) lib/action_view/helpers/form_helper.rb:459:in `apply_form_for_options!'
actionview (4.2.10) lib/action_view/helpers/form_helper.rb:434:in `form_for'

Workaround was:

  def to_key
    nil
  end
TylerRick commented 6 years ago

I tried including model :a so it had a main model. I even tried upgrading to reform-rails 0.2.0.rc2 and reform 2.3.0.rc1 (was on latest release, 2.2.4, before).

When I tried stepping into model with a debugger, I noticed this:

[17, 26] in /home/tyler/.gem/ruby/2.4.1/gems/reform-2.3.0.rc1/lib/reform/form/composition.rb
   17:     # class CoverSongForm < Reform::Form
   18:     #   model :song, on: :cover_song
   19:     def model(main_model, options={})
   20:       super
   21: 
   22:       composition_model = options[:on] || main_model
   23: 
   24:       # FIXME: this should just delegate to :model as in FB, and the comp would take care of it internally.
=> 25:       [:persisted?, :to_key, :to_param].each do |method|
   26:         define_method method do
   27:           model[composition_model].send(method)
   28:         end
   29:       end

So it looks like it's supposed to delegate to the main model.

But I also found this occurrence of persisted? in the code:

reform-rails-0.2.0.rc2/lib/reform/form/active_model.rb:      delegates :model, *[:persisted?, :to_key, :to_param, :id] # Uber::Delegates

and I'm guessing that overrides the previous delegation and is what breaks things (for those of us using reform-rails).

How can we get it working how it's supposed to (delegate those methods to main model)?

TylerRick commented 6 years ago

Well, it looks like you can work around it by copying and pasting code!

  [:persisted?, :to_key, :to_param].each do |method|
    define_method method do
      model[:main_model].send(method)
    end
  end
fernandes commented 6 years ago

@TylerRick are you using reform-rails gem or just reform?

Usually reform-rails add the Form::ActiveModes that makes the form builder happy.

MyForm.ancestors includes `Reform::Form::ActiveModel?

look the source that's exactly what you are doing :wink:

fernandes commented 6 years ago

more info on the docs

TylerRick commented 6 years ago

Yes, I'm using reform-rails. Yes, it delegates those methods to model. The problem is that model is a hash of models and not a single model, because I'm using include Composition.

It needs to delegate the messages to model[:main_model] instead ... which it would do (see debugger output above) except that Reform::Form::ActiveModel overrides the correct "default" behavior and makes it delegate to plain model instead. So it seems reform-rails (Reform::Form::ActiveModel) is actually the source of the problem.

fernandes commented 6 years ago

ohhhh yeah @TylerRick , now I see what you are facing

How would you make with "plain" Rails?

Create a model the inherits Hash and include ActiveModel::Model on this class, is an option?