diff --git a/packages/remote-feature-flag-controller/CHANGELOG.md b/packages/remote-feature-flag-controller/CHANGELOG.md index 35fd65a9d04..2ce5dff6a70 100644 --- a/packages/remote-feature-flag-controller/CHANGELOG.md +++ b/packages/remote-feature-flag-controller/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add override functionality to remote feature flags ([#7271](https://github.com/MetaMask/core/pull/7271)) + - `setFlagOverride(flagName, value)` - Set a local override for a specific feature flag + - `clearFlagOverride(flagName)` - Clear the local override for a specific feature flag + - `clearAllOverrides()` - Clear all local feature flag overrides +- Add new controller state properties ([#7271](https://github.com/MetaMask/core/pull/7271)) + - `localOverrides` - Local overrides for feature flags that take precedence over remote flags + - `rawRemoteFeatureFlags` - Raw flag value for arrays that were processed from arrays to single value +- Export additional controller action types ([#7271](https://github.com/MetaMask/core/pull/7271)) + - `RemoteFeatureFlagControllerSetFlagOverrideAction` + - `RemoteFeatureFlagControllerClearFlagOverrideAction` + - `RemoteFeatureFlagControllerClearAllOverridesAction` + +### Changed + +- Enhanced feature flag processing to preserve raw A/B test arrays for visibility and override capabilities ([#7271](https://github.com/MetaMask/core/pull/7271)) +- Local overrides now take precedence over remote feature flags when both exist ([#7271](https://github.com/MetaMask/core/pull/7271)) +- Controller state now includes metadata for new properties with appropriate persistence and logging settings ([#7271](https://github.com/MetaMask/core/pull/7271)) + ## [3.0.0] ### Added diff --git a/packages/remote-feature-flag-controller/src/index.ts b/packages/remote-feature-flag-controller/src/index.ts index 9906d3bc04d..ec621bf55ef 100644 --- a/packages/remote-feature-flag-controller/src/index.ts +++ b/packages/remote-feature-flag-controller/src/index.ts @@ -5,6 +5,9 @@ export type { RemoteFeatureFlagControllerActions, RemoteFeatureFlagControllerGetStateAction, RemoteFeatureFlagControllerUpdateRemoteFeatureFlagsAction, + RemoteFeatureFlagControllerSetFlagOverrideAction, + RemoteFeatureFlagControllerClearFlagOverrideAction, + RemoteFeatureFlagControllerClearAllOverridesAction, RemoteFeatureFlagControllerEvents, RemoteFeatureFlagControllerStateChangeEvent, } from './remote-feature-flag-controller'; diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller-types.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller-types.ts index 6bf8e5ea2fc..119c71a7cf7 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller-types.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller-types.ts @@ -51,3 +51,25 @@ export type ServiceResponse = { remoteFeatureFlags: FeatureFlags; cacheTimestamp: number | null; }; + +/** + * Describes the shape of the state object for the {@link RemoteFeatureFlagController}. + */ +export type RemoteFeatureFlagControllerState = { + /** + * The collection of feature flags and their respective values, which can be objects. + */ + remoteFeatureFlags: FeatureFlags; + /** + * Local overrides for feature flags that take precedence over remote flags. + */ + localOverrides: FeatureFlags; + /** + * Raw A/B test flag arrays for flags that were processed from arrays to single values. + */ + rawRemoteFeatureFlags: FeatureFlags; + /** + * The timestamp of the last successful feature flag cache. + */ + cacheTimestamp: number; +}; diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts index 4ac1f584ad0..fd63729f33e 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts @@ -9,6 +9,7 @@ import type { import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import { RemoteFeatureFlagController, + controllerName, DEFAULT_CACHE_DURATION, getDefaultRemoteFeatureFlagControllerState, } from './remote-feature-flag-controller'; @@ -18,8 +19,6 @@ import type { } from './remote-feature-flag-controller'; import type { FeatureFlags } from './remote-feature-flag-controller-types'; -const controllerName = 'RemoteFeatureFlagController'; - const MOCK_FLAGS: FeatureFlags = { feature1: true, feature2: { chrome: '<109' }, @@ -88,6 +87,8 @@ describe('RemoteFeatureFlagController', () => { expect(controller.state).toStrictEqual({ remoteFeatureFlags: {}, + localOverrides: {}, + rawRemoteFeatureFlags: {}, cacheTimestamp: 0, }); }); @@ -97,6 +98,8 @@ describe('RemoteFeatureFlagController', () => { expect(controller.state).toStrictEqual({ remoteFeatureFlags: {}, + localOverrides: {}, + rawRemoteFeatureFlags: {}, cacheTimestamp: 0, }); }); @@ -105,6 +108,8 @@ describe('RemoteFeatureFlagController', () => { const customState = { remoteFeatureFlags: MOCK_FLAGS_TWO, cacheTimestamp: 123456789, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }; const controller = createController({ state: customState }); @@ -640,11 +645,154 @@ describe('RemoteFeatureFlagController', () => { it('should return default state', () => { expect(getDefaultRemoteFeatureFlagControllerState()).toStrictEqual({ remoteFeatureFlags: {}, + localOverrides: {}, + rawRemoteFeatureFlags: {}, cacheTimestamp: 0, }); }); }); + describe('override functionality', () => { + describe('setFlagOverride', () => { + it('sets a local override for a feature flag', () => { + const controller = createController(); + + controller.setFlagOverride('testFlag', true); + + expect(controller.state.localOverrides).toStrictEqual({ + testFlag: true, + }); + }); + + it('overwrites existing override for the same flag', () => { + const controller = createController(); + + controller.setFlagOverride('testFlag', true); + controller.setFlagOverride('testFlag', false); + + expect(controller.state.localOverrides).toStrictEqual({ + testFlag: false, + }); + }); + + it('preserves other overrides when setting a new one', () => { + const controller = createController(); + + controller.setFlagOverride('flag1', 'value1'); + controller.setFlagOverride('flag2', 'value2'); + + expect(controller.state.localOverrides).toStrictEqual({ + flag1: 'value1', + flag2: 'value2', + }); + }); + }); + + describe('clearFlagOverride', () => { + it('removes a specific override', () => { + const controller = createController(); + + controller.setFlagOverride('flag1', 'value1'); + controller.setFlagOverride('flag2', 'value2'); + controller.clearFlagOverride('flag1'); + + expect(controller.state.localOverrides).toStrictEqual({ + flag2: 'value2', + }); + }); + + it('does not affect state when clearing non-existent override', () => { + const controller = createController(); + + controller.setFlagOverride('flag1', 'value1'); + controller.clearFlagOverride('nonExistentFlag'); + + expect(controller.state.localOverrides).toStrictEqual({ + flag1: 'value1', + }); + }); + }); + + describe('clearAllOverrides', () => { + it('removes all overrides', () => { + const controller = createController(); + + controller.setFlagOverride('flag1', 'value1'); + controller.setFlagOverride('flag2', 'value2'); + controller.clearAllOverrides(); + + expect(controller.state.localOverrides).toStrictEqual({}); + }); + + it('does not affect state when no overrides exist', () => { + const controller = createController(); + + controller.clearAllOverrides(); + + expect(controller.state.localOverrides).toStrictEqual({}); + }); + }); + + describe('integration with remote flags', () => { + it('preserves overrides when remote flags are updated', async () => { + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: { remoteFlag: 'initialRemoteValue' }, + }); + const controller = createController({ clientConfigApiService }); + + // Set override before fetching remote flags + controller.setFlagOverride('overrideFlag', 'overrideValue'); + + await controller.updateRemoteFeatureFlags(); + + // Mock different remote response for second fetch + jest + .spyOn(clientConfigApiService, 'fetchRemoteFeatureFlags') + .mockResolvedValueOnce({ + remoteFeatureFlags: { remoteFlag: 'updatedRemoteValue' }, + cacheTimestamp: Date.now(), + }); + + // Force cache expiration and update + jest + .spyOn(Date, 'now') + .mockReturnValue(Date.now() + DEFAULT_CACHE_DURATION + 1000); + await controller.updateRemoteFeatureFlags(); + + // Overrides should be preserved + expect(controller.state.localOverrides).toStrictEqual({ + overrideFlag: 'overrideValue', + }); + + // Remote flags should be updated + expect(controller.state.remoteFeatureFlags).toStrictEqual({ + remoteFlag: 'updatedRemoteValue', + }); + }); + + it('overrides work with threshold-based feature flags', async () => { + const clientConfigApiService = buildClientConfigApiService({ + remoteFeatureFlags: MOCK_FLAGS_WITH_THRESHOLD, + }); + const controller = createController({ + clientConfigApiService, + getMetaMetricsId: () => MOCK_METRICS_ID, + }); + + await controller.updateRemoteFeatureFlags(); + + // Override the threshold flag + controller.setFlagOverride('testFlagForThreshold', 'overriddenValue'); + + // Access flag value with override taking precedence + const flagValue = + controller.state.localOverrides.testFlagForThreshold ?? + controller.state.remoteFeatureFlags.testFlagForThreshold; + expect(flagValue).toBe('overriddenValue'); + }); + }); + }); + describe('metadata', () => { it('includes expected state in debug snapshots', () => { const controller = createController(); @@ -658,6 +806,8 @@ describe('RemoteFeatureFlagController', () => { ).toMatchInlineSnapshot(` Object { "cacheTimestamp": 0, + "localOverrides": Object {}, + "rawRemoteFeatureFlags": Object {}, "remoteFeatureFlags": Object {}, } `); @@ -675,6 +825,8 @@ describe('RemoteFeatureFlagController', () => { ).toMatchInlineSnapshot(` Object { "cacheTimestamp": 0, + "localOverrides": Object {}, + "rawRemoteFeatureFlags": Object {}, "remoteFeatureFlags": Object {}, } `); @@ -692,6 +844,8 @@ describe('RemoteFeatureFlagController', () => { ).toMatchInlineSnapshot(` Object { "cacheTimestamp": 0, + "localOverrides": Object {}, + "rawRemoteFeatureFlags": Object {}, "remoteFeatureFlags": Object {}, } `); @@ -708,6 +862,7 @@ describe('RemoteFeatureFlagController', () => { ), ).toMatchInlineSnapshot(` Object { + "localOverrides": Object {}, "remoteFeatureFlags": Object {}, } `); diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts index b1fee7ac67c..338ef838c74 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts @@ -21,13 +21,15 @@ import { isVersionFeatureFlag, getVersionData } from './utils/version'; // === GENERAL === -const controllerName = 'RemoteFeatureFlagController'; +export const controllerName = 'RemoteFeatureFlagController'; export const DEFAULT_CACHE_DURATION = 24 * 60 * 60 * 1000; // 1 day // === STATE === export type RemoteFeatureFlagControllerState = { remoteFeatureFlags: FeatureFlags; + localOverrides: FeatureFlags; + rawRemoteFeatureFlags: FeatureFlags; cacheTimestamp: number; }; @@ -38,6 +40,18 @@ const remoteFeatureFlagControllerMetadata = { includeInDebugSnapshot: true, usedInUi: true, }, + localOverrides: { + includeInStateLogs: true, + persist: true, + includeInDebugSnapshot: true, + usedInUi: true, + }, + rawRemoteFeatureFlags: { + includeInStateLogs: true, + persist: true, + includeInDebugSnapshot: true, + usedInUi: false, + }, cacheTimestamp: { includeInStateLogs: true, persist: true, @@ -62,9 +76,27 @@ export type RemoteFeatureFlagControllerUpdateRemoteFeatureFlagsAction = { handler: RemoteFeatureFlagController['updateRemoteFeatureFlags']; }; +export type RemoteFeatureFlagControllerSetFlagOverrideAction = { + type: `${typeof controllerName}:setFlagOverride`; + handler: RemoteFeatureFlagController['setFlagOverride']; +}; + +export type RemoteFeatureFlagControllerClearFlagOverrideAction = { + type: `${typeof controllerName}:clearFlagOverride`; + handler: RemoteFeatureFlagController['clearFlagOverride']; +}; + +export type RemoteFeatureFlagControllerClearAllOverridesAction = { + type: `${typeof controllerName}:clearAllOverrides`; + handler: RemoteFeatureFlagController['clearAllOverrides']; +}; + export type RemoteFeatureFlagControllerActions = | RemoteFeatureFlagControllerGetStateAction - | RemoteFeatureFlagControllerUpdateRemoteFeatureFlagsAction; + | RemoteFeatureFlagControllerUpdateRemoteFeatureFlagsAction + | RemoteFeatureFlagControllerSetFlagOverrideAction + | RemoteFeatureFlagControllerClearFlagOverrideAction + | RemoteFeatureFlagControllerClearAllOverridesAction; export type RemoteFeatureFlagControllerStateChangeEvent = ControllerStateChangeEvent< @@ -89,6 +121,8 @@ export type RemoteFeatureFlagControllerMessenger = Messenger< export function getDefaultRemoteFeatureFlagControllerState(): RemoteFeatureFlagControllerState { return { remoteFeatureFlags: {}, + localOverrides: {}, + rawRemoteFeatureFlags: {}, cacheTimestamp: 0, }; } @@ -218,6 +252,8 @@ export class RemoteFeatureFlagController extends BaseController< this.update(() => { return { remoteFeatureFlags: processedRemoteFeatureFlags, + localOverrides: this.state.localOverrides, + rawRemoteFeatureFlags: remoteFeatureFlags, cacheTimestamp: Date.now(), }; }); @@ -291,4 +327,50 @@ export class RemoteFeatureFlagController extends BaseController< disable(): void { this.#disabled = true; } + + /** + * Sets a local override for a specific feature flag. + * + * @param flagName - The name of the feature flag to override. + * @param value - The override value for the feature flag. + */ + setFlagOverride(flagName: string, value: Json): void { + this.update(() => { + return { + ...this.state, + localOverrides: { + ...this.state.localOverrides, + [flagName]: value, + }, + }; + }); + } + + /** + * Clears the local override for a specific feature flag. + * + * @param flagName - The name of the feature flag to clear. + */ + clearFlagOverride(flagName: string): void { + const newOverrides = { ...this.state.localOverrides }; + delete newOverrides[flagName]; + this.update(() => { + return { + ...this.state, + localOverrides: newOverrides, + }; + }); + } + + /** + * Clears all local feature flag overrides. + */ + clearAllOverrides(): void { + this.update(() => { + return { + ...this.state, + localOverrides: {}, + }; + }); + } } diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 334785a8e3c..c77c85132cc 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -278,7 +278,12 @@ const setupController = async ( remoteFeatureFlagControllerMessenger.registerActionHandler( 'RemoteFeatureFlagController:getState', - () => ({ cacheTimestamp: 0, remoteFeatureFlags: {} }), + () => ({ + cacheTimestamp: 0, + remoteFeatureFlags: {}, + rawRemoteFeatureFlags: {}, + localOverrides: {}, + }), ); const options: TransactionControllerOptions = { diff --git a/packages/transaction-controller/src/utils/feature-flags.test.ts b/packages/transaction-controller/src/utils/feature-flags.test.ts index fa39f35de50..ee03a1d0b38 100644 --- a/packages/transaction-controller/src/utils/feature-flags.test.ts +++ b/packages/transaction-controller/src/utils/feature-flags.test.ts @@ -74,6 +74,8 @@ describe('Feature Flags Utils', () => { getFeatureFlagsMock.mockReturnValue({ cacheTimestamp: 0, remoteFeatureFlags: featureFlags, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); } diff --git a/packages/transaction-pay-controller/src/strategy/bridge/bridge-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/bridge/bridge-quotes.test.ts index cc66be6101c..b4fb249f6d5 100644 --- a/packages/transaction-pay-controller/src/strategy/bridge/bridge-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/bridge/bridge-quotes.test.ts @@ -130,6 +130,8 @@ describe('Bridge Quotes Utils', () => { remoteFeatureFlags: { confirmations_pay: getFeatureFlagsMock(), }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, })); getFeatureFlagsMock.mockReturnValue(FEATURE_FLAGS_MOCK); @@ -1019,6 +1021,8 @@ describe('Bridge Quotes Utils', () => { }, }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); const result = getBridgeRefreshInterval({ @@ -1042,6 +1046,8 @@ describe('Bridge Quotes Utils', () => { refreshRate: 456000, }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); const result = getBridgeRefreshInterval({ @@ -1064,6 +1070,8 @@ describe('Bridge Quotes Utils', () => { }, }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); const result = getBridgeRefreshInterval({ diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts index 8f17983a1e7..4b9aac7f09f 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts @@ -194,6 +194,8 @@ describe('Relay Quotes Utils', () => { getRemoteFeatureFlagControllerStateMock.mockReturnValue({ cacheTimestamp: 0, remoteFeatureFlags: {}, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); getGasBufferMock.mockReturnValue(1.0); @@ -571,6 +573,8 @@ describe('Relay Quotes Utils', () => { relayQuoteUrl, }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); await getRelayQuotes({ @@ -931,6 +935,8 @@ describe('Relay Quotes Utils', () => { relayDisabledGasStationChains: [QUOTE_REQUEST_MOCK.sourceChainId], }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); const result = await getRelayQuotes({ diff --git a/packages/transaction-pay-controller/src/utils/feature-flags.test.ts b/packages/transaction-pay-controller/src/utils/feature-flags.test.ts index 31e789d8e78..e5713b393ec 100644 --- a/packages/transaction-pay-controller/src/utils/feature-flags.test.ts +++ b/packages/transaction-pay-controller/src/utils/feature-flags.test.ts @@ -29,6 +29,8 @@ describe('Feature Flags Utils', () => { getRemoteFeatureFlagControllerStateMock.mockReturnValue({ cacheTimestamp: 0, remoteFeatureFlags: {}, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); }); @@ -62,6 +64,8 @@ describe('Feature Flags Utils', () => { slippage: SLIPPAGE_MOCK, }, }, + rawRemoteFeatureFlags: {}, + localOverrides: {}, }); const featureFlags = getFeatureFlags(messenger); @@ -95,6 +99,8 @@ describe('Feature Flags Utils', () => { }, }, }, + localOverrides: {}, + rawRemoteFeatureFlags: {}, }); const gasBuffer = getGasBuffer(messenger, CHAIN_ID_MOCK); @@ -118,6 +124,8 @@ describe('Feature Flags Utils', () => { }, }, }, + localOverrides: {}, + rawRemoteFeatureFlags: {}, }); const gasBuffer = getGasBuffer(messenger, CHAIN_ID_MOCK); @@ -141,6 +149,8 @@ describe('Feature Flags Utils', () => { }, }, }, + localOverrides: {}, + rawRemoteFeatureFlags: {}, }); const gasBuffer = getGasBuffer(messenger, CHAIN_ID_DIFFERENT_MOCK); @@ -163,6 +173,8 @@ describe('Feature Flags Utils', () => { }, }, }, + localOverrides: {}, + rawRemoteFeatureFlags: {}, }); const gasBuffer = getGasBuffer(messenger, CHAIN_ID_DIFFERENT_MOCK);