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

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

Open
tjdavey opened this issue May 28, 2020 · 2 comments · May be fixed by #25
Open

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

tjdavey opened this issue May 28, 2020 · 2 comments · May be fixed by #25

Comments

@tjdavey
Copy link

tjdavey commented May 28, 2020

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 tjdavey linked a pull request May 28, 2020 that will close this issue
@GBH
Copy link
Member

GBH commented Jun 8, 2020

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.

@tjdavey
Copy link
Author

tjdavey commented Jun 8, 2020

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants