Use type_for_attribute to determine primary_key for find
#1851
Conversation
| sig { params(args: T.any(String, Symbol, ::ActiveSupport::Multibyte::Chars, T::Boolean, BigDecimal, Numeric, ::ActiveRecord::Type::Binary::Data, ::ActiveRecord::Type::Time::Value, Date, Time, ::ActiveSupport::Duration, T::Class[T.anything])).returns(::Post) } | ||
| sig { params(args: T::Array[T.any(String, Symbol, ::ActiveSupport::Multibyte::Chars, T::Boolean, BigDecimal, Numeric, ::ActiveRecord::Type::Binary::Data, ::ActiveRecord::Type::Time::Value, Date, Time, ::ActiveSupport::Duration, T::Class[T.anything])]).returns(T::Enumerable[::Post]) } | ||
| sig { params(args: T.any(String, Symbol, ::ActiveSupport::Multibyte::Chars, T::Boolean, BigDecimal, Numeric, ::ActiveRecord::Type::Binary::Data, ::ActiveRecord::Type::Time::Value, Date, Time, ::ActiveSupport::Duration, T::Class[T.anything], T.nilable(::CustomId))).returns(::Post) } | ||
| sig { params(args: T::Array[T.any(String, Symbol, ::ActiveSupport::Multibyte::Chars, T::Boolean, BigDecimal, Numeric, ::ActiveRecord::Type::Binary::Data, ::ActiveRecord::Type::Time::Value, Date, Time, ::ActiveSupport::Duration, T::Class[T.anything], T.nilable(::CustomId))]).returns(T::Enumerable[::Post]) } |
There was a problem hiding this comment.
The code should not generate a nilable type, since Model.find(nil) will raise:
[...]/activerecord-7.1.3.2/lib/active_record/relation/finder_methods.rb:497:in 'find_with_ids': Couldn't find Model without an ID (ActiveRecord::RecordNotFound)
There was a problem hiding this comment.
True, this is because the deserialize sig says that's the return value so it's extracting from that. I could do a Regex replace of T.nilable( and )?
There was a problem hiding this comment.
Yeah I guess that's the quickest way. Keep in mind you could also see ::T.nilable since it uses this:
tapioca/lib/tapioca/helpers/rbi_helper.rb
Line 92 in 0c24553
|
Thanks @alex-tan, this needs a rebase on main and a CLA agreement using https://github.com/Shopify/tapioca/actions/runs/8599879286/job/23563693229?pr=1851#step:2:12 |
|
Thanks @KaanOzkan. I'm working on getting the CLA signed by my company. I'll get this rebased in the meantime. |
alex-tan
force-pushed
the
primary-key-attr-reflection
branch
from
da45252
to
c2d409d
Compare
alex-tan
force-pushed
the
primary-key-attr-reflection
branch
from
cf26ef7
to
9c563a2
Compare
|
I have signed the CLA! |
alex-tan
force-pushed
the
primary-key-attr-reflection
branch
from
9c563a2
to
8912c37
Compare
|
Ah weird. I just rebased again 👍 |
Motivation
Issue #1845
We use custom ID types for our models. The current sigs for
finddon't reflect the use ofActiveRecord::Attributes::ClassMethods.attributeImplementation
I utilized
Tapioca::Dsl::Helpers::ActiveModelTypeHelper.type_forwhich reflects on the signatures of theActiveModel::Type::Valuesubclasses to determine what other potential types should be in the signatures.Tests
I have added tests.