Columns in AbstractFieldArray subclasses do not get their ID set correctly

[Berdir]

Preconditions (*)

1. Magento 2.3.1

Steps to reproduce (*)

Create a subclass of AbstractFieldArray and include at least one custom renderer to display a select element with options. Optionally a regular column with a standard textfield. Store some values.

Expected result (*)

Both the textfield and the select should have their previously stored default value set.

Actual result (*)

Only the textfield has a default value. People are doing strange workarounds through _prepareArrayRow, like in https://magento.stackexchange.com/questions/202208/how-to-properly-load-config-table-data-into-admin-panel with extra option attributes, but the source of the problem seems to be that the select has an empty ID attribute, bucause no 'id' is passed to the renderer. Instead a 'input_id' data key is passed in, but not used:

<select name="groups[yellowcube][fields][allowed_methods][value][_1553583037209_209][allowed_methods]" id=""

I was able to fix this by passing the id through $attributes, and then it just worked:

            $this->_methodRenderer = $this->getLayout()->createBlock(
                Codes::class,
                '',
                [
                    'is_render_to_js_template' => true,
                    'data' => [
                        'id' =>  $this->_getCellInputElementId('<%- _id %>', 'allowed_methods'),
                    ],
                ]
            );

Results in:

<select name="groups[yellowcube][fields][allowed_methods][value][_1553583037209_209][allowed_methods]" id="_1553583037209_209_allowed_methods"

I think this should be done by default, similar to how the input name is set.

This happens in \Magento\Config\Block\System\Config\Form\Field\FieldArray\AbstractFieldArray::renderCellTemplate(), where it sets the name and input id, but the InputId is a non-unused attribute, that should be Id instead.

[magento-engcom-team]

Hi @Berdir. Thank you for your report. To help us process this issue please make sure that you provided the following information:

• [ ] Summary of the issue
• [ ] Information on your environment
• [ ] Steps to reproduce
• [ ] Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@Berdir do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

• [ ] yes
• [ ] no

[Berdir]

Actually looks like setInputName() is equally wrong there, and the link above as well as my code have existing workarounds for that that re-map setInputName() or map getName() to getinputName().

[Berdir]

Created a merge request that shows how the proposed change allows to simplify implementations like \Magento\CatalogInventory\Block\Adminhtml\Form\Field\Minsaleqty while it should still work the same way. One case is a bit interesting though, \Magento\Braintree\Block\Adminhtml\Form\Field\CcTypes is a multiple-select and changes the name to append [].

[github-jira-sync-bot]

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-10931 is successfully created for this GitHub issue.

[m2-assistant[bot]]

:white_check_mark: Confirmed by @engcom-Delta. Thank you for verifying the issue.
Issue Available: @engcom-Delta, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.