Skip to content

Commit e74cc3a

Browse files
fix(feature-flags): preserve valid org flags when stored flags contain invalid values (#5058)
1 parent aed0848 commit e74cc3a

3 files changed

Lines changed: 28 additions & 5 deletions

File tree

‎apps/webapp/app/components/admin/FeatureFlagsDialog.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { CheckboxWithLabel } from "~/components/primitives/Checkbox";
1515
import { cn } from "~/utils/cn";
1616
import {
1717
FEATURE_FLAG,
18+
derivedFlagsClearedWith,
1819
ORG_LOCKED_FLAG_EFFECT,
1920
ORG_LOCKED_FLAGS,
2021
isUnchangedInvalidFlag,
@@ -96,7 +97,9 @@ export function FeatureFlagsDialog({
9697
const [saveError, setSaveError] = useState<string | null>(null);
9798
const [unlocked, setUnlocked] = useState(false);
9899

99-
const isLocked = (key: string) => !unlocked && ORG_LOCKED_FLAGS.includes(key);
100+
const isLocked = (key: string) =>
101+
(!unlocked || ORG_LOCKED_FLAG_EFFECT[key as FeatureFlagKey] === "derived") &&
102+
ORG_LOCKED_FLAGS.includes(key);
100103

101104
useEffect(() => {
102105
if (open && orgId) {
@@ -148,6 +151,9 @@ export function FeatureFlagsDialog({
148151
setOverrides((prev) => {
149152
const next = { ...prev };
150153
delete next[key];
154+
for (const derivedKey of derivedFlagsClearedWith(key)) {
155+
delete next[derivedKey];
156+
}
151157
return next;
152158
});
153159
};

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
FeatureFlagCatalog,
88
GLOBAL_LOCKED_FLAGS,
99
GRACED_FLAG_GROUPS,
10+
parseStoredOrgFlags,
1011
validatePartialFeatureFlags,
1112
} from "~/v3/featureFlags";
1213
import { env } from "~/env.server";
@@ -400,10 +401,8 @@ export function mergeOrgFeatureFlags(
400401
globalFlags: Partial<FeatureFlagCatalog>,
401402
orgFeatureFlags: unknown
402403
) {
403-
const parsed = orgFeatureFlags
404-
? validatePartialFeatureFlags(orgFeatureFlags as Record<string, unknown>)
405-
: ({ success: false } as const);
406-
return { ...globalFlags, ...(parsed.success ? parsed.data : {}) };
404+
const { valid } = parseStoredOrgFlags(orgFeatureFlags);
405+
return { ...globalFlags, ...valid };
407406
}
408407

409408
/**

‎apps/webapp/test/storedOrgFlags.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { mergeOrgFeatureFlags } from "~/v3/featureFlags.server";
12
import { describe, expect, it } from "vitest";
23
import {
34
FEATURE_FLAG,
@@ -94,3 +95,20 @@ describe("isUnchangedInvalidFlag", () => {
9495
expect(isUnchangedInvalidFlag(KEY, { [KEY]: { x: null } }, {})).toBe(false);
9596
});
9697
});
98+
99+
describe("mergeOrgFeatureFlags", () => {
100+
it("keeps valid org overrides when one stored flag value is invalid", () => {
101+
const globalFlags = {
102+
[FEATURE_FLAG.hasLogsPageAccess]: false,
103+
[FEATURE_FLAG.realtimeBackend]: "redis" as const,
104+
};
105+
const orgFeatureFlags = {
106+
[FEATURE_FLAG.hasLogsPageAccess]: true,
107+
[FEATURE_FLAG.realtimeBackend]: "not-a-backend",
108+
};
109+
110+
const merged = mergeOrgFeatureFlags(globalFlags, orgFeatureFlags);
111+
expect(merged[FEATURE_FLAG.hasLogsPageAccess]).toBe(true);
112+
expect(merged[FEATURE_FLAG.realtimeBackend]).toBe("redis");
113+
});
114+
});

0 commit comments

Comments
 (0)