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

Update active record relations DSL count to accept a block argument #2154

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dmarsh-gusto
Copy link

Motivation

While working on a Rails application, I have code that is of the format Model.count {|m| m.foo_relation.boolean?}.

I don't want to write this as .select{}.count as that would violate the Rubocop::Cop::Performance::Cop

The fix is to update the type signatures DSL for active record relations to allow count to accept an optional block argument.

see https://edgeapi.rubyonrails.org/classes/ActiveRecord/Calculations.html#method-i-count

Implementation

Added to a create_block_param call for active record relations count method, lib/tapioca/dsl/compilers/active_record_relations.rb

Tests

Updated existing tests for active record relations count method to accept an optional block parameter.

@dmarsh-gusto dmarsh-gusto requested a review from a team as a code owner January 16, 2025 17:36
@@ -402,6 +402,7 @@ def create_group_chain_methods(klass)
"count",
parameters: [
create_opt_param("column_name", type: "T.untyped", default: "nil"),
create_block_param("block", type: "T.nilable(T.proc.params(record: T.untyped).returns(T.untyped))"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know why groupchain count is different than regular count definition. I think it'll be good to take a closer look and be consistent if appropriate. Regular one uses overloaded sigs instead

sig.add_param("column_name", "T.nilable(T.any(String, Symbol))")
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants