feat: playground api returns removed context values under a new warnings property (#6784)

This PR expands upon #6773 by returning the list of removed properties
in the API response. To achieve this, I added a new top-level `warnings`
key to the API response and added an `invalidContextProperties` property
under it. This is a list with the keys that were removed.

## Discussion points

**Should we return the type of each removed key's value?** We could
expand upon this by also returning the type that was considered invalid
for the property, e.g. `invalidProp: 'object'`. This would give us more
information that we could display to the user. However, I'm not sure
it's useful? We already return the input as-is, so you can always
cross-check. And the only type we allow for non-`properties` top-level
properties is `string`. Does it give any useful info? I think if we want
to display this in the UI, we might be better off cross-referencing with
the input?

**Can properties be invalid for any other reason?** As far as I can
tell, that's the only reason properties can be invalid for the context.
OpenAPI will prevent you from using a type other than string for the
context fields we have defined and does not let you add non-string
properties to the `properties` object. So all we have to deal with are
top-level properties. And as long as they are strings, then they should
be valid.

**Should we instead infer the diff when creating the model?** In this
first approach, I've amended the `clean-context` function to also return
the list of context fields it has removed. The downside to this approach
is that we need to thread it through a few more hoops. Another approach
would be to compare the input context with the context used to evaluate
one of the features when we create the view model and derive the missing
keys from that. This would probably work in 98 percent of cases.
However, if your result contains no flags, then we can't calculate the
diff. But maybe that's alright? It would likely be fewer lines of code
(but might require additional testing), although picking an environment
from feels hacky.

src/lib/features/playground/clean-context.test.ts

@@ -1,15 +1,15 @@
 import { cleanContext } from './clean-context';
 
-test('strips invalid context properties from the context', async () => {
+const invalidJsonTypes = {
-    const invalidJsonTypes = {
+    object: {},
-        object: {},
+    array: [],
-        array: [],
+    true: true,
-        true: true,
+    false: false,
-        false: false,
+    number: 123,
-        number: 123,
+    null: null,
-        null: null,
+};
-    };
 
+test('strips invalid context properties from the context', async () => {
     const validValues = {
         appName: 'test',
     };
@@ -19,7 +19,7 @@ test('strips invalid context properties from the context', async () => {
         ...validValues,
     };
 
-    const cleanedContext = cleanContext(inputContext);
+    const { context: cleanedContext } = cleanContext(inputContext);
 
     expect(cleanedContext).toStrictEqual(validValues);
 });
@@ -29,7 +29,25 @@ test("doesn't add non-existing properties", async () => {
         appName: 'test',
     };
 
-    const output = cleanContext(input);
+    const { context: output } = cleanContext(input);
 
     expect(output).toStrictEqual(input);
 });
+
+test('it returns the names of all the properties it removed', async () => {
+    const { removedProperties } = cleanContext({
+        appName: 'test',
+        ...invalidJsonTypes,
+    });
+
+    const invalidProperties = Object.keys(invalidJsonTypes);
+
+    // verify that the two lists contain all the same elements
+    expect(removedProperties).toEqual(
+        expect.arrayContaining(invalidProperties),
+    );
+
+    expect(invalidProperties).toEqual(
+        expect.arrayContaining(removedProperties),
+    );
+});

src/lib/features/playground/clean-context.ts

@@ -1,16 +1,26 @@
 import type { SdkContextSchema } from '../../openapi';
 
-export const cleanContext = (context: SdkContextSchema): SdkContextSchema => {
+export const cleanContext = (
+    context: SdkContextSchema,
+): { context: SdkContextSchema; removedProperties: string[] } => {
     const { appName, ...otherContextFields } = context;
+    const removedProperties: string[] = [];
 
     const cleanedContextFields = Object.fromEntries(
-        Object.entries(otherContextFields).filter(
+        Object.entries(otherContextFields).filter(([key, value]) => {
-            ([key, value]) => key === 'properties' || typeof value === 'string',
+            if (key === 'properties' || typeof value === 'string') {
-        ),
+                return true;
+            }
+            removedProperties.push(key);
+            return false;
+        }),
     );
 
     return {
-        ...cleanedContextFields,
+        context: {
-        appName,
+            ...cleanedContextFields,
+            appName,
+        },
+        removedProperties,
     };
 };

src/lib/features/playground/playground-api.e2e.test.ts

@@ -76,3 +76,30 @@ test('returns the input context exactly as it came in, even if invalid values ha
 
     expect(body.input.context).toMatchObject(inputContext);
 });
+
+test('adds all removed top-level context properties to the list of warnings', async () => {
+    const invalidData = {
+        invalid1: {},
+        invalid2: {},
+    };
+
+    const inputContext = {
+        ...invalidData,
+        appName: 'test',
+    };
+
+    const { body } = await app.request
+        .post('/api/admin/playground/advanced')
+        .send({
+            context: inputContext,
+            environments: ['production'],
+            projects: '*',
+        })
+        .expect(200);
+
+    const warned = body.warnings.invalidContextProperties;
+    const invalidKeys = Object.keys(invalidData);
+
+    expect(warned).toEqual(expect.arrayContaining(invalidKeys));
+    expect(invalidKeys).toEqual(expect.arrayContaining(warned));
+});

src/lib/features/playground/playground-service.ts

@@ -103,7 +103,10 @@ export class PlaygroundService {
         context: SdkContextSchema,
         limit: number,
         userId: number,
-    ): Promise<AdvancedPlaygroundFeatureEvaluationResult[]> {
+    ): Promise<{
+        result: AdvancedPlaygroundFeatureEvaluationResult[];
+        invalidContextProperties: string[];
+    }> {
         const segments = await this.segmentReadModel.getActive();
 
         let filteredProjects: typeof projects = projects;
@@ -126,7 +129,9 @@ export class PlaygroundService {
             ),
         );
 
-        const contexts = generateObjectCombinations(cleanContext(context));
+        const { context: cleanedContext, removedProperties } =
+            cleanContext(context);
+        const contexts = generateObjectCombinations(cleanedContext);
 
         validateQueryComplexity(
             environments.length,
@@ -151,7 +156,7 @@ export class PlaygroundService {
         );
         const items = results.flat();
         const itemsByName = groupBy(items, (item) => item.name);
-        return Object.values(itemsByName).map((entries) => {
+        const result = Object.values(itemsByName).map((entries) => {
             const groupedEnvironments = groupBy(
                 entries,
                 (entry) => entry.environment,
@@ -162,6 +167,11 @@ export class PlaygroundService {
                 environments: groupedEnvironments,
             };
         });
+
+        return {
+            result,
+            invalidContextProperties: removedProperties,
+        };
     }
 
     private async evaluate({

src/lib/features/playground/playground-view-model.ts

@@ -40,6 +40,7 @@ const addStrategyEditLink = (
 export const advancedPlaygroundViewModel = (
     input: AdvancedPlaygroundRequestSchema,
     playgroundResult: AdvancedPlaygroundFeatureEvaluationResult[],
+    invalidContextProperties?: string[],
 ): AdvancedPlaygroundResponseSchema => {
     const features = playgroundResult.map(({ environments, ...rest }) => {
         const transformedEnvironments = Object.entries(environments).map(
@@ -79,6 +80,10 @@ export const advancedPlaygroundViewModel = (
         };
     });
 
+    if (invalidContextProperties?.length) {
+        return { features, input, warnings: { invalidContextProperties } };
+    }
+
     return { features, input };
 };
 

src/lib/features/playground/playground.ts

@@ -125,16 +125,21 @@ export default class PlaygroundController extends Controller {
                 ? Number.parseInt(payload?.value)
                 : 15000;
 
-        const result = await this.playgroundService.evaluateAdvancedQuery(
+        const { result, invalidContextProperties } =
-            req.body.projects || '*',
+            await this.playgroundService.evaluateAdvancedQuery(
-            req.body.environments,
+                req.body.projects || '*',
-            req.body.context,
+                req.body.environments,
-            limit,
+                req.body.context,
-            extractUserIdFromUser(user),
+                limit,
-        );
+                extractUserIdFromUser(user),
+            );
 
         const response: AdvancedPlaygroundResponseSchema =
-            advancedPlaygroundViewModel(req.body, result);
+            advancedPlaygroundViewModel(
+                req.body,
+                result,
+                invalidContextProperties,
+            );
 
         res.json(response);
     }

src/lib/openapi/spec/advanced-playground-response-schema.ts

@@ -30,6 +30,20 @@ export const advancedPlaygroundResponseSchema = {
                 $ref: advancedPlaygroundFeatureSchema.$id,
             },
         },
+        warnings: {
+            type: 'object',
+            description: 'Warnings that occurred during evaluation.',
+            properties: {
+                invalidContextProperties: {
+                    type: 'array',
+                    description:
+                        'A list of top-level context properties that were provided as input that are not valid due to being the wrong type.',
+                    items: {
+                        type: 'string',
+                    },
+                },
+            },
+        },
     },
     components: {
         schemas: {