From bfb06a22ed132ed5e6347f2879e5d5bb4ed789a2 Mon Sep 17 00:00:00 2001
From: Christian Fehmer <fehmer@users.noreply.github.com>
Date: Tue, 13 May 2025 16:43:45 +0200
Subject: [PATCH] impr(config): handle invalid config values (@fehmer) (#6555)

---
 frontend/__tests__/utils/config.spec.ts | 61 ++++++++++++++++++++-----
 frontend/src/ts/utils/config.ts         | 49 ++++++++++++++++++--
 2 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/frontend/__tests__/utils/config.spec.ts b/frontend/__tests__/utils/config.spec.ts
index 5e5fc90dd..4de9a812b 100644
--- a/frontend/__tests__/utils/config.spec.ts
+++ b/frontend/__tests__/utils/config.spec.ts
@@ -2,6 +2,8 @@ import { getDefaultConfig } from "../../src/ts/constants/default-config";
 import { migrateConfig } from "../../src/ts/utils/config";
 import { PartialConfig } from "@monkeytype/contracts/schemas/configs";
 
+const defaultConfig = getDefaultConfig();
+
 describe("config.ts", () => {
   describe("migrateConfig", () => {
     it("should carry over properties from the default config", () => {
@@ -36,8 +38,40 @@ describe("config.ts", () => {
       expect(result.time).toEqual(120);
       expect(result.accountChart).toEqual(["off", "off", "off", "off"]);
     });
-    it("should not convert legacy values if current values are already present", () => {
-      const testCases = [
+    describe("should replace value with default config if invalid", () => {
+      it.for([
+        {
+          given: { theme: "invalid" },
+          expected: { theme: defaultConfig.theme },
+        },
+        {
+          given: { minWpm: "invalid" },
+          expected: { minWpm: defaultConfig.minWpm },
+        },
+        {
+          given: { customThemeColors: ["#ffffff"] },
+          expected: { customThemeColors: defaultConfig.customThemeColors },
+        },
+        {
+          given: { accountChart: [true, false, false, true] },
+          expected: { accountChart: defaultConfig.accountChart },
+        },
+        {
+          given: {
+            favThemes: ["nord", "invalid", "serika_dark", "invalid2", "8008"],
+          },
+          expected: { favThemes: ["nord", "serika_dark", "8008"] },
+        },
+      ])(`$given`, ({ given, expected }) => {
+        const description = `given: ${JSON.stringify(
+          given
+        )}, expected: ${JSON.stringify(expected)} `;
+        const result = migrateConfig(given);
+        expect(result, description).toEqual(expect.objectContaining(expected));
+      });
+    });
+    describe("should not convert legacy values if current values are already present", () => {
+      it.for([
         {
           given: { showLiveAcc: true, timerStyle: "mini", liveAccStyle: "off" },
           expected: { liveAccStyle: "off" },
@@ -66,18 +100,15 @@ describe("config.ts", () => {
           given: { showTimerProgress: true, timerStyle: "mini" },
           expected: { timerStyle: "mini" },
         },
-      ];
+      ])(`$given`, ({ given, expected }) => {
+        //WHEN
 
-      //WHEN
-      testCases.forEach((test) => {
         const description = `given: ${JSON.stringify(
-          test.given
-        )}, expected: ${JSON.stringify(test.expected)} `;
+          given
+        )}, expected: ${JSON.stringify(expected)} `;
 
-        const result = migrateConfig(test.given);
-        expect(result, description).toEqual(
-          expect.objectContaining(test.expected)
-        );
+        const result = migrateConfig(given);
+        expect(result, description).toEqual(expect.objectContaining(expected));
       });
     });
     describe("should convert legacy values", () => {
@@ -154,6 +185,14 @@ describe("config.ts", () => {
           given: { indicateTypos: true },
           expected: { indicateTypos: "replace" },
         },
+        {
+          given: {
+            favThemes: ["purpurite", "80s_after_dark", "luna", "pulse"],
+          },
+          expected: {
+            favThemes: ["80s_after_dark", "luna", "pulse"],
+          },
+        },
       ])(`$given`, ({ given, expected }) => {
         const description = `given: ${JSON.stringify(
           given
diff --git a/frontend/src/ts/utils/config.ts b/frontend/src/ts/utils/config.ts
index c048a226a..11be340ed 100644
--- a/frontend/src/ts/utils/config.ts
+++ b/frontend/src/ts/utils/config.ts
@@ -7,16 +7,13 @@ import {
 import { typedKeys } from "./misc";
 import * as ConfigSchemas from "@monkeytype/contracts/schemas/configs";
 import { getDefaultConfig } from "../constants/default-config";
-
 /**
  * migrates possible outdated config and merges with the default config values
  * @param config partial or possible outdated config
  * @returns
  */
 export function migrateConfig(config: PartialConfig | object): Config {
-  //todo this assumes config is matching all schemas
-  //i think we should go through each value and validate
-  return mergeWithDefaultConfig(replaceLegacyValues(config));
+  return mergeWithDefaultConfig(sanitizeConfig(replaceLegacyValues(config)));
 }
 
 function mergeWithDefaultConfig(config: PartialConfig): Config {
@@ -30,6 +27,50 @@ function mergeWithDefaultConfig(config: PartialConfig): Config {
   return mergedConfig;
 }
 
+/**
+ * remove all values from the config which are not valid
+ */
+function sanitizeConfig(
+  config: ConfigSchemas.PartialConfig
+): ConfigSchemas.PartialConfig {
+  const validate = ConfigSchemas.PartialConfigSchema.safeParse(config);
+
+  if (validate.success) {
+    return config;
+  }
+
+  const errors: Map<string, number[] | undefined> = new Map();
+  for (const error of validate.error.errors) {
+    const element = error.path[0] as string;
+    let val = errors.get(element);
+    if (typeof error.path[1] === "number") {
+      val = [...(val ?? []), error.path[1]];
+    }
+    errors.set(element, val);
+  }
+
+  return Object.fromEntries(
+    Object.entries(config).map(([key, value]) => {
+      if (!errors.has(key)) {
+        return [key, value];
+      }
+
+      const error = errors.get(key);
+
+      if (
+        Array.isArray(value) &&
+        error !== undefined && //error is not on the array itself
+        error.length < value.length //not all items in the array are invalid
+      ) {
+        //some items of the array are invalid
+        return [key, value.filter((_element, index) => !error.includes(index))];
+      } else {
+        return [key, undefined];
+      }
+    })
+  ) as ConfigSchemas.PartialConfig;
+}
+
 export function replaceLegacyValues(
   configObj: ConfigSchemas.PartialConfig
 ): ConfigSchemas.PartialConfig {