Bugfix #827 Fix nested forms f.fields

[AquisTech]

Added fields method.
This I am using in my project and is running well.
But please review and suggest if you foresee any issues or any other changes are expected.

[tagliala]

Hi, thanks for the report and for the PR

Could you please

1. Fix the RuboCop offence reported by Travis CI
2. Add a failing spec

[AquisTech]

Hi, thanks for the report and for the PR

Could you please

1. Fix the RuboCop offence reported by Travis CI
2. Add a failing spec

Sure @tagliala

[coveralls]

Coverage remained the same at 100.0% when pulling 46bb703 on AquisTech:bugfix-827-nested-form-breaks-for-fields into 34959f1 on DavyJonesLocker:main.

[coveralls]

Coverage decreased (-8.5%) to 91.522% when pulling a0073c9 on AquisTech:bugfix-827-nested-form-breaks-for-fields into 2b8021e on DavyJonesLocker:main.

[tagliala]

Amazing, thanks

Specs are still failing on CI for different reasons

Please take a look at the Travis Build

If this method is only used in Rails 6.1, all the previous versions of rails should not test it. You may want to provide conditional testing like it has been done with form_with

https://github.com/DavyJonesLocker/client_side_validations/blob/main/test/action_view/cases/test_form_with_helpers.rb#L5

[tagliala]

Please rebase on main, I've already cherry-picked 1159720, thanks

[AquisTech]

@tagliala I am trying to fix specs.
Also I will add it conditionally.
One thing I observed. Generation of ids for form_with and fields depends on Rails.application.config.action_view.form_with_generates_ids for a few versions of Rails id did not get generated by default. But for recent versions it gets generated by default.
The code is working for me as I am latest version.
For CSV we force to have ids as our JS depends on them. So any suggestions how should we force that behaviour?
In my specs, I am able to get id for fields but its not prepending parent name in it.

Failure:
ClientSideValidations::FormWithActionViewHelpersTest#test_form_with_nested_fields_inherit_validation_settings [/home/anand/projects/client_side_validations/test/action_view/cases/test_form_with_helpers.rb:285]:
Expected: <form action="/posts" accept-charset="UTF-8" method="post" data-client-side-validations="{&quot;html_settings&quot;:{&quot;type&quot;:&quot;ActionView::Helpers::FormBuilder&quot;,&quot;input_tag&quot;:&quot;\u003cspan id=\&quot;input_tag\&quot;\u003e\u003c/span\u003e&quot;,&quot;label_tag&quot;:&quot;\u003clabel id=\&quot;label_tag\&quot;\u003e\u003c/label\u003e&quot;},&quot;number_format&quot;:{&quot;separator&quot;:&quot;.&quot;,&quot;delimiter&quot;:&quot;,&quot;},&quot;validators&quot;:{&quot;post[comment][title]&quot;:{&quot;presence&quot;:[{&quot;message&quot;:&quot;can&#39;t be blank&quot;}]}}}" novalidate="novalidate" data-remote="true"><input name="utf8" type="hidden" value="&#x2713;" /><input type="text" name="post[comment][title]" id="post_comment_title" /></form>
Actual: <form action="/posts" accept-charset="UTF-8" data-remote="true" novalidate="novalidate" data-client-side-validations="{&quot;html_settings&quot;:{&quot;type&quot;:&quot;ActionView::Helpers::FormBuilder&quot;,&quot;input_tag&quot;:&quot;\u003cspan id=\&quot;input_tag\&quot;\u003e\u003c/span\u003e&quot;,&quot;label_tag&quot;:&quot;\u003clabel id=\&quot;label_tag\&quot;\u003e\u003c/label\u003e&quot;},&quot;number_format&quot;:{&quot;separator&quot;:&quot;.&quot;,&quot;delimiter&quot;:&quot;,&quot;},&quot;validators&quot;:{}}" method="post"><input name="utf8" type="hidden" value="&#x2713;" /><input type="text" name="comment[title]" id="comment_title" /></form>

Difference is
<input type="text" name="post[comment][title]" id="post_comment_title" />
and
<input type="text" name="comment[title]" id="comment_title" />

[tagliala]

For CSV we force to have ids as our JS depends on them. So any suggestions how should we force that behaviour?

Is this question related to this PR?

[tagliala]

I can see that there are some errors like

ClientSideValidations::FormWithActionViewHelpersTest#test_form_with_nested_fields_with_nested_attributes:
ArgumentError: wrong number of arguments (given 2, expected 0..1)

This happens because the specs are calling f.fields(:comments, [@comment]), which is incorrect (in other words: fields tests can't be a copy & paste of fields_for)

Here it is the signature of fields: https://apidock.com/rails/v5.1.7/ActionView/Helpers/FormHelper/fields

Since I do not use fields, please keep working on specs and then fix them

[AquisTech]

@tagliala Yes you are right about spec errors. I was fixing failures on local. So that signature is corrected but now there are failures due to mismatch between expected and actual behaviour in terms of ID being created. I will keep working to fix specs over weekend.
Mainly I dont want to override whole method just to get the ID right on some Rails version.
Can we add some note in README saying that "Rails.application.config.action_view.form_with_generates_ids this MUST be set to true in your rails app for CSV to work."
First I will work on this weekend I will try to fix specs and then we can decide about the note in Readme

[tagliala]

expected and actual behaviour in terms of ID being created.

There is a helper called automatic_id that checks the version of rails and decides whether to add the id or not.

client_side_validations/test/action_view/cases/test_legacy_form_with_helpers.rb

Lines 137 to 139 in 2b8021e

	def automatic_id(id)
	id if Rails.version >= '5.2'
	end

But I think that if the ID is not provided, then there will be issues with the front-end validations (CSV uses ids to identify fields)

Can we add some note in README saying that "Rails.application.config.action_view.form_with_generates_ids this MUST be set to true in your rails app for CSV to work."

I guess this is an issue only for 5.0 and 5.1, and we should add to the readme this note, correct?

https://guides.rubyonrails.org/configuring.html

[tagliala]

@AquisTech I'm working on this and I've got the issue, thanks

[tagliala]

@AquisTech this one was important to set that configuration variable according to Rails version. But during the tests, it still doesn't generate the id, so I'm keep investigating.

config.try is used because this method didn't exist in Rails 5.0