Delegate primary_key to super in abstract class #240
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues and PRs:
table_name
is nil #218When I submitted PR #218, my approach was rather symptomatic. Now, I have a better understanding.
composite_primary_key
as mentioned in the Ruby on Rails 7.1 Release Notes.primary_key
is checked for all ActiveRecord classes, including abstract classes..primary_key
method implemented in this gem throws an exception for abstract classes becausetable_name
is nil. I added a nil guard to makeAbstractClass.primary_key
return nil (Fix issue to avoid errors whentable_name
is nil #218).However, this approach seems inappropriate. As indicated by the following link, ActiveRecord is designed to have
AbstractClass.primary_key
return'id'
and not nil:https://github.com/rails/rails/blob/de4d8744744acab2dd9db0683ccf784ea45810b2/activerecord/lib/active_record/attribute_methods/primary_key.rb#L106-L116
The value of
AbstractClass.primary_key
is not the concern of this PR, so I have updated the implementation to delegate tosuper
in such cases.If there are any other approaches or suggestions, please let me know and I will implement them accordingly.