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

doc(active_record): adding more examples about scope control #50342

Merged

Conversation

JohnAnon9771
Copy link
Contributor

@JohnAnon9771 JohnAnon9771 commented Dec 13, 2023

Motivation / Background

In the rails contributors discord, there was a discussion in the past about a Rails behavior that is somewhat unknown to Rails newbies (I am included in that list) where it showed that Rails can identify which relationship you define according to the scope of the model.

I'm opening this PR for opinions on how this can be documented, as it seems to be something very interesting about Rails and which is only noticed if you have the correct intuition that Rails can do this.

# Build a list of candidates to search for
candidates = []
name.scan(/::|$/) { candidates.unshift "#{$`}::#{type_name}" }
candidates << type_name

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.

@rails-bot rails-bot bot added the docs label Dec 13, 2023
guides/source/association_basics.md Outdated Show resolved Hide resolved
guides/source/association_basics.md Outdated Show resolved Hide resolved
guides/source/association_basics.md Outdated Show resolved Hide resolved
guides/source/association_basics.md Outdated Show resolved Hide resolved
@JohnAnon9771 JohnAnon9771 force-pushed the doc/improve-controlling-association-scope branch from 60269e1 to 0634bc7 Compare December 20, 2023 23:27
@JohnAnon9771
Copy link
Contributor Author

Thank you very much @p8 for the feedback, I have already made the suggested changes.

@@ -788,7 +788,30 @@ module MyApplication
end
```

This will work fine, because both the `Supplier` and the `Account` class are defined within the same scope. But the following will _not_ work, because `Supplier` and `Account` are defined in different scopes:
This will work fine, because both the `Supplier` and the `Account` class are defined within the same scope (`MyApplication::Business`). This organization allows structuring models into folders based on their scope:
Copy link
Member

@p8 p8 Dec 21, 2023

Choose a reason for hiding this comment

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

A small change to maybe make things a bit clearer what the advantage is:

Suggested change
This will work fine, because both the `Supplier` and the `Account` class are defined within the same scope (`MyApplication::Business`). This organization allows structuring models into folders based on their scope:
This will work fine, because both the `Supplier` and the `Account` class are defined within the same scope (`MyApplication::Business`). This organization allows structuring models into folders based on their scope, without having to explicitly add the scope to every association:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 😄

@JohnAnon9771 JohnAnon9771 force-pushed the doc/improve-controlling-association-scope branch from 0634bc7 to c430cc3 Compare December 22, 2023 04:00
@p8 p8 merged commit 082ffa9 into rails:main Jan 5, 2024
4 checks passed
@p8
Copy link
Member

p8 commented Jan 5, 2024

Thanks @JohnAnon9771 !

jonathanhefner added a commit that referenced this pull request Jan 7, 2024
jonathanhefner added a commit that referenced this pull request Jan 7, 2024
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

2 participants