fix: Ensure that test data user targets are handled correctly. (#223)

User targets require special logic to put the variation/values into
`targets` instead of just in `contextTargets`. This was not being done,
so user specific targets were not evaluating correctly.

Github issue: launchdarkly/node-server-sdk#283

Author: kinyoklion

packages/shared/sdk-server/__tests__/LDClient.evaluation.test.ts

@@ -137,6 +137,29 @@ describe('given an LDClient with test data', () => {
       reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' },
     });
   });
+
+  it('evaluates and updates user targets from test data', async () => {
+    // This is a specific test case for a customer issue.
+    // It tests the combination of evaluation and test data functionality.
+    const userUuid = '1234';
+    const userContextObject = {
+      kind: 'user',
+      key: userUuid,
+    };
+
+    await td.update(
+      td.flag('my-feature-flag-1').variationForUser(userUuid, false).fallthroughVariation(false),
+    );
+    const valueA = await client.variation('my-feature-flag-1', userContextObject, 'default');
+    expect(valueA).toEqual(false);
+
+    await td.update(
+      td.flag('my-feature-flag-1').variationForUser(userUuid, true).fallthroughVariation(false),
+    );
+
+    const valueB = await client.variation('my-feature-flag-1', userContextObject, 'default');
+    expect(valueB).toEqual(true);
+  });
 });
 
 describe('given an offline client', () => {

packages/shared/sdk-server/__tests__/integrations/test_data/TestData.test.ts

@@ -207,9 +207,8 @@ describe('given a TestData instance', () => {
     expect(builtFlag.fallthrough).toEqual({ variation: 1 });
     expect(builtFlag.offVariation).toEqual(0);
     expect(builtFlag.variations).toEqual([true, false]);
-    expect(builtFlag.contextTargets).toEqual([
-      { contextKind: 'user', values: ['billy'], variation: 0 },
-    ]);
+    expect(builtFlag.contextTargets).toEqual([{ contextKind: 'user', values: [], variation: 0 }]);
+    expect(builtFlag.targets).toEqual([{ values: ['billy'], variation: 0 }]);
     expect(builtFlag.rules).toEqual(flagRules);
   });
 
@@ -249,16 +248,32 @@ describe('given a TestData instance', () => {
   it('can set boolean values for a specific user target', () => {
     const flag = td.flag('test-flag').variationForContext('user', 'potato', false);
     const flag2 = td.flag('test-flag').variationForUser('potato', true);
-    expect(flag.build(0).contextTargets).toEqual([
+    const builtFlag1 = flag.build(0);
+    const builtFlag2 = flag2.build(0);
+    // User targets order by the context targets, but use values from
+    // the legacy targets.
+    expect(builtFlag1.contextTargets).toEqual([
       {
         contextKind: 'user',
+        variation: 1,
+        values: [],
+      },
+    ]);
+    expect(builtFlag1.targets).toEqual([
+      {
         variation: 1,
         values: ['potato'],
       },
     ]);
-    expect(flag2.build(0).contextTargets).toEqual([
+    expect(builtFlag2.contextTargets).toEqual([
       {
         contextKind: 'user',
+        variation: 0,
+        values: [],
+      },
+    ]);
+    expect(builtFlag2.targets).toEqual([
+      {
         variation: 0,
         values: ['potato'],
       },
@@ -347,13 +362,32 @@ describe('given a TestData instance', () => {
   it('can move a targeted context from one variation to another', () => {
     const flag = td
       .flag('test-flag')
-      .variationForContext('user', 'ben', false)
-      .variationForContext('user', 'ben', true);
+      .variationForContext('org', 'ben', false)
+      .variationForContext('org', 'ben', true);
+    // Because there was only one target in the first variation there will be only
+    // a single variation after that target is removed.
+    expect(flag.build(1).contextTargets).toEqual([
+      {
+        contextKind: 'org',
+        variation: 0,
+        values: ['ben'],
+      },
+    ]);
+  });
+
+  it('can move a targeted user from one variation to another', () => {
+    const flag = td.flag('test-flag').variationForUser('ben', false).variationForUser('ben', true);
     // Because there was only one target in the first variation there will be only
     // a single variation after that target is removed.
     expect(flag.build(1).contextTargets).toEqual([
       {
         contextKind: 'user',
+        variation: 0,
+        values: [],
+      },
+    ]);
+    expect(flag.build(1).targets).toEqual([
+      {
         variation: 0,
         values: ['ben'],
       },
@@ -363,19 +397,51 @@ describe('given a TestData instance', () => {
   it('if a targeted context is moved from one variation to another, then other targets remain for that variation', () => {
     const flag = td
       .flag('test-flag')
-      .variationForContext('user', 'ben', false)
-      .variationForContext('user', 'joe', false)
-      .variationForContext('user', 'ben', true);
+      .variationForUser('ben', false)
+      .variationForUser('joe', false)
+      .variationForUser('ben', true);
 
     expect(flag.build(1).contextTargets).toEqual([
       {
         contextKind: 'user',
         variation: 0,
-        values: ['ben'],
+        values: [],
       },
       {
         contextKind: 'user',
         variation: 1,
+        values: [],
+      },
+    ]);
+
+    expect(flag.build(1).targets).toEqual([
+      {
+        variation: 0,
+        values: ['ben'],
+      },
+      {
+        variation: 1,
+        values: ['joe'],
+      },
+    ]);
+  });
+
+  it('if a targeted user is moved from one variation to another, then other targets remain for that variation', () => {
+    const flag = td
+      .flag('test-flag')
+      .variationForContext('org', 'ben', false)
+      .variationForContext('org', 'joe', false)
+      .variationForContext('org', 'ben', true);
+
+    expect(flag.build(1).contextTargets).toEqual([
+      {
+        contextKind: 'org',
+        variation: 0,
+        values: ['ben'],
+      },
+      {
+        contextKind: 'org',
+        variation: 1,
         values: ['joe'],
       },
     ]);
@@ -393,13 +459,14 @@ describe('given a TestData instance', () => {
       {
         contextKind: 'user',
         variation: 1,
-        values: ['ben'],
+        values: [],
       },
       {
         contextKind: 'potato',
         variation: 1,
         values: ['russet', 'yukon'],
       },
     ]);
+    expect(flag.build(0).targets).toEqual([{ variation: 1, values: ['ben'] }]);
   });
 });

packages/shared/sdk-server/src/integrations/test_data/TestDataFlagBuilder.ts

@@ -394,18 +394,24 @@ export default class TestDataFlagBuilder {
 
     if (this.data.targetsByVariation) {
       const contextTargets: Target[] = [];
+      const userTargets: Omit<Target, 'contextKind'>[] = [];
       Object.entries(this.data.targetsByVariation).forEach(
         ([variation, contextTargetsForVariation]) => {
           Object.entries(contextTargetsForVariation).forEach(([contextKind, values]) => {
+            const numberVariation = parseInt(variation, 10);
             contextTargets.push({
               contextKind,
-              values,
+              values: contextKind === 'user' ? [] : values,
               // Iterating the object it will be a string.
-              variation: parseInt(variation, 10),
+              variation: numberVariation,
             });
+            if (contextKind === 'user') {
+              userTargets.push({ values, variation: numberVariation });
+            }
           });
         },
       );
+      baseFlagObject.targets = userTargets;
       baseFlagObject.contextTargets = contextTargets;
     }