Skip to content
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

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Conversation

matsales28
Copy link
Collaborator

@matsales28 matsales28 commented Jan 19, 2024

On this PR, we're adding the matcher for the delegated_type macro, which is basically a belongs_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.

class Vehicle < ActiveRecord::Base
  delegated_type :drivable, types: %w(Car Truck)
end

# Usage 
RSpec.describe Vehicle, type: :model do
 it { should have_delegated_type(:drivable) }
end

@matsales28 matsales28 self-assigned this Jan 19, 2024
Comment on lines +1737 to +1744
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
Copy link
Collaborator Author

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.

rails/rails#50662

Comment on lines +16 to +20
if polymorphic?
subject
else
reflection.klass
end
Copy link
Collaborator Author

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.

@matsales28 matsales28 marked this pull request as ready for review February 2, 2024 13:53
@matsales28 matsales28 requested review from vsppedro and removed request for vsppedro February 23, 2024 18:50
Copy link
Collaborator

@vsppedro vsppedro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@matsales28 matsales28 merged commit 6fb56db into main Mar 15, 2024
16 checks passed
@matsales28 matsales28 deleted the delegated-types-matcher branch March 15, 2024 14:53
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.

None yet

2 participants