Fix attributes passed to form has_many not being set on new record form items#8550
Merged
javierjulio merged 1 commit intoactiveadmin:masterfrom Nov 21, 2024
Fs00:fix-has-many-custom-attrs
Merged
Fix attributes passed to form has_many not being set on new record form items#8550javierjulio merged 1 commit intoactiveadmin:masterfrom Fs00:fix-has-many-custom-attrs
javierjulio merged 1 commit intoactiveadmin:masterfrom
Fs00:fix-has-many-custom-attrs
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8550 +/- ##
=======================================
Coverage 99.11% 99.11%
=======================================
Files 141 141
Lines 4076 4076
=======================================
Hits 4040 4040
Misses 36 36 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
javierjulio
changed the title
has_many not being set on new record form itemshas_many not being set on new record form items
Member
|
@Fs00 this looks great, thank you. Yes, you should open a backport PR on the 3-0-stable branch please. My focus is on v4. Usually another maintainer will pick up doing a v3 release. We have at least another change considered to do on v3 regarding Sass deprecations. Either way I will approve and merge that PR if you create it. In the meantime, I'll merge this. Thank you. |
javierjulio
changed the title
has_many not being set on new record form items
javierjulio
approved these changes
Nov 21, 2024
Contributor
Author
|
Thank you @javierjulio. I have opened #8551 to backport the change to AA 3. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a behavior inconsistency that occurs when passing custom attributes to the form builder
has_manymethod, like in the following example:which builds the following HTML (I kept only the relevant parts):
However, when clicking the "New Record" link to add a new item to the association, the
<fieldset>generated for the new item didn't have the user-provided attributes on it (it was just<fieldset class="inputs has_many_fields">...</fieldset>).I've discovered that the HTML used for new items is stored in the
data-htmlattribute of the link and generated by a different method that was ignoring all custom attributes except forclass.So I've changed the method to make sure that custom attributes are not ignored and added tests for the relevant scenarios.
We'd need this fix on a 3.x patch, should I open the backport PR once this is merged or will you take care of it?