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

Don't double-encode nested field_id and field_name index #47436

Merged
merged 2 commits into from Jul 2, 2023

Conversation

seanpdoyle
Copy link
Contributor

Motivation / Background

Closes #45483
Closes #45523

To quote #45483:

field_name is adding an extra index parameter, the extra [0] in
parent[children_attributes][0][0][grandchildren_attributes][]

Detail

To resolve that issue, this commit reads its default field_id and field_name method's index: option directly from the @options. Prior to this commit, that value was read from the @index instance variable.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@l15n
Copy link

l15n commented Jun 20, 2023

👋 is this PR missing anything to be merged? Would love to have this included in a future update so that I can get ride of a workaround. #45483 (comment)

@amatsuda
Copy link
Member

@seanpdoyle This looks good. Thank you for the fix! Could you update CHANGELOG?

Closes rails#45483
Closes rails#45523

To quote rails#45483:

> `field_name` is adding an extra index parameter, the extra `[0]` in
> `parent[children_attributes][0][0][grandchildren_attributes][]`

To resolve that issue, this commit reads its default `field_id` and
`field_name` method's `index:` option directly from the `@options`.
Prior to this commit, that value was read from the `@index` instance
variable.
@seanpdoyle seanpdoyle force-pushed the action-view-nested-field-name-calls branch from d24ac24 to 5cb8678 Compare June 30, 2023 13:54
@seanpdoyle
Copy link
Contributor Author

@amatsuda thank you for the review. I've rebased, added a CHANGELOG entry, and re-pushed.

@amatsuda amatsuda merged commit f8c06f9 into rails:main Jul 2, 2023
9 checks passed
@amatsuda
Copy link
Member

amatsuda commented Jul 2, 2023

@seanpdoyle Thank you!

@seanpdoyle seanpdoyle deleted the action-view-nested-field-name-calls branch July 3, 2023 17:52
@l15n
Copy link

l15n commented Jul 6, 2023

Thanks for following up!

@shanecav84
Copy link

@seanpdoyle Thanks!

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

Successfully merging this pull request may close these issues.

ActionView::FormBuilder#field_name returns incorrect field name for deeply-nested associations
4 participants