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

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

Closed
shanecav84 opened this issue Jun 28, 2022 · 5 comments · Fixed by #47436

Comments

@shanecav84
Copy link

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

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "sqlite3"
  gem 'rails'
  gem 'sprockets-rails'
end

require 'rails'
require "active_record/railtie"
require "action_controller/railtie"
require "action_view/railtie"
require "sprockets/railtie"
require "minitest/autorun"

class TestApp < Rails::Application; end

Rails.application.routes.draw do
  resources :parents
end

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :parents, force: true do |t|
  end

  create_table :children, force: true do |t|
    t.integer :parent_id
  end

  create_table :grandchildren, force: true do |t|
    t.integer :child_id
  end
end

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
end

class Parent < ApplicationRecord
  has_many :children
  accepts_nested_attributes_for :children
end

class Child < ApplicationRecord
  belongs_to :parent
  has_many :grandchildren
  accepts_nested_attributes_for :grandchildren
end

class Grandchild < ApplicationRecord
  belongs_to :child
end


TEMPLATE = <<~ERB
<%= form_with model: parent do |f| %>
 <%= f.field_name :children_attributes, multiple: true %>
  <%= f.fields_for :children do |child_f| %>
     <%= child_f.field_name :grandchildren_attributes, multiple: true %>
  <% end %>
<% end %>
ERB

class ApplicationController < ActionController::Base
  include ActionDispatch::Routing::UrlFor
  include Rails.application.routes.url_helpers
end

class BugTest < Minitest::Test

  def setup
    @parent = Parent.new
    child = Child.new
    grandchild = Grandchild.new
    child.grandchildren << grandchild
    @parent.children << child
    @rendered = ApplicationController.renderer.render inline: TEMPLATE, locals: { parent: @parent }
  end

  def test_grandchildren_field_name
    assert_match('parent[children_attributes][0][grandchildren_attributes][]', @rendered)
  end
end

Expected behavior

Don't add an extra index parameter (parent[children_attributes][0][0][grandchildren_attributes][])

Actual behavior

An extra index parameter is added

System configuration

Rails version: 7

Ruby version: 3

@shanecav84
Copy link
Author

Tagging @seanpdoyle bc it looks like he introduced field_name.

@seanpdoyle
Copy link
Contributor

Thank you for opening this issue @shanecav84. I've reproduced it with a failing test on a branch on my fork: https://github.com/rails/rails/compare/seanpdoyle:action-view-nested-field-name-calls.

I'll investigate!

@sushantmittal
Copy link
Contributor

@seanpdoyle @shanecav84 : Fixed this issue here: #45523

@shanecav84
Copy link
Author

That PR fixes the issue I am having.

@l15n
Copy link

l15n commented Jun 20, 2023

I've also encountered this in the wild while attempting to upgrade an app to Rails 7.

i.e.

# In _form.html.haml
= item.products.each do |product|
  = f.fields_for :product, product do |pf|
    #product-description-text-area { data: { id: pf.field_id(:description), name: pf.field_name(:description) } }

# where the form object's model is like
class Item  < ApplicationRecord
  has_many :products
  accepts_nested_attributes_for :products
end

# Expected result in `data-id` attribute
# => item_products_attributes_0_description
# Actual
# => item_products_attributes_0_0_description

The use case here is to pass rails style form element attributes to the javascript layer to so that it can build a fancier form element.

For the moment, I have been able to workaround for this specific use case by explicitly setting the index to nil, i.e. pf.field_id(:description, index: nil)

seanpdoyle added a commit to seanpdoyle/rails that referenced this issue Jun 30, 2023
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.
amatsuda pushed a commit that referenced this issue Jul 2, 2023
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][]`

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.

Signed-off-by: Akira Matsuda <ronnie@dio.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants