fix(webapp): preserve valid org flags when invalid flag is stored (#5058)

apps/webapp/app/v3/featureFlags.server.ts

@@ -7,7 +7,7 @@ import {
   FeatureFlagCatalog,
   GLOBAL_LOCKED_FLAGS,
   GRACED_FLAG_GROUPS,
-  validatePartialFeatureFlags,
+  mergeOrgFeatureFlags,
 } from "~/v3/featureFlags";
 import { env } from "~/env.server";
 import { isAlertsEnabledEnv } from "~/v3/canUseAlertsList";
@@ -396,15 +396,7 @@ export async function globalFeatureFlags() {
 }
 
 /** The global set with one org's overrides on top. */
-export function mergeOrgFeatureFlags(
-  globalFlags: Partial<FeatureFlagCatalog>,
-  orgFeatureFlags: unknown
-) {
-  const parsed = orgFeatureFlags
-    ? validatePartialFeatureFlags(orgFeatureFlags as Record<string, unknown>)
-    : ({ success: false } as const);
-  return { ...globalFlags, ...(parsed.success ? parsed.data : {}) };
-}
+export { mergeOrgFeatureFlags } from "~/v3/featureFlags";
 
 /**
  * The flags that apply to one organization. Server-side callers that need the

apps/webapp/app/v3/featureFlags.ts

@@ -389,6 +389,15 @@ export function parseStoredOrgFlags(raw: unknown): StoredOrgFlags {
   return result;
 }
 
+/** The global set with one org's overrides on top. */
+export function mergeOrgFeatureFlags(
+  globalFlags: Partial<FeatureFlagCatalog>,
+  orgFeatureFlags: unknown
+): Partial<FeatureFlagCatalog> {
+  const validOrgFlags = orgFeatureFlags ? parseStoredOrgFlags(orgFeatureFlags).valid : {};
+  return { ...globalFlags, ...validOrgFlags };
+}
+
 /** Whether `key` still holds the invalid value the org has stored, i.e. the admin hasn't touched it. */
 export function isUnchangedInvalidFlag(
   key: string,

apps/webapp/test/storedOrgFlags.test.ts

@@ -4,6 +4,7 @@ import {
   ORG_LOCKED_FLAGS,
   ORG_LOCKED_FLAG_EFFECT,
   isUnchangedInvalidFlag,
+  mergeOrgFeatureFlags,
   parseStoredOrgFlags,
   validatePartialFeatureFlags,
 } from "~/v3/featureFlags";
@@ -94,3 +95,46 @@ describe("isUnchangedInvalidFlag", () => {
     expect(isUnchangedInvalidFlag(KEY, { [KEY]: { x: null } }, {})).toBe(false);
   });
 });
+
+describe("mergeOrgFeatureFlags", () => {
+  const globalFlags = {
+    [FEATURE_FLAG.hasLogsPageAccess]: false,
+    [FEATURE_FLAG.hasQueryAccess]: false,
+  };
+
+  it("merges valid org overrides on top of global flags", () => {
+    const result = mergeOrgFeatureFlags(globalFlags, {
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+    });
+    expect(result).toEqual({
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+      [FEATURE_FLAG.hasQueryAccess]: false,
+    });
+  });
+
+  it("preserves valid org flags when an invalid flag is present in stored org flags", () => {
+    const result = mergeOrgFeatureFlags(globalFlags, {
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+      [FEATURE_FLAG.snapshotStoreMode]: "invalid-mode",
+    });
+    expect(result).toEqual({
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+      [FEATURE_FLAG.hasQueryAccess]: false,
+    });
+  });
+
+  it("ignores unknown keys in stored org flags when merging", () => {
+    const result = mergeOrgFeatureFlags(globalFlags, {
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+      unknownKey: "should-be-ignored",
+    });
+    expect(result).toEqual({
+      [FEATURE_FLAG.hasLogsPageAccess]: true,
+      [FEATURE_FLAG.hasQueryAccess]: false,
+    });
+  });
+
+  it.each([null, undefined, "", 123, []])("returns global flags unchanged for %j", (raw) => {
+    expect(mergeOrgFeatureFlags(globalFlags, raw)).toEqual(globalFlags);
+  });
+});