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_nameis nil #218When I submitted PR #218, my approach was rather symptomatic. Now, I have a better understanding.
composite_primary_keyas mentioned in the Ruby on Rails 7.1 Release Notes.primary_keyis checked for all ActiveRecord classes, including abstract classes..primary_keymethod implemented in this gem throws an exception for abstract classes becausetable_nameis nil. I added a nil guard to makeAbstractClass.primary_keyreturn nil (Fix issue to avoid errors whentable_nameis nil #218).However, this approach seems inappropriate. As indicated by the following link, ActiveRecord is designed to have
AbstractClass.primary_keyreturn'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_keyis not the concern of this PR, so I have updated the implementation to delegate tosuperin such cases.If there are any other approaches or suggestions, please let me know and I will implement them accordingly.