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 count issue when deep filtering #13928

Merged
merged 9 commits into from Aug 10, 2022
Merged

Conversation

Bassel17
Copy link
Member

@Bassel17 Bassel17 commented Aug 1, 2022

What does it do?

Changes the count implementation in the query builder, so it uses countDistinct() based on the column id

Why is it needed?

When counting with deep filters columns were counted multiple times based on their relation

How to test it?

Create a content type with a relation (one-to-many or many-to-many) (you can take a look at the issue linked below for more details) add multiple elements to the relation field and then try to get the content type while filtering on that field, the total in the meta should be correct.

Related issue(s)/PR(s)

fix #7631

@Bassel17 Bassel17 added source: core:database Source is core/database package pr: fix This PR is fixing a bug labels Aug 1, 2022
@Bassel17 Bassel17 changed the title count columns based on distinct ID Fix count issue when deep filtering Aug 1, 2022
@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Merging #13928 (0f451ca) into master (36ecf8c) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #13928      +/-   ##
==========================================
- Coverage   55.35%   55.34%   -0.01%     
==========================================
  Files        1258     1258              
  Lines       31699    31702       +3     
  Branches     5734     5735       +1     
==========================================
  Hits        17546    17546              
- Misses      12337    12340       +3     
  Partials     1816     1816              
Flag Coverage Δ
front 58.10% <ø> (ø)
unit 48.58% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/core/database/lib/query/query-builder.js 1.89% <0.00%> (-0.03%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Bassel17 Bassel17 added this to the 4.3.3 milestone Aug 2, 2022
innerdvations
innerdvations previously approved these changes Aug 2, 2022
@Kyle772
Copy link

Kyle772 commented Aug 2, 2022

Thank you for pursuing this @Bassel17 and @alexandrebodin !

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM

@Bassel17 Bassel17 merged commit b21a0c2 into master Aug 10, 2022
@Bassel17 Bassel17 deleted the fix/counting-when-deep-filtering branch August 10, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: core:database Source is core/database package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Count Metadata doesn't take filtering into account
5 participants