• 
      

    Fix inflated counts in `GroupMemberCountColumn` and `PendingCountColumn` when both columns are displayed simultaneously.

    Review Request #15095 — Created June 4, 2026 and updated

    Information

    Review Board
    release-8.x

    Reviewers

    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
    with Count('users'), and PendingCountColumn.augment_queryset_for_data()
    annotates the same queryset with Count('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 use distinct=True on 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

    • Ran full test suite.
    • Added test_augment_queryset_for_data to both test files to verify each
      column produces correct counts on its own.
    • Added test_counts_with_both_columns_displayed to GroupListViewTests,
      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
    Fix inflated group member/pending counts when both columns are displayed.
    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. …

    maubin maubin

    continuation line over-indented for visual indent Column: 45 Error code: E127

    reviewbot reviewbot

    continuation line over-indented for visual indent Column: 45 Error code: E127

    reviewbot reviewbot

    PendingCountColumn is used for GroupDataGrid, but also for UsersDataGrid and potentially more future columns. This subquery is only correct for …

    maubin maubin

    Similar to my comment above, the new tests that are in this file shouldn't actually exist here, since they're unique …

    maubin maubin

    continuation line unaligned for hanging indent Column: 25 Error code: E131

    reviewbot reviewbot

    continuation line unaligned for hanging indent Column: 37 Error code: E131

    reviewbot reviewbot
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    dan.casares
    maubin
    1. 
        
    2. Show all issues

      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.

    3. reviewboard/datagrids/columns.py (Diff revision 2)
       
       
       
      Show all issues

      PendingCountColumn is used for GroupDataGrid, but also for UsersDataGrid and potentially more future columns. This subquery is only correct for GroupDataGrid, so we can't use it here in the base class because it would give the wrong results for UsersDataGrid.

      The Django docs mention setting distinct=True as a solution, would that work here?

      1. Good catch. You're right that the Subquery approach was model-specific and would break for UsersDataGrid. Updated to use distinct=True on both Count() annotations instead, which is model-agnostic and is the correct defensive practice for any Count() across an M2M relationship.

        We just need to make sure that any future uses of Count() also need to have distinct, or they too will have this issue.

    4. Show all issues

      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 for GroupDataGrid.

      1. Agreed. Removed those tests from both column test files. The regression test now lives in test_groups_list.py as test_counts_with_both_columns_displayed.

    5. 
        
    dan.casares
    Review request changed
    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 queryset
        with Count('users'), and PendingCountColumn.augment_queryset_for_data()
        annotates the same queryset with Count('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 use correlated Subquery

    ~   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 use distinct=True on 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 GroupMemberCountColumn and
      PendingCountColumn to confirm they still pass.
      ~
    • Ran full test suite.
       
    • Added test_augment_queryset_for_data to both test files to verify each
      column produces correct counts on its own.
    ~  
    • Added test_augment_queryset_for_data_with_pending_count_column to
      GroupMemberCountColumnTests and
      test_augment_queryset_for_data_with_member_count_column to
      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 are 2 and 3. 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_displayed to GroupListViewTests,
      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.
    Commits:
    Summary ID
    Fix inflated group member/pending counts when both columns are displayed.
    06ac167016b17b981ec8842e0d52d579246dfe63
    Fix inflated group member/pending counts when both columns are displayed.
    f721950f05ebd810ced958f457d0ce5335a152c5

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    dan.casares
    Review request changed
    Commits:
    Summary ID
    Fix inflated group member/pending counts when both columns are displayed.
    f721950f05ebd810ced958f457d0ce5335a152c5
    Fix inflated group member/pending counts when both columns are displayed.
    74902656194dbfbef0c26ccbc5de874a26e57268

    Checks run (2 succeeded)

    flake8 passed.
    JSHint passed.