Skip to content

Commit aa67ce2

Browse files
committed
fix(webapp): read valid stored org flags key-by-key in mergeOrgFeatureFlags
1 parent aed0848 commit aa67ce2

3 files changed

Lines changed: 38 additions & 6 deletions

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
FEATURE_FLAG,
1818
ORG_LOCKED_FLAG_EFFECT,
1919
ORG_LOCKED_FLAGS,
20+
derivedFlagsClearedWith,
2021
isUnchangedInvalidFlag,
2122
type FeatureFlagKey,
2223
type FlagControlType,
@@ -96,12 +97,15 @@ 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+
ORG_LOCKED_FLAG_EFFECT[key as FeatureFlagKey] === "derived" ||
102+
(!unlocked && ORG_LOCKED_FLAGS.includes(key as FeatureFlagKey));
100103

101104
useEffect(() => {
102105
if (open && orgId) {
103106
// oxlint-disable-next-line react/set-state-in-effect -- This effect intentionally synchronizes local state after an external or lifecycle change.
104107
setSaveError(null);
108+
setUnlocked(false);
105109
setOverrides({});
106110
setInitialOverrides({});
107111
loadFeatureFlags(`/admin/api/v2/orgs/${orgId}/feature-flags`);
@@ -148,6 +152,9 @@ export function FeatureFlagsDialog({
148152
setOverrides((prev) => {
149153
const next = { ...prev };
150154
delete next[key];
155+
for (const derived of derivedFlagsClearedWith(key)) {
156+
delete next[derived];
157+
}
151158
return next;
152159
});
153160
};

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
FeatureFlagCatalog,
88
GLOBAL_LOCKED_FLAGS,
99
GRACED_FLAG_GROUPS,
10-
validatePartialFeatureFlags,
10+
parseStoredOrgFlags,
1111
} from "~/v3/featureFlags";
1212
import { env } from "~/env.server";
1313
import { isAlertsEnabledEnv } from "~/v3/canUseAlertsList";
@@ -400,10 +400,7 @@ export function mergeOrgFeatureFlags(
400400
globalFlags: Partial<FeatureFlagCatalog>,
401401
orgFeatureFlags: unknown
402402
) {
403-
const parsed = orgFeatureFlags
404-
? validatePartialFeatureFlags(orgFeatureFlags as Record<string, unknown>)
405-
: ({ success: false } as const);
406-
return { ...globalFlags, ...(parsed.success ? parsed.data : {}) };
403+
return { ...globalFlags, ...parseStoredOrgFlags(orgFeatureFlags).valid };
407404
}
408405

409406
/**

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
parseStoredOrgFlags,
88
validatePartialFeatureFlags,
99
} from "~/v3/featureFlags";
10+
import { mergeOrgFeatureFlags } from "~/v3/featureFlags.server";
1011

1112
describe("parseStoredOrgFlags", () => {
1213
it("matches validatePartialFeatureFlags for an all-valid blob", () => {
@@ -64,6 +65,33 @@ describe("parseStoredOrgFlags", () => {
6465
});
6566
});
6667

68+
describe("mergeOrgFeatureFlags", () => {
69+
it("applies valid org overrides while falling back to global for invalid and unknown keys", () => {
70+
const globalFlags = {
71+
[FEATURE_FLAG.hasLogsPageAccess]: false,
72+
[FEATURE_FLAG.realtimeBackend]: "electric" as const,
73+
[FEATURE_FLAG.hasQueryAccess]: false,
74+
};
75+
76+
const merged = mergeOrgFeatureFlags(globalFlags, {
77+
[FEATURE_FLAG.hasLogsPageAccess]: true,
78+
[FEATURE_FLAG.realtimeBackend]: "not-a-backend",
79+
someNewerFlag: true,
80+
});
81+
82+
expect(merged).toEqual({
83+
[FEATURE_FLAG.hasLogsPageAccess]: true,
84+
[FEATURE_FLAG.realtimeBackend]: "electric",
85+
[FEATURE_FLAG.hasQueryAccess]: false,
86+
});
87+
});
88+
89+
it.each([null, undefined, [], "invalid"])("returns global flags when org flags are %j", (raw) => {
90+
const globalFlags = { [FEATURE_FLAG.hasLogsPageAccess]: true };
91+
expect(mergeOrgFeatureFlags(globalFlags, raw)).toEqual(globalFlags);
92+
});
93+
});
94+
6795
describe("ORG_LOCKED_FLAG_EFFECT", () => {
6896
it("classifies every org-locked flag and nothing else", () => {
6997
expect(Object.keys(ORG_LOCKED_FLAG_EFFECT).sort()).toEqual([...ORG_LOCKED_FLAGS].sort());

0 commit comments

Comments
 (0)