Skip to content

Conversation

@dlwr
Copy link
Contributor

@dlwr dlwr commented Jun 24, 2024

Related Issues and PRs:

When I submitted PR #218, my approach was rather symptomatic. Now, I have a better understanding.

  • Rails 7.1 has added support for composite_primary_key as mentioned in the Ruby on Rails 7.1 Release Notes.
  • This change ensures that primary_key is checked for all ActiveRecord classes, including abstract classes.
  • The .primary_key method implemented in this gem throws an exception for abstract classes because table_name is nil. I added a nil guard to make AbstractClass.primary_key return nil (Fix issue to avoid errors when table_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 to super in such cases.

If there are any other approaches or suggestions, please let me know and I will implement them accordingly.

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.

1 participant