Fix inflated counts in `GroupMemberCountColumn` and `PendingCountColumn` when both columns are displayed simultaneously.
Review Request #15095 — Created June 4, 2026 and updated
When both the Members and Open Review Requests columns are displayed on the
Groups datagrid (either on the All Groups page or the search results page), the
counts shown for each group are incorrectly inflated — often
by a factor equal to the other column's true count.The root cause is a
Cartesian product in the SQL generated by Django's ORM.
GroupMemberCountColumn.augment_queryset_for_data()annotates the queryset
withCount('users'), andPendingCountColumn.augment_queryset_for_data()
annotates the same queryset withCount('review_requests', filter=...). When
both annotations are applied to the same queryset, Django generates a single
query with two M2M JOINs. These JOINs multiply each other — a group with 2
members and 3 open review requests produces 6 intermediate rows, causing both
COUNT()calls to return 6 instead of the correct values.Both
augment_queryset_for_data()methods now usedistinct=Trueon their
Count()annotations.COUNT(DISTINCT ...)eliminates the duplicates
introduced by the other join, producing correct counts regardless of which
other M2M columns are active simultaneously. This is also the correct defensive
practice for anyCount()across an M2M relationship.https://docs.djangoproject.com/en/4.2/topics/db/aggregation/#combining-multiple-aggregations
- Ran full test suite.
- Added
test_augment_queryset_for_datato both test files to verify each
column produces correct counts on its own. - Added
test_counts_with_both_columns_displayedtoGroupListViewTests,
which enables both the Members and Open Review Requests columns
simultaneously, creates a group with 2 members and 3 open review requests,
and asserts the counts are correct. Confirmed this test fails against the old
code (returning 6 for both) and passes with the fix. - Visually verified on the All Groups page that enabling both the Members and
Open Review Requests columns now shows correct counts and that the values on
the Users and Review Request pages remained unchanged.
| Summary | ID |
|---|---|
| 74902656194dbfbef0c26ccbc5de874a26e57268 |
| Description | From | Last Updated |
|---|---|---|
|
It's also necessary to run the full Review Board test suite as part of the testing done for each change. … |
|
|
|
continuation line over-indented for visual indent Column: 45 Error code: E127 |
|
|
|
continuation line over-indented for visual indent Column: 45 Error code: E127 |
|
|
|
PendingCountColumn is used for GroupDataGrid, but also for UsersDataGrid and potentially more future columns. This subquery is only correct for … |
|
|
|
Similar to my comment above, the new tests that are in this file shouldn't actually exist here, since they're unique … |
|
|
|
continuation line unaligned for hanging indent Column: 25 Error code: E131 |
|
|
|
continuation line unaligned for hanging indent Column: 37 Error code: E131 |
|
- Commits:
-
Summary ID f591dd0a6254ef88eaeb7498de506f6ea73b884b 06ac167016b17b981ec8842e0d52d579246dfe63
Checks run (2 succeeded)
-
-
It's also necessary to run the full Review Board test suite as part of the testing done for each change. This is useful to check if your change has any unexpected side effects on other parts of the codebase.
-
PendingCountColumnis used forGroupDataGrid, but also forUsersDataGridand potentially more future columns. This subquery is only correct forGroupDataGrid, so we can't use it here in the base class because it would give the wrong results forUsersDataGrid.The Django docs mention setting
distinct=Trueas a solution, would that work here? -
Similar to my comment above, the new tests that are in this file shouldn't actually exist here, since they're unique to Groups and the
GroupDataGrid's pending count column.You should remove these tests and I think add tests to
UsersDataGridTests, and create tests forGroupDataGrid.
- Description:
-
When both the Members and Open Review Requests columns are displayed on the
~ Groups datagrid, the counts shown for each group are incorrectly inflated, ~ often by a factor equal to the other column's true count. ~ Groups datagrid (either on the All Groups page or the search results page), the ~ counts shown for each group are incorrectly inflated — often + by a factor equal to the other column's true count. ~ The root cause is a Cartesian product in the SQL generated by Django's ORM.
~ The root cause is a
+ Cartesian product in the SQL generated by Django's ORM. GroupMemberCountColumn.augment_queryset_for_data()annotates the querysetwith Count('users'), andPendingCountColumn.augment_queryset_for_data()annotates the same queryset with Count('review_requests', filter=...). Whenboth annotations are applied to the same queryset, Django generates a single query with two M2M JOINs. These JOINs multiply each other — a group with 2 members and 3 open review requests produces 6 intermediate rows, causing both COUNT()calls to return 6 instead of the correct values.~ Both
augment_queryset_for_data()methods now use correlatedSubquery~ expressions instead of bare Count()annotations. Each subquery computes its~ count independently for each group row, so the two M2M JOINs no longer ~ interact. ~ Both
augment_queryset_for_data()methods now usedistinct=Trueon their~ Count()annotations.COUNT(DISTINCT ...)eliminates the duplicates~ introduced by the other join, producing correct counts regardless of which ~ other M2M columns are active simultaneously. This is also the correct defensive + practice for any Count()across an M2M relationship.https://docs.djangoproject.com/en/4.2/topics/db/aggregation/#combining-multiple-aggregations
- Testing Done:
-
~ - Ran the existing unit tests for
GroupMemberCountColumnand
PendingCountColumnto confirm they still pass.
~ - Ran full test suite.
- Added
test_augment_queryset_for_datato both test files to verify each
column produces correct counts on its own.
~ - Added
test_augment_queryset_for_data_with_pending_count_columnto
GroupMemberCountColumnTestsand
test_augment_queryset_for_data_with_member_count_columnto
PendingCountColumnTests— both set up a group with 2 members and 3 open
review requests, apply both column augmentations to the same queryset, and
assert the counts are2and3. Confirmed these tests fail against the old
code (returning 6 for both) and pass with the fix.
~ - Visually verified on the All Groups page that enabling both the Members and
Open Review Requests columns now shows correct counts.
~ - Added
test_counts_with_both_columns_displayedtoGroupListViewTests,
which enables both the Members and Open Review Requests columns
simultaneously, creates a group with 2 members and 3 open review requests,
and asserts the counts are correct. Confirmed this test fails against the old
code (returning 6 for both) and passes with the fix.
~ - Visually verified on the All Groups page that enabling both the Members and
Open Review Requests columns now shows correct counts and that the values on
the Users and Review Request pages remained unchanged.
- Ran the existing unit tests for
- Commits:
-
Summary ID 06ac167016b17b981ec8842e0d52d579246dfe63 f721950f05ebd810ced958f457d0ce5335a152c5