use ActiveRecordColumnTypeHelper for primary key types #2377
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KaanOzkan
approved these changes
Aug 22, 2025
Contributor
KaanOzkan
left a comment
There was a problem hiding this comment.
Can you sign the CLA please? It's a CI check.
| primary_key_type = constant.type_for_attribute(constant.primary_key) | ||
| type = Tapioca::Dsl::Helpers::ActiveModelTypeHelper.type_for(primary_key_type) | ||
| type = RBIHelper.as_non_nilable_type(type) | ||
| column_type_helper = Tapioca::Dsl::Helpers::ActiveRecordColumnTypeHelper.new( |
Contributor
There was a problem hiding this comment.
I think this is a fine change. For AR helper will refer to AM helper so the support for custom types attribute :id, CustomId.new will still work.
Contributor
Author
|
I have signed the CLA! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
I couldn't find an existing issue for this, but I noticed while upgrading from 16 to 17, the ids method was generating as
Array[T.untyped]for our UUID primary keys, while other similar UUID returning methods were generating asStringImplementation
I don't know intention behind
ActiveModelTypeHelpervsActiveRecordColumnHelper, but since these are primary keys and tables exist, it seemed fine to useActiveRecordColumnHelper? That seemed preferable compared to changingActiveModelTypeHelperat all. Also, there is some other code using the Model Helper for:findand also seems to care about primary keys, but I don't know if that should also useActiveRecordColumnHelpertoo.Tests
I wasn't sure the best way to test this, given it seems pretty postgres specific, but happy to add some! Seems like existing tests are passing fwiw.
Thanks for looking!