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.

Sign up for GitHub

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

Jump to bottom

[Fix #49055] Raise an ArgumentError when #accepts_nested_attributes_for is redeclared for an association #49056

Merged
merged 1 commit into from Sep 6, 2023
Merged

[Fix #49055] Raise an ArgumentError when #accepts_nested_attributes_for is redeclared for an association #49056

merged 1 commit into from Sep 6, 2023

Conversation

joshuay03
Copy link
Contributor

@joshuay03 joshuay03 commented Aug 28, 2023 •
edited

Motivation / Background

This Pull Request has been created to fix #49055.

Detail

This Pull Request changes ActiveRecord::NestedAttributes#accepts_nested_attributes_for to raise an ArgumentError when it has already been declared for an association in the same class. There is a use case for overriding this in subclasses, that functionality is unchanged.

Additional information

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.

@joshuay03 joshuay03 changed the title [Fix #49055] Raise an ArgumentError when #accepts_nested_attributes_for is redeclared for an association [Fix #49055] Raise an ArgumentError when #accepts_nested_attributes_for is redeclared for an association Aug 28, 2023
@joshuay03 joshuay03 force-pushed the raise-on-duplicate-accepts-nested-attributes-for branch 4 times, most recently from 64e03d9 to a42c466 Compare August 28, 2023 06:44
@joshuay03
Copy link
Contributor Author

joshuay03 commented Aug 28, 2023 •
edited

Looking into the failing tests, all of them seem to be from activerecord/test/cases/nested_attributes_test.rb due to redeclaring accepts_nested_attributes_for . I'll fix these up soon.

I'll also be careful not to change the expectation here when doing so.

@joshuay03 joshuay03 mentioned this pull request Aug 28, 2023
Closed
@joshuay03 joshuay03 force-pushed the raise-on-duplicate-accepts-nested-attributes-for branch 2 times, most recently from 1c24635 to 0d1c3d2 Compare August 28, 2023 07:45
@ghiculescu ghiculescu added the ready PRs ready to merge label Aug 28, 2023
@joshuay03 joshuay03 force-pushed the raise-on-duplicate-accepts-nested-attributes-for branch from 0d1c3d2 to 4cdb1c9 Compare August 29, 2023 21:39
@joshuay03 joshuay03 force-pushed the raise-on-duplicate-accepts-nested-attributes-for branch from 4cdb1c9 to 56f5311 Compare August 30, 2023 13:05
@joshuay03
Copy link
Contributor Author

Cheers for the review @seanpdoyle I've addressed your comments.

@joshuay03 @rafaelfranca
[Fix rails#49055] Raise an ArgumentError when `#accepts_nested_attr…
0a54155 
…ibutes_for` is redeclared for an association
@rafaelfranca rafaelfranca force-pushed the raise-on-duplicate-accepts-nested-attributes-for branch from 56f5311 to 0a54155 Compare September 6, 2023 15:28
@rafaelfranca rafaelfranca merged commit b6166e8 into rails:main Sep 6, 2023
4 checks passed
rafaelfranca added a commit that referenced this pull request Oct 1, 2023
@rafaelfranca
Revert "Merge pull request #49056 from joshuay03/raise-on-duplicate-a…
0ba14eb 
…ccepts-nested-attributes-for"

This reverts commit b6166e8, reversing
changes made to 5415d3a.
rafaelfranca added a commit that referenced this pull request Oct 1, 2023
@rafaelfranca
Revert "Merge pull request #49056 from joshuay03/raise-on-duplicate-a…
f9133f7 
…ccepts-nested-attributes-for"

This reverts commit b6166e8, reversing
changes made to 5415d3a.

Reason: The reverted behavior broke the case where the
`#accepts_nested_attributes_for` was defined in a concern and
where overridden in the class that included the concern.
@joshuay03 joshuay03 mentioned this pull request Jan 12, 2024
Open
4 tasks
@joshuay03 joshuay03 deleted the raise-on-duplicate-accepts-nested-attributes-for branch January 12, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafaelfranca rafaelfranca rafaelfranca left review comments

@ghiculescu ghiculescu ghiculescu left review comments

@eileencodes eileencodes eileencodes left review comments

@chaadow chaadow chaadow left review comments

@seanpdoyle seanpdoyle seanpdoyle left review comments

Assignees
No one assigned
Labels
activerecord ready PRs ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

#accepts_nested_attributes_for does not raise an error when redeclaring for an association
6 participants
@joshuay03 @rafaelfranca @ghiculescu @eileencodes @chaadow @seanpdoyle