Simple_form: 3.5.1 breaks simple_fields_for for custom form objects

Created on 19 Feb 2018  路  12Comments  路  Source: heartcombo/simple_form

Environment

  • Ruby 2.5.0
  • Rails 5.1.4
  • Simple Form 3.5.1

Current/ expected behavior

Prior to 3.5.1, I was able to build my own form objects using http://dry-rb.org/gems/dry-types/ . It is looking something like

class CategoryForm < DryTypes::Model
  attribute :names, DryTypes::Strict::Array.member(DryTypes::Translation)
end

and

  = simple_form_for category_form do |f|
    = f.simple_fields_for :names do |name_fields|
      - name = name_fields.object
      = name_fields.input :value, label: "Name (#{name.language.name})"

After the update, name_fields.object returns nil and breaks my code. Does somebody have a hint which change could have caused the issue?

Most helpful comment

I guess it isn't a breaking change since it doesn't break any features described in the README. The fact it supported non-ActiveModel compliant objects was an unintentional feature and strictly speaking a bug since that meant it did not have parity with form_for, which is what the patch fixed.

It is just a bit unfortunate.

All 12 comments

Probably because object gets converted into to_model in 3.5.1 My SimpleDelegator decorators are broken too in 3.5.1 so freezing to 3.5.0

It seems we have a similar problem, if we have a wrapped model (presenter using SimpleDelegator) then object.form returns the model, not the wrapped model.

StudyAssignmentPresenter.new(StudyAssignment.new).class # => StudyAssignmentPresenter
StudyAssignmentPresenter.new(StudyAssignment.new).to_model.class # => StudyAssignment

You can hack around this as such:

class StudyAssignmentPresenter < SimpleDelegator
  def to_model; self; end
end

The PR introducing the call to to_model is here: https://github.com/plataformatec/simple_form/pull/1256

I can confirm that reverting the change from https://github.com/plataformatec/simple_form/pull/1256 fixes the problem for me!

@lacco does adding def to_model; self; end to CategoryForm work? You could also consider include ActiveModel::Conversion.

/cc @timurvafin

Actually I am already providing to_model to make use of Rails automatisms to guess URL, HTTP method, ... : def to_model; category; end. I am sure that def to_model; self; end should solve my problems with simple_form, but I would rewrite the other code.

You are right, now from version 3.5.1 we have to implement #to_model if we are using a PORO or Object that doesn't respond to it. BTW, it is the same behavior of form_for.

This information should be included in the releases notes, sorry about that 馃槬 . I will add some documentation about it.

Thank you folks.

@lacco just for curiosity. Does DryTypes::Model implements #to_model and other Active Model methods or you are expliciting it in the form?

@felipegimenesptec You are correct, my DryTypes::Model implementation is aware of ActiveModel features, the implementation looks roughly like this:

class DryTypes::Model < Dry::Struct
  include ActiveModel::Conversion
  include ActiveModel::Validations
  extend ActiveModel::Naming

  # This is actually overwritten to def to_model; category; end in my case
  def to_model
    self
  end

  def persisted?
    to_model == self ? false : to_model.persisted?
  end
end

I don't understand why this issue is closed, can you provide any context? Apps that break in a patch-level upgrade from 3.5.0 to 3.5.1 seems like it must be a bug? I would consider it a bug for patch-level release to ever include breaking changes...

I consider it a bug fix since Simple Form should work as a Rails Form like form_for do and it wasn't.

I guess it isn't a breaking change since it doesn't break any features described in the README. The fact it supported non-ActiveModel compliant objects was an unintentional feature and strictly speaking a bug since that meant it did not have parity with form_for, which is what the patch fixed.

It is just a bit unfortunate.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aalvrz picture aalvrz  路  4Comments

frankyston picture frankyston  路  4Comments

aximuseng picture aximuseng  路  4Comments

mszyndel picture mszyndel  路  5Comments

sarunw picture sarunw  路  4Comments