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

Add cop Rails/AcceptsNestedAttributesForUpdateOnly #1035

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

olivier-thatch
Copy link

@olivier-thatch olivier-thatch commented Jun 23, 2023

Add cop Rails/AcceptsNestedAttributesForUpdateOnly.

This cop looks for accepts_nested_attributes_for attributes writers that don't specify an :update_only option.

The default value for :update_only is false, meaning that a new nested model will be created unless the ID of the existing model is explicitly provided in the attributes. This is not always the desired behavior, so I think a cop that forces an explicit decision regarding which value :update_only should take would be a valuable addition.

Implementation-wise, this cop is based on the existing Rails/HasOneHasManyDependent cop that does something very similar (enforce an explicit :dependent value for has_one and has_many associations).


Before submitting the PR make sure the following are checked:

@olivier-thatch olivier-thatch force-pushed the accepts-nested-attributes-for-update-only branch from c26a1a8 to bd69597 Compare June 23, 2023 00:00
@olivier-thatch olivier-thatch changed the title Add cop Rails/AcceptsNestedAttributesForUpdateOnly Add cop Rails/AcceptsNestedAttributesForUpdateOnly Aug 17, 2023
@pirj
Copy link
Member

pirj commented Oct 12, 2023

This is so since Rails 3.0. Wondering why among ~200 usages of a-n-a, only two projects used this option? 🤔
Don't we usually pass the id along?

By the way, did you check if GitLab already has a cop for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants