-
-
Notifications
You must be signed in to change notification settings - Fork 909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add have_delegated_type
matcher
#1606
Conversation
correct = types.all? do |type| | ||
scope_name = type.tableize.tr('/', '_') | ||
singular = scope_name.singularize | ||
query = "#{singular}?" | ||
|
||
Object.const_defined?(type) && @subject.respond_to?(query) && | ||
@subject.respond_to?(singular) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not great, but currently, there's no way to check the types besides doing that. A PR was merged recently, adding a types
method to the object, which will greatly simplify this check. I will update this method once it is released in Rails.
if polymorphic? | ||
subject | ||
else | ||
reflection.klass | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll wait to see what happens when running all the specs, but when the relation is polymorphic, the reflection.klass
method raises an error. I found this while trying to create a spec to replicate a case where you have a polymorphic association or delegated_type one, but you also pass the scopes
qualifier to that association. When trying to make a spec for that cause, I noticed this problem.
My solution was to return a valid ActiveRecord object(which in that case, I returned the subject itself) because this will be used only to grab the value of the where_values_hash
in the ModelReflector#extract_relation_clause_from
, which I think doesn't matter which model you're using to. But we can think on another solution here.
20c4fb7
to
9581bd4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
On this PR, we're adding the matcher for the
delegated_type
macro, which is basically abelongs_to
polymorphic association where you define the possible classes the polymorphic relation can act as.Currently, it is hard to check those possible classes as Rails does not expose them in any way, so we need to check if the methods for those classes are defined, which is not ideal.