Skip to content

Conversation

@moznion
Copy link
Contributor

@moznion moznion commented Aug 7, 2025

In v4.0.0, switching from extend ActiveModel::Naming to only extend ActiveModel::Translation (ref: #230) can cause "undefined method 'model_name'" errors when ActiveHash models include ActiveModel::Serialization, particularly with Rails 8.0+; actually I encountered this problem in our project.

While ActiveModel::Translation includes ActiveModel::Naming internally, the inclusion doesn't always properly expose the model_name class method when extended. This breaks compatibility for models that depend on model_name being available at the class level.

This commit explicitly extends both ActiveModel::Naming and ActiveModel::Translation to ensure model_name is always available while maintaining i18n support. This approach ensures compatibility across all Rails versions.

In v4.0.0, switching from `extend ActiveModel::Naming` to only `extend
ActiveModel::Translation` can cause "undefined method 'model_name'" errors
when ActiveHash models include ActiveModel::Serialization, particularly
with Rails 8.0+.

While ActiveModel::Translation includes ActiveModel::Naming internally,
the inclusion doesn't always properly expose the model_name class method
when extended. This breaks compatibility for models that depend on
model_name being available at the class level.

This commit explicitly extends both ActiveModel::Naming and
ActiveModel::Translation to ensure model_name is always available while
maintaining i18n support. This approach ensures compatibility across all
Rails versions.

Signed-off-by: moznion <moznion@mail.moznion.net>
@kbrock
Copy link
Collaborator

kbrock commented Aug 7, 2025

@flavorjones We're now including Translation and now Naming. Does this feel like we're including too much?

@flavorjones
Copy link
Collaborator

@kbrock I just took a moment to go back and read the prior pull requests, and it seems like these changes are being driven by actual user needs, and it's not adding complexity to the gem. My first reaction, without much thought, is that it's probably OK.

That said, I do not fully understand the problem being addressed by this PR. @moznion can you please (at a minimum) help us reproduce what you're seeing in a dev environment? If possible, though, a test would be better.

@kbrock
Copy link
Collaborator

kbrock commented Aug 7, 2025

ok, so this is a swap. Sounds like a straight forward solution. Also, sounds like this is a bug.

I also saw one PR asking to add JSON, and worried we are going towards unnecessarily including every module.

Helping us come up with a test case would be great.

…ase does not raise a NoMethodError

Signed-off-by: moznion <moznion@mail.moznion.net>
@moznion
Copy link
Contributor Author

moznion commented Aug 12, 2025

@kbrock @flavorjones Thank you for your suggestions.
I just added a test case in 9c4af39. This test is quite simple but you can reproduce the problem I reported by commenting out extend ActiveModel::Naming in lib/active_hash/base.rb.

Copy link
Collaborator

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

Thanks for posting this fix
LGTM

@kbrock kbrock merged commit de01f82 into active-hash:master Aug 18, 2025
20 checks passed
@moznion moznion deleted the active_model_naming branch August 19, 2025 01:56
@moznion
Copy link
Contributor Author

moznion commented Aug 19, 2025

I appreciate your support. I hope the new version becomes available soon.

@masawada
Copy link

@kbrock @flavorjones Hi, could you release a new version including this fix? Thanks!

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.

4 participants