From 593019f609c2da05385979afb429c8c235bfde7e Mon Sep 17 00:00:00 2001
From: Haven <haven@posthog.com>
Date: Mon, 18 Nov 2024 11:52:57 -0800
Subject: [PATCH] fix(flags): render # of affected users for conditions sets
 that have no conditions (#26137)

---
 .../FeatureFlagReleaseConditionsLogic.ts      | 17 ++++-
 .../featureFlagReleaseConditionsLogic.test.ts | 66 +++++++++++--------
 2 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts
index 88ae79a8fa1..9c247bf0d6c 100644
--- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts
+++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts
@@ -164,7 +164,7 @@ export const featureFlagReleaseConditionsLogic = kea<featureFlagReleaseCondition
             actions.setTotalUsers(response.total_users)
         },
         addConditionSet: () => {
-            actions.setAffectedUsers(values.filters.groups.length - 1, -1)
+            actions.setAffectedUsers(values.filters.groups.length - 1, values.totalUsers || -1)
         },
         removeConditionSet: ({ index }) => {
             const previousLength = Object.keys(values.affectedUsers).length
@@ -183,9 +183,20 @@ export const featureFlagReleaseConditionsLogic = kea<featureFlagReleaseCondition
                 actions.setAffectedUsers(index, undefined)
 
                 const properties = condition.properties
-                if (!properties || properties?.length === 0 || properties.some(isEmptyProperty)) {
-                    // don't compute for full rollouts or empty conditions
+                if (!properties || properties.some(isEmptyProperty)) {
+                    // don't compute for incomplete conditions
                     usersAffected.push(Promise.resolve({ users_affected: -1, total_users: -1 }))
+                } else if (properties.length === 0) {
+                    // Request total users for empty condition sets
+                    const responsePromise = api.create(
+                        `api/projects/${values.currentTeamId}/feature_flags/user_blast_radius`,
+                        {
+                            condition: { properties: [] },
+                            group_type_index: values.filters?.aggregation_group_type_index ?? null,
+                        }
+                    )
+
+                    usersAffected.push(responsePromise)
                 } else {
                     const responsePromise = api.create(
                         `api/projects/${values.currentTeamId}/feature_flags/user_blast_radius`,
diff --git a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts
index 1ea91f0d9f0..9ee90f5307c 100644
--- a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts
+++ b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts
@@ -128,6 +128,7 @@ describe('the feature flag release conditions logic', () => {
                     .mockReturnValueOnce(Promise.resolve({ users_affected: 140, total_users: 2000 }))
                     .mockReturnValueOnce(Promise.resolve({ users_affected: 240, total_users: 2002 }))
                     .mockReturnValueOnce(Promise.resolve({ users_affected: 500, total_users: 2000 }))
+                    .mockReturnValueOnce(Promise.resolve({ users_affected: 750, total_users: 2001 }))
 
                 logic.mount()
             })
@@ -138,30 +139,44 @@ describe('the feature flag release conditions logic', () => {
                 })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: -1, 1: undefined, 2: undefined, 3: undefined },
+                    affectedUsers: { 0: 140, 1: undefined, 2: undefined, 3: undefined },
                     totalUsers: null,
                 })
                 .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: -1, 1: 140 },
-                    totalUsers: 2000,
-                })
-                .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
-                .toMatchValues({
-                    affectedUsers: { 0: -1, 1: 140, 2: 240 },
+                    affectedUsers: { 0: 140, 1: 240 },
                     totalUsers: 2002,
                 })
                 .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: -1, 1: 140, 2: 240, 3: 500 },
+                    affectedUsers: { 0: 140, 1: 240, 2: 500 },
                     totalUsers: 2000,
                 })
+                .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
+                .toMatchValues({
+                    affectedUsers: { 0: 140, 1: 240, 2: 500, 3: 750 },
+                    totalUsers: 2001,
+                })
         })
 
         it('updates when adding conditions to a flag', async () => {
             jest.spyOn(api, 'create')
-                .mockReturnValueOnce(Promise.resolve({ users_affected: 140, total_users: 2000 }))
-                .mockReturnValueOnce(Promise.resolve({ users_affected: 240, total_users: 2000 }))
+                .mockReturnValueOnce(Promise.resolve({ users_affected: 124, total_users: 2000 }))
+                .mockReturnValueOnce(Promise.resolve({ users_affected: 248, total_users: 2000 }))
+                .mockReturnValueOnce(Promise.resolve({ users_affected: 496, total_users: 2000 }))
+
+            logic?.unmount()
+            logic = featureFlagReleaseConditionsLogic({
+                id: '5678',
+                filters: generateFeatureFlagFilters([
+                    {
+                        properties: [],
+                        rollout_percentage: 50,
+                        variant: null,
+                    },
+                ]),
+            })
+            logic.mount()
 
             await expectLogic(logic, () => {
                 logic.actions.updateConditionSet(0, 20, [
@@ -176,12 +191,11 @@ describe('the feature flag release conditions logic', () => {
                 // first call is to clear the affected users on mount
                 // second call is to set the affected users for mount logic conditions
                 // third call is to set the affected users for the updateConditionSet action
-                .toDispatchActions(['setAffectedUsers', 'setAffectedUsers', 'setAffectedUsers'])
+                .toDispatchActions(['setAffectedUsers', 'setAffectedUsers', 'setAffectedUsers', 'setTotalUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: undefined },
-                    totalUsers: null,
+                    affectedUsers: { 0: 124 },
+                    totalUsers: 2000,
                 })
-                .toNotHaveDispatchedActions(['setTotalUsers'])
 
             await expectLogic(logic, () => {
                 logic.actions.updateConditionSet(0, 20, [
@@ -196,11 +210,11 @@ describe('the feature flag release conditions logic', () => {
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
                     affectedUsers: { 0: undefined },
-                    totalUsers: null,
+                    totalUsers: 2000,
                 })
                 .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 140 },
+                    affectedUsers: { 0: 248 },
                     totalUsers: 2000,
                 })
 
@@ -210,7 +224,8 @@ describe('the feature flag release conditions logic', () => {
             })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 140, 1: -1 },
+                    // expect the new empty condition set to initialize affected users to be same as total users
+                    affectedUsers: { 0: 248, 1: 2000 },
                     totalUsers: 2000,
                 })
                 .toNotHaveDispatchedActions(['setTotalUsers'])
@@ -228,7 +243,7 @@ describe('the feature flag release conditions logic', () => {
             })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 140, 1: undefined },
+                    affectedUsers: { 0: 248, 1: undefined },
                     totalUsers: 2000,
                 })
                 .toNotHaveDispatchedActions(['setTotalUsers'])
@@ -246,12 +261,12 @@ describe('the feature flag release conditions logic', () => {
             })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 140, 1: undefined },
+                    affectedUsers: { 0: 248, 1: undefined },
                     totalUsers: 2000,
                 })
                 .toDispatchActions(['setAffectedUsers', 'setTotalUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 140, 1: 240 },
+                    affectedUsers: { 0: 248, 1: 496 },
                     totalUsers: 2000,
                 })
 
@@ -261,11 +276,11 @@ describe('the feature flag release conditions logic', () => {
             })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 240, 1: 240 },
+                    affectedUsers: { 0: 496, 1: 496 },
                 })
                 .toDispatchActions(['setAffectedUsers'])
                 .toMatchValues({
-                    affectedUsers: { 0: 240, 1: undefined },
+                    affectedUsers: { 0: 496, 1: undefined },
                 })
         })
 
@@ -313,7 +328,6 @@ describe('the feature flag release conditions logic', () => {
                 jest.spyOn(api, 'create')
 
                 logic?.unmount()
-
                 logic = featureFlagReleaseConditionsLogic({
                     id: '12345',
                     filters: generateFeatureFlagFilters([
@@ -359,11 +373,11 @@ describe('the feature flag release conditions logic', () => {
                         'setTotalUsers',
                     ])
                     .toMatchValues({
-                        affectedUsers: { 0: -1, 1: 120, 2: 120 },
+                        affectedUsers: { 0: 120, 1: 120, 2: 120 },
                         totalUsers: 2000,
                     })
 
-                expect(api.create).toHaveBeenCalledTimes(2)
+                expect(api.create).toHaveBeenCalledTimes(4)
 
                 await expectLogic(logic, () => {
                     logic.actions.updateConditionSet(0, 20, undefined, undefined)
@@ -378,7 +392,7 @@ describe('the feature flag release conditions logic', () => {
                 }).toNotHaveDispatchedActions(['setAffectedUsers', 'setTotalUsers'])
 
                 // no extra calls when changing rollout percentage
-                expect(api.create).toHaveBeenCalledTimes(2)
+                expect(api.create).toHaveBeenCalledTimes(4)
             })
         })
     })