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

Fix signature of GroupChain#size method #2153

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

paracycle
Copy link
Member

Motivation

Follow up to the comment #2044 (comment) on #2044.

Implementation

Handle the size method separately and make it not take any parameters and return an Integer.

Tests

Updated tests

The `size` method does not take any parameters and returns an `Integer`.
@paracycle paracycle requested a review from a team as a code owner January 14, 2025 20:06
@paracycle paracycle merged commit 7527f5b into main Jan 14, 2025
35 of 36 checks passed
@paracycle paracycle deleted the uk-relation-size-method branch January 14, 2025 21:15
@iMacTia
Copy link

iMacTia commented Jan 16, 2025

Hey, I don't think this change is correct?
If you call size on a GroupChain (that is, a Relation where you called group) then the result is actually gonna be a hash, like the previous signature said. This change seem to actually introduce a bug?

We realised this while upgrading to tapioca 0.16.8, we have the following code that works in production:

Model
  .select(...)
  .having(...)
  .group(...)
  .size.values.sum

After the upgrade, we get an error like Method "values" does not exist on "Integer", but when I run this locally I can see #size returns an hash like { id => 2 }.
What did you base this change on, if I may ask? Are there other situations where size would return an Integer when called on a GroupChain?

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

Successfully merging this pull request may close these issues.

3 participants