mirror of
https://github.com/Unleash/unleash.git
synced 2025-11-24 20:06:55 +01:00
fix: allow external flag resolver to override false experiments with variants in getAll (#10966)
Fixes a bug / uncovered edge case in the flag resolver in Unleash: If a local experiment was defined as false (the typical default value), then that flag could only ever be returned as a boolean from the `ui-config` endpoint. In other words, even if the external resolver has a variant for that flag, the UI would never get the variant. The fix is to not just check `isEnabled` for false flags, but instead: - use `getVariant` - then check `variant.enabled` (in which case we have a variant and can return it) - else check `variant.feature_enabled`, falling back to `isEnabled` only if `feature_enabled` is null/undefined.
This commit is contained in:
parent
6732278e65
commit
89a3578826
@ -29,12 +29,11 @@ test('should produce UI flags with extra dynamic flags', () => {
|
||||
|
||||
test('should use external resolver for dynamic flags', () => {
|
||||
const externalResolver = {
|
||||
isEnabled: (name: string) => {
|
||||
if (name === 'extraFlag') {
|
||||
return true;
|
||||
}
|
||||
},
|
||||
getVariant: () => defaultVariant,
|
||||
isEnabled: (name: string) => name === 'extraFlag',
|
||||
getVariant: (name: string) => ({
|
||||
...defaultVariant,
|
||||
feature_enabled: name === 'extraFlag',
|
||||
}),
|
||||
getStaticContext: () => ({}),
|
||||
};
|
||||
|
||||
@ -225,6 +224,53 @@ test('should call external resolver getVariant when not overridden to be true, e
|
||||
);
|
||||
});
|
||||
|
||||
test('should allow overriding false experiments with externally resolved variants when getting all flags (getAll)', () => {
|
||||
const variant = {
|
||||
enabled: true,
|
||||
name: 'variant',
|
||||
};
|
||||
|
||||
const externalResolver = {
|
||||
isEnabled: () => false,
|
||||
getVariant: () => variant,
|
||||
getStaticContext: () => ({}),
|
||||
};
|
||||
|
||||
const config = {
|
||||
flags: { willStayBool: true, willBeVariant: false },
|
||||
externalResolver,
|
||||
};
|
||||
|
||||
const resolver = new FlagResolver(config as IExperimentalOptions);
|
||||
const flags = resolver.getAll() as typeof config.flags;
|
||||
|
||||
expect(flags.willStayBool).toStrictEqual(true);
|
||||
expect(flags.willBeVariant).toStrictEqual(variant);
|
||||
});
|
||||
|
||||
test('should fall back to isEnabled if variant.feature_enabled is not defined in getAll', () => {
|
||||
const variant = {
|
||||
enabled: false,
|
||||
name: 'variant',
|
||||
};
|
||||
|
||||
const externalResolver = {
|
||||
isEnabled: () => true,
|
||||
getVariant: () => variant,
|
||||
getStaticContext: () => ({}),
|
||||
};
|
||||
|
||||
const config = {
|
||||
flags: { flag: false },
|
||||
externalResolver,
|
||||
};
|
||||
|
||||
const resolver = new FlagResolver(config as IExperimentalOptions);
|
||||
const flags = resolver.getAll() as typeof config.flags;
|
||||
|
||||
expect(flags.flag).toStrictEqual(true);
|
||||
});
|
||||
|
||||
test('should call external resolver getStaticContext ', () => {
|
||||
const variant = {
|
||||
enabled: true,
|
||||
|
||||
@ -27,10 +27,17 @@ export default class FlagResolver implements IFlagResolver {
|
||||
const flag = flags[flagName];
|
||||
if (typeof flag === 'boolean') {
|
||||
if (!flag) {
|
||||
flags[flagName] = this.externalResolver.isEnabled(
|
||||
const variant = this.externalResolver.getVariant(
|
||||
flagName,
|
||||
context,
|
||||
);
|
||||
if (variant.enabled) {
|
||||
flags[flagName] = variant;
|
||||
} else {
|
||||
flags[flagName] =
|
||||
variant.feature_enabled ??
|
||||
this.externalResolver.isEnabled(flagName, context);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!flag?.enabled) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user