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

[Fix #51720] Infer association klass as top level if model has same demodularized name #51721

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
* Fix inference of association model on nested models with the same demodularized name.

E.g. with the following setup:

```ruby
class Nested::Post < ApplicationRecord
has_one :post, through: :other
end
```

Before, `#post` would infer the model as `Nested::Post`, but now it correctly infers `Post`.

*Joshua Young*

* Add public method for checking if a table is ignored by the schema cache.

Previously, an application would need to reimplement `ignored_table?` from the schema cache class to check if a table was set to be ignored. This adds a public method to support this and updates the schema cache to use that directly.
Expand Down
8 changes: 6 additions & 2 deletions activerecord/lib/active_record/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,17 @@ def autosave=(autosave)
# a new association object. Use +build_association+ or +create_association+
# instead. This allows plugins to hook into association object creation.
def klass
@klass ||= compute_class(class_name)
@klass ||= compute_class(compute_name(class_name))
end

def compute_class(name)
name.constantize
end

def compute_name(name) # :nodoc:
active_record.name.demodulize == name ? "::#{name}" : name
end

# Returns +true+ if +self+ and +other_aggregation+ have the same +name+ attribute, +active_record+ attribute,
# and +other_aggregation+ has an options hash assigned to it.
def ==(other_aggregation)
Expand Down Expand Up @@ -994,7 +998,7 @@ def through_reflection?
end

def klass
@klass ||= delegate_reflection.compute_class(class_name)
@klass ||= delegate_reflection.compute_class(compute_name(class_name))
end

# Returns the source of the through reflection. It checks both a singularized
Expand Down
27 changes: 27 additions & 0 deletions activerecord/test/cases/reflection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
require "models/user_with_invalid_relation"
require "models/hardback"
require "models/sharded/comment"
require "models/admin"
require "models/admin/user"
require "models/user"

class ReflectionTest < ActiveRecord::TestCase
include ActiveRecord::Reflection
Expand Down Expand Up @@ -181,6 +184,30 @@ def test_reflection_klass_requires_ar_subclass
end
end

def test_reflection_klass_with_same_demodularized_name
reflection = ActiveRecord::Reflection.create(
:has_one,
:user,
nil,
{},
Admin::User
)

assert_equal User, reflection.klass
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before patch:

Screenshot 2024-05-03 at 1 10 59 PM

end

def test_reflection_klass_with_same_demodularized_different_modularized_name
reflection = ActiveRecord::Reflection.create(
:has_one,
:user,
nil,
{ class_name: "Nested::User" },
Admin::User
)

assert_equal Nested::User, reflection.klass
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to make sure we don't regress on this case.

end

def test_aggregation_reflection
reflection_for_address = AggregateReflection.new(
:address, nil, { mapping: [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,9 @@ class User < ActiveRecord::Base
class UserWithNotification < User
after_create -> { Notification.create! message: "A new user has been created." }
end

module Nested
class User < ActiveRecord::Base
self.table_name = "users"
end
end