fix: config applying issues (@miodec, @fehmer) (#6812)

!nuf

frontend/__tests__/root/config-metadata.spec.ts

@@ -145,7 +145,7 @@ describe("ConfigMeta", () => {
     }> = {
       funbox: [
         {
-          value: "gibberish" as any,
+          value: ["gibberish"],
           given: { mode: "quote" },
           fail: true,
         },

frontend/__tests__/root/config.spec.ts

@@ -2,7 +2,6 @@ import * as Config from "../../src/ts/config";
 import * as Misc from "../../src/ts/utils/misc";
 import {
   CustomThemeColors,
-  FunboxName,
   ConfigKey,
   Config as ConfigType,
   CaretStyleSchema,
@@ -176,7 +175,7 @@ describe("Config", () => {
       }> = {
         funbox: [
           {
-            value: "gibberish" as any,
+            value: ["gibberish"],
             given: { mode: "quote" },
             fail: true,
           },
@@ -945,8 +944,6 @@ describe("Config", () => {
   it("setFunbox", () => {
     expect(Config.setFunbox(["mirror"])).toBe(true);
     expect(Config.setFunbox(["mirror", "58008"])).toBe(true);
-
-    expect(Config.setFunbox([stringOfLength(101) as FunboxName])).toBe(false);
   });
   it("setPaceCaretCustomSpeed", () => {
     expect(Config.setPaceCaretCustomSpeed(0)).toBe(true);
@@ -1099,6 +1096,106 @@ describe("Config", () => {
     expect(Config.setCustomLayoutfluid("qwerty#qwertz" as any)).toBe(false);
     expect(Config.setCustomLayoutfluid("invalid" as any)).toBe(false);
   });
+
+  describe("apply", () => {
+    it("should fill missing values with defaults", () => {
+      //GIVEN
+      Config.apply({
+        numbers: true,
+        punctuation: true,
+      });
+      const config = getConfig();
+      expect(config.mode).toBe("time");
+      expect(config.numbers).toBe(true);
+      expect(config.punctuation).toBe(true);
+    });
+
+    describe("should reset to default if setting failed", () => {
+      const testCases: {
+        display: string;
+        value: Partial<ConfigType>;
+        expected: Partial<ConfigType>;
+      }[] = [
+        {
+          // invalid funbox
+          display: "invalid funbox",
+          value: { funbox: ["invalid_funbox"] as any },
+          expected: { funbox: [] },
+        },
+        {
+          display: "mode incompatible with funbox",
+          value: { mode: "quote", funbox: ["58008"] as any },
+          expected: { funbox: [] },
+        },
+        {
+          display: "invalid customLayoutfluid",
+          value: { funbox: ["58008", "gibberish"] as any },
+          expected: { funbox: [] },
+        },
+      ];
+
+      it.each(testCases)("$display", ({ value, expected }) => {
+        Config.apply(value);
+        const config = getConfig();
+        const applied = Object.fromEntries(
+          Object.entries(config).filter(([key]) =>
+            Object.keys(expected).includes(key)
+          )
+        );
+        expect(applied).toEqual(expected);
+      });
+    });
+
+    describe("should apply keys in an order to avoid overrides", () => {
+      const testCases: {
+        display: string;
+        value: Partial<ConfigType>;
+        expected: Partial<ConfigType>;
+      }[] = [
+        {
+          display: "quote length shouldnt override mode",
+          value: { quoteLength: [0], mode: "time" },
+          expected: { quoteLength: [0], mode: "time" },
+        },
+      ];
+
+      it.each(testCases)("$display", ({ value, expected }) => {
+        Config.apply(value);
+        const config = getConfig();
+        const applied = Object.fromEntries(
+          Object.entries(config).filter(([key]) =>
+            Object.keys(expected).includes(key)
+          )
+        );
+        expect(applied).toEqual(expected);
+      });
+    });
+
+    it("should apply a partial config but keep the rest unchanged", () => {
+      replaceConfig({
+        numbers: true,
+      });
+      Config.apply({
+        punctuation: true,
+      });
+      const config = getConfig();
+      expect(config.numbers).toBe(true);
+    });
+
+    it("should reset all values to default if fullReset is true", () => {
+      replaceConfig({
+        numbers: true,
+      });
+      Config.apply(
+        {
+          punctuation: true,
+        },
+        true
+      );
+      const config = getConfig();
+      expect(config.numbers).toBe(false);
+    });
+  });
 });
 
 function customThemeColors(n: number): CustomThemeColors {

frontend/src/ts/config-metadata.ts

@@ -1,3 +1,4 @@
+import { checkCompatibility } from "@monkeytype/funbox";
 import * as DB from "./db";
 import * as Notifications from "./elements/notifications";
 import { isAuthenticated } from "./firebase";
@@ -5,6 +6,7 @@ import { canSetFunboxWithConfig } from "./test/funbox/funbox-validation";
 import { isDevEnvironment, reloadAfter } from "./utils/misc";
 import * as ConfigSchemas from "@monkeytype/schemas/configs";
 import { roundTo1 } from "@monkeytype/util/numbers";
+import { capitalizeFirstLetter } from "./utils/strings";
 // type SetBlock = {
 //   [K in keyof ConfigSchemas.Config]?: ConfigSchemas.Config[K][];
 // };
@@ -261,7 +263,17 @@ export const configMetadata: ConfigMetadataObject = {
     icon: "fa-gamepad",
     changeRequiresRestart: true,
     isBlocked: ({ value, currentConfig }) => {
-      for (const funbox of currentConfig.funbox) {
+      if (!checkCompatibility(value)) {
+        Notifications.add(
+          `${capitalizeFirstLetter(
+            value.join(", ")
+          )} is an invalid combination of funboxes`,
+          0
+        );
+        return true;
+      }
+
+      for (const funbox of value) {
         if (!canSetFunboxWithConfig(funbox, currentConfig)) {
           Notifications.add(
             `${value}" cannot be enabled with the current config`,
@@ -270,6 +282,7 @@ export const configMetadata: ConfigMetadataObject = {
           return true;
         }
       }
+
       return false;
     },
   },
@@ -750,6 +763,12 @@ export const configMetadata: ConfigMetadataObject = {
   ads: {
     icon: "fa-ad",
     changeRequiresRestart: false,
+    overrideValue: ({ value }) => {
+      if (isDevEnvironment()) {
+        return "off";
+      }
+      return value;
+    },
     isBlocked: ({ value }) => {
       if (value !== "off" && isDevEnvironment()) {
         Notifications.add("Ads are disabled in development mode.", 0);

frontend/src/ts/config.ts

@@ -42,7 +42,7 @@ const configLS = new LocalStorageWithSchema({
   },
 });
 
-let config = {
+let config: Config = {
   ...getDefaultConfig(),
 };
 
@@ -85,20 +85,6 @@ export function saveFullConfigToLocalStorage(noDbCheck = false): void {
   ConfigEvent.dispatch("saveToLocalStorage", stringified);
 }
 
-// type ConfigMetadata = Partial<
-//   Record<
-//     ConfigSchemas.ConfigKey,
-//     {
-//       configKey: ConfigSchemas.ConfigKey;
-//       schema: z.ZodTypeAny;
-//       displayString?: string;
-//       preventSet: (
-//         value: ConfigSchemas.Config[keyof ConfigSchemas.Config]
-//       ) => boolean;
-//     }
-//   >
-// >;
-
 function isConfigChangeBlocked(): boolean {
   if (TestState.isActive && config.funbox.includes("no_quit")) {
     Notifications.add("No quit funbox is active. Please finish the test.", 0, {
@@ -119,6 +105,14 @@ export function genericSet<T extends keyof ConfigSchemas.Config>(
     throw new Error(`Config metadata for key "${key}" is not defined.`);
   }
 
+  if (metadata.overrideValue) {
+    value = metadata.overrideValue({
+      value,
+      currentValue: config[key],
+      currentConfig: config,
+    });
+  }
+
   const previousValue = config[key];
 
   if (
@@ -129,47 +123,40 @@ export function genericSet<T extends keyof ConfigSchemas.Config>(
     Notifications.add("No quit funbox is active. Please finish the test.", 0, {
       important: true,
     });
+    console.warn(
+      `Could not set config key "${key}" with value "${JSON.stringify(
+        value
+      )}" - no quit funbox active.`
+    );
     return false;
   }
 
-  // if (metadata.setBlock) {
-  //   let block = false;
-  //   for (const blockKey of typedKeys(metadata.setBlock)) {
-  //     const blockValues = metadata.setBlock[blockKey] ?? [];
-  //     if (
-  //       config[blockKey] !== undefined &&
-  //       (blockValues as Array<(typeof config)[typeof blockKey]>).includes(
-  //         config[blockKey]
-  //       )
-  //     ) {
-  //       block = true;
-  //       break;
-  //     }
-  //   }
-  //   if (block) {
-  //     return false;
-  //   }
-  // }
-
   if (metadata.isBlocked?.({ value, currentConfig: config })) {
+    console.warn(
+      `Could not set config key "${key}" with value "${JSON.stringify(
+        value
+      )}" - blocked.`
+    );
     return false;
   }
 
-  if (metadata.overrideValue) {
-    value = metadata.overrideValue({
-      value,
-      currentValue: config[key],
-      currentConfig: config,
-    });
-  }
-
   const schema = ConfigSchemas.ConfigSchema.shape[key] as ZodSchema;
 
   if (!isConfigValueValid(metadata.displayString ?? key, value, schema)) {
+    console.warn(
+      `Could not set config key "${key}" with value "${JSON.stringify(
+        value
+      )}" - invalid value.`
+    );
     return false;
   }
 
   if (!canSetConfigWithCurrentFunboxes(key, value, config.funbox)) {
+    console.warn(
+      `Could not set config key "${key}" with value "${JSON.stringify(
+        value
+      )}" - funbox conflict.`
+    );
     return false;
   }
 
@@ -813,34 +800,67 @@ export function setBurstHeatmap(value: boolean, nosave?: boolean): boolean {
   return genericSet("burstHeatmap", value, nosave);
 }
 
+const lastConfigsToApply: Set<keyof Config> = new Set([
+  "punctuation",
+  "numbers",
+  "quoteLength",
+  "time",
+  "words",
+  "mode",
+  "funbox",
+]);
+
 export async function apply(
-  configToApply: Config | Partial<Config>
+  configToApply: Config | Partial<Config>,
+  fullReset = false
 ): Promise<void> {
-  if (configToApply === undefined) return;
+  if (configToApply === undefined || configToApply === null) return;
 
   ConfigEvent.dispatch("fullConfigChange");
 
-  const configObj = configToApply as Config;
-  (Object.keys(getDefaultConfig()) as (keyof Config)[]).forEach((configKey) => {
-    if (configObj[configKey] === undefined) {
-      const newValue = getDefaultConfig()[configKey];
-      (configObj[configKey] as typeof newValue) = newValue;
-    }
-  });
-  if (configObj !== undefined && configObj !== null) {
-    for (const configKey of typedKeys(configObj)) {
-      const configValue = configObj[configKey];
-      genericSet(configKey, configValue, true);
-    }
-
-    ConfigEvent.dispatch(
-      "configApplied",
-      undefined,
-      undefined,
-      undefined,
-      config
-    );
+  const defaultConfig = getDefaultConfig();
+  for (const key of typedKeys(fullReset ? defaultConfig : configToApply)) {
+    //@ts-expect-error this is fine, both are of type config
+    config[key] = defaultConfig[key];
   }
+
+  const partialKeys = typedKeys(configToApply);
+  const partialKeysToApplyFirst = partialKeys.filter(
+    (key) => !lastConfigsToApply.has(key)
+  );
+  const partialKeysToApplyLast = Array.from(lastConfigsToApply.values()).filter(
+    (key) => partialKeys.includes(key)
+  );
+  const partialKeysToApply = [
+    ...partialKeysToApplyFirst,
+    ...partialKeysToApplyLast,
+  ];
+
+  const configKeysToReset: (keyof Config)[] = [];
+
+  for (const configKey of partialKeysToApply) {
+    const configValue = configToApply[
+      configKey
+    ] as ConfigSchemas.Config[keyof Config];
+
+    const set = genericSet(configKey, configValue, true);
+
+    if (!set) {
+      configKeysToReset.push(configKey);
+    }
+  }
+
+  for (const key of configKeysToReset) {
+    saveToLocalStorage(key);
+  }
+
+  ConfigEvent.dispatch(
+    "configApplied",
+    undefined,
+    undefined,
+    undefined,
+    config
+  );
   ConfigEvent.dispatch("fullConfigChangeFinished");
 }
 
@@ -856,7 +876,7 @@ export async function loadFromLocalStorage(): Promise<void> {
   if (newConfig === undefined) {
     await reset();
   } else {
-    await apply(newConfig);
+    await apply(newConfig, true);
     saveFullConfigToLocalStorage(true);
   }
   loadDone();
@@ -889,7 +909,7 @@ export async function applyFromJson(json: string): Promise<void> {
         },
       }
     );
-    await apply(parsedConfig);
+    await apply(parsedConfig, true);
     saveFullConfigToLocalStorage();
     Notifications.add("Done", 1);
   } catch (e) {

frontend/src/ts/controllers/account-controller.ts

@@ -154,7 +154,7 @@ async function getDataAndInit(): Promise<boolean> {
     console.log(
       "no local config or local and db configs are different - applying db"
     );
-    await UpdateConfig.apply(snapshot.config);
+    await UpdateConfig.apply(snapshot.config, true);
     UpdateConfig.saveFullConfigToLocalStorage(true);
 
     //funboxes might be different and they wont activate on the account page

frontend/src/ts/controllers/preset-controller.ts

@@ -15,15 +15,12 @@ export async function apply(_id: string): Promise<void> {
   if (presetToApply === undefined) {
     return;
   }
-  if (isPartialPreset(presetToApply)) {
-    const combinedConfig = {
-      ...UpdateConfig.getConfigChanges(),
-      ...replaceLegacyValues(presetToApply.config),
-    };
-    await UpdateConfig.apply(migrateConfig(combinedConfig));
-  } else {
-    await UpdateConfig.apply(migrateConfig(presetToApply.config));
-  }
+
+  await UpdateConfig.apply(
+    migrateConfig(replaceLegacyValues(presetToApply.config)),
+    !isPartialPreset(presetToApply)
+  );
+
   if (
     !isPartialPreset(presetToApply) ||
     presetToApply.settingGroups?.includes("behavior")

packages/funbox/src/validation.ts

@@ -1,5 +1,5 @@
 import { intersect } from "@monkeytype/util/arrays";
-import { FunboxForcedConfig } from "./types";
+import { FunboxForcedConfig, FunboxMetadata } from "./types";
 import { getFunbox } from "./list";
 import { FunboxName } from "@monkeytype/schemas/configs";
 import { safeNumber } from "@monkeytype/util/numbers";
@@ -9,10 +9,22 @@ export function checkCompatibility(
   withFunbox?: FunboxName
 ): boolean {
   if (funboxNames.length === 0) return true;
-  let funboxesToCheck = getFunbox(funboxNames);
 
-  if (withFunbox !== undefined) {
-    funboxesToCheck = funboxesToCheck.concat(getFunbox(withFunbox));
+  let funboxesToCheck: FunboxMetadata[];
+
+  try {
+    funboxesToCheck = getFunbox(funboxNames);
+
+    if (withFunbox !== undefined) {
+      const toAdd = getFunbox(withFunbox);
+      funboxesToCheck = funboxesToCheck.concat(toAdd);
+    }
+  } catch (error) {
+    console.error(
+      "Error when getting funboxes for a compatibility check:",
+      error
+    );
+    return false;
   }
 
   const allFunboxesAreValid = funboxesToCheck.every((f) => f !== undefined);

packages/funbox/tsconfig.json

@@ -8,7 +8,7 @@
     "moduleResolution": "Bundler",
     "module": "ES6",
     "target": "ES2015",
-    "lib": ["es2019"]
+    "lib": ["es2019", "dom"]
   },
   "include": ["src"],
   "exclude": ["node_modules", "dist"]