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

Nested attribute docs improvements #45829

Merged

Conversation

ghiculescu
Copy link
Member

2 small improvemennts to the docs for ActiveRecord::NestedAttributes.

  • Include a link to ActionView::Helpers::FormHelper::fields_for, since you would typically use these tools in tandem I think it makes sense to point you to it.
  • Show an example of how to write an integration test when using ActionView::Helpers::FormHelper::fields_for. You could argue that this example should also go in Action View, but I think it makes more sense here as the "parent" doc for the Nested Attributes feature.

@ghiculescu
Copy link
Member Author

The red build is unrelated (this is docs only).

@ghiculescu ghiculescu added the ready PRs ready to merge label Aug 15, 2022
Copy link
Member

@skipkayhil skipkayhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small formatting changes

activerecord/lib/active_record/nested_attributes.rb Outdated Show resolved Hide resolved
activerecord/lib/active_record/nested_attributes.rb Outdated Show resolved Hide resolved
activerecord/lib/active_record/nested_attributes.rb Outdated Show resolved Hide resolved
@ghiculescu ghiculescu force-pushed the nested-attribute-docs-improvements branch from 9fc279f to 9062aff Compare August 15, 2022 17:20
2 small improvemennts to the docs for `ActiveRecord::NestedAttributes`.

- Include a link to `ActionView::Helpers::FormHelper::fields_for`, since you would typically use these tools in tandem I think it makes sense to point you to it.
- Show an example of how to write an integration test when using `ActionView::Helpers::FormHelper::fields_for`. You could argue that this example should also go in Action View, but I think it makes more sense here as the "parent" doc for the Nested Attributes feature.
@ghiculescu ghiculescu force-pushed the nested-attribute-docs-improvements branch from 9062aff to 10a8320 Compare August 15, 2022 17:26
Comment on lines +291 to +292
# If you are using ActionView::Helpers::FormHelper#fields_for, your integration
# tests should replicate the HTML structure it provides. For example;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this always apply when using accepts_nested_attributes_for?
Even when you're not using fields_for?

Copy link
Member

@p8 p8 Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also wondering if describing the integration with ActionView and ActionController is more suited for the guides. Maybe we should instead link to https://guides.rubyonrails.org/form_helpers.html#nested-forms ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p8 i agree with these suggestions but I won’t get to them for a little while. If you like, I’d love for you to take over the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghiculescu Sure! 😄

Co-authored-by: Petrik de Heus <petrik@deheus.net>
@rafaelfranca rafaelfranca merged commit 606fdfb into rails:main Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants