From 0a43d341c0163733e96ca18d218b84e66ea0b0f5 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Mon, 27 Nov 2023 11:20:25 +0100
Subject: [PATCH] fix: check whether a usage data is defined (#5393)

The previous check would return `false` if the value was 0, causing a
bug where the usage data wouldn't be included.

This also adds tests to ensure that usage data for CR segments is
propagated correctly because that's where I first encountered the issue.

Before this fix, if the values were 0, the data would display like the
bottom element in the screenshot:


![image](https://github.com/Unleash/unleash/assets/17786332/9642b945-12c4-4217-aec9-7fef4a88e9af)
---
 src/lib/db/segment-store.ts                |  5 +-
 src/test/e2e/api/admin/segment.e2e.test.ts | 83 +++++++++++++++++++---
 2 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/src/lib/db/segment-store.ts b/src/lib/db/segment-store.ts
index 7e86fdb9c4..1ab21549d8 100644
--- a/src/lib/db/segment-store.ts
+++ b/src/lib/db/segment-store.ts
@@ -12,6 +12,7 @@ import { PartialSome } from '../types/partial';
 import User from '../types/user';
 import { Db } from './db';
 import { IFlagResolver } from '../types';
+import { isDefined } from '../util';
 
 const T = {
     segments: 'segments',
@@ -373,10 +374,10 @@ export default class SegmentStore implements ISegmentStore {
             constraints: row.constraints,
             createdBy: row.created_by,
             createdAt: row.created_at,
-            ...(row.used_in_projects && {
+            ...(isDefined(row.used_in_projects) && {
                 usedInProjects: Number(row.used_in_projects),
             }),
-            ...(row.used_in_features && {
+            ...(isDefined(row.used_in_features) && {
                 usedInFeatures: Number(row.used_in_features),
             }),
         };
diff --git a/src/test/e2e/api/admin/segment.e2e.test.ts b/src/test/e2e/api/admin/segment.e2e.test.ts
index cad9c0670d..3845e8f965 100644
--- a/src/test/e2e/api/admin/segment.e2e.test.ts
+++ b/src/test/e2e/api/admin/segment.e2e.test.ts
@@ -108,17 +108,19 @@ const validateSegment = (
         .expect(expectStatusCode);
 
 beforeAll(async () => {
-    db = await dbInit('segments_api_serial', getLogger);
-    app = await setupAppWithCustomConfig(
-        db.stores,
-        {
-            experimental: {
-                flags: {
-                    anonymiseEventLog: true,
-                    detectSegmentUsageInChangeRequests: true,
-                },
+    const customOptions = {
+        experimental: {
+            flags: {
+                anonymiseEventLog: true,
+                detectSegmentUsageInChangeRequests: true,
             },
         },
+    };
+
+    db = await dbInit('segments_api_serial', getLogger, customOptions);
+    app = await setupAppWithCustomConfig(
+        db.stores,
+        customOptions,
         db.rawDatabase,
     );
 });
@@ -630,4 +632,67 @@ describe('detect strategy usage in change requests', () => {
 
         expect(changeRequestStrategies).toMatchObject([{ id: strategyId }]);
     });
+
+    test('Should show usage in features and projects in CRs', async () => {
+        // Change request data is only counted for enterprise
+        // instances, so we'll instantiate our own version of the app
+        // for that.
+        const enterpriseApp = await setupAppWithCustomConfig(
+            db.stores,
+            {
+                enterpriseVersion: '5.3.0',
+                ui: { environment: 'Enterprise' },
+                experimental: {
+                    flags: {
+                        detectSegmentUsageInChangeRequests: true,
+                    },
+                },
+            },
+            db.rawDatabase,
+        );
+
+        // likewise, we want to fetch from the right app to make sure
+        // we get the right data
+        const enterpriseFetchSegments = () =>
+            enterpriseApp.request
+                .get(SEGMENTS_BASE_PATH)
+                .expect(200)
+                .then((res) => res.body.segments);
+
+        // because they use the same db, we can use the regular app
+        // (through `createSegment` and `createFeatureToggle`) to
+        // create the segment and the flag
+        await createSegment({ name: 'a', constraints: [] });
+        const toggle = mockFeatureToggle();
+        await createFeatureToggle(app, toggle);
+        const [segment] = await enterpriseFetchSegments();
+
+        expect(segment).toMatchObject({ usedInFeatures: 0, usedInProjects: 0 });
+
+        await db.rawDatabase.table('change_request_events').insert({
+            feature: toggle.name,
+            action: 'addStrategy',
+            payload: {
+                name: 'flexibleRollout',
+                title: '',
+                disabled: false,
+                segments: [segment.id],
+                variants: [],
+                parameters: {
+                    groupId: toggle.name,
+                    rollout: '100',
+                    stickiness: 'default',
+                },
+                constraints: [],
+            },
+            created_at: '2023-01-01 00:01:00',
+            change_request_id: CR_ID,
+            created_by: user.id,
+        });
+
+        const segments = await enterpriseFetchSegments();
+        expect(segments).toMatchObject([
+            { usedInFeatures: 1, usedInProjects: 1 },
+        ]);
+    });
 });