Fix :inverse_of documentation #31446
Conversation
|
(@rails-bot has picked a reviewer for you, use r? to override) |
guides/source/association_basics.md
Outdated
| @@ -1746,7 +1739,7 @@ There may be times when you wish to customize the query used by `has_many`. Such | |||
|
|
|||
| ```ruby | |||
| class Author < ApplicationRecord | |||
| has_many :books, -> { where processed: true } | |||
| has_many :books, -> { where processed: true }, inverse_of: :author | |||
There was a problem hiding this comment.
This and below are not for :inverse_of examples.
Do we need these changes?
There was a problem hiding this comment.
I think updating all examples is the way to since scopes are only mentioned once in the text, that way when people copy-paste an example they'll get the right thing. We could address this in another way too.
There was a problem hiding this comment.
has_many :books, ..., inverse_of: :author is not always the right association name. At least we have another example has_many :books, inverse_of: 'writer' in the guide.
class Author < ApplicationRecord
has_many :books, inverse_of: 'writer'
end
class Book < ApplicationRecord
belongs_to :writer, class_name: 'Author', foreign_key: 'author_id'
end
There was a problem hiding this comment.
I agree with @kamipo that those examples don't need to change. They also are giving the impression that if you define an association you already need to define the inverse_of:
guides/source/association_basics.md
Outdated
| @@ -2264,7 +2259,7 @@ The `group` method supplies an attribute name to group the result set by, using | |||
|
|
|||
| ```ruby | |||
| class Parts < ApplicationRecord | |||
| has_and_belongs_to_many :assemblies, -> { group "factory" } | |||
| has_and_belongs_to_many :assemblies, -> { group "factory" }, inverse_of :parts | |||
There was a problem hiding this comment.
Is this (group with inverse_of) right example?
guides/source/association_basics.md
Outdated
| @@ -735,12 +735,10 @@ a.first_name = 'David' | |||
| a.first_name == b.author.first_name # => true | |||
| ``` | |||
|
|
|||
| Active Record supports automatic identification for most associations with standard names. However, Active Record will not automatically identify bi-directional associations that contain any of the following options: | |||
| Active Record supports automatic identification for most associations with standard names. However, Active Record will not automatically identify bi-directional associations that contain a scope or any of the following options: | |||
|
|
|||
| * `:conditions` | |||
There was a problem hiding this comment.
Just spotted in passing, but.. didn't we stop supporting :conditions options on associations in 4.x? 🙊
…ic options, and is needed for bi-directionality with a scope [ci skip] Remove :conditions opion from association basics guide This got replaced by scopes.
bdewater
force-pushed
the
inverse-of-options-docs
branch
from
992af0f
to
4ab4364
Compare
|
Addressed all feedback and squashed my commits. |
|
Reverted the remaining examples changes in e81f207. |
Before this commit the `inverse_of` note in the `:foreign_key` option docs was a bit misleading. The text was originally added in [91fd651][] to describe adding `inverse_of` to any `belongs_to` associations on a join model: > If you are going to modify the association (rather than just read from it), then it is > a good idea to set the <tt>:inverse_of</tt> option. That text makes sense for the `:through` option, but it got copied over to all the `:foreign_key` option docs as well in rails#31446. Adding `:inverse_of` to an association with a custom `:foreign_key` is often desirable even if you are only reading the associations, since it can prevent N+1 queries (e.g. `post.comments.map(&:post)`). This commit changes the text to suggest adding `:inverse_of` all the time when `:foreign_key` is set, rather than only when the association gets modified. This commit also points to the `Setting Inverses` section from the `:through` option docs. `Setting Inverses` directly follows `Association Join Models` and is perhaps better named `Setting Inverses on Association Join Models`. It's probably the more relevant part for the `:through` option docs, but it's easy to miss because of the section break. This seemed like the easiest fix, but I could also rename the section or remove the section break entirely. [91fd651]: rails@91fd651
Came across a few issues when working on rubocop/rubocop#5045 - the docs contradict the code in
reflection.rba few ways::inverse_ofworks for:as,:throughand:polymorphicoptionsINVALID_AUTOMATIC_INVERSE_OPTIONSconstant in the codeThis can be backported to older maintained versions with the caveat that automatic inverse for polymorphic relations is 5.2 only (#28808)