comfy-bootstrap-form icon indicating copy to clipboard operation
comfy-bootstrap-form copied to clipboard

Models without `error` hash generate `undefined method '[]' for nil:NilClass`

Open tjdavey opened this issue 4 years ago • 2 comments

Valid form elements generate undefined method '[]' for nil:NilClass when the form's model does not include an errors hash.

There are some scenarios when rails forms models are not set from an ActiveRecord model. In these scenarios, unless they emulate the errors hash produced by ActiveRecord, the rendering of the form element will result in the aforementioned error.

Example:

<%= bootstrap_form_with(model: my_model) do |f| %>
  <%= f.text_field :top_level_field %>
  <%= f.fields_for :nested_hash, OpenStruct.new(my_model.nested_hash) do |ff| %>
    <%= ff.text_field :nested_hash_field %>
  <% end %>
  <%= f.primary "Save" %>
<% end %>

In this scenario, with vanilla rails form elements, OpenStruct provides all the necessary methods for the nested_hash_field property to be rendered correctly. Using comfy-bootstrap-form this will generate an error unless the nested_hash includes an errors property.

Suggested Fix

The draw_errors method in form_builder.rb already includes a fast-return path if the model is nil. The criteria should this should be expanded to also return if the errors hash is not present.

tjdavey avatar May 28 '20 11:05 tjdavey

Was thinking about this a bit. What you're describing is a bit odd.

If you're not actually sending ActiveRecord object you should use form_with scope: :not_a_model and then just declare values manually. If you have something complicated going on you want to use a form object:

class FormObject
   include ActiveModel::Model
end

And then your form renders as if it's dealing with normal ActiveRecord object.

Now, if you want to render form and pass an object that that only holds accessors and nothing else you'd need to sense if object responds to .errors and it happens to be an array. Your PR assumes that .errors exists. That's always true on OpenStruct objects, but not true for any other random object.

GBH avatar Jun 08 '20 14:06 GBH

Yeah, this is a bit confusing because I had a specific use-case in mind when I ran into this issue and perhaps didn't describe it adequately. The purpose of the changes here were to attempt to replicate the experience you receive from the vanilla form and fields_for elements and I was less concerned about the Rails semantics of the objects.

The particular scenario I was looking to resolve revolves around nested hashes in ActiveRecord models (eg. JSONB fields in Postgres models) whereby setting these properties on a form can be done by creating an object with the appropriate accessors and passing that into a form or fields_for object, commonly done by converting the hash to an OpenStruct. This works fine in vanilla forms, but obviously the lack of the errors accessor is an issue when using the comfy bootstrap forms

Based on what I saw in the Vanilla forms source the only hard requirement for the object passed into the form is that it has the setter/getter methods.

If we harden up the error method check to fix the case where error isn't defined would that be an appropriate solution in your mind?

tjdavey avatar Jun 08 '20 23:06 tjdavey