diff --git a/flagsmith-core.ts b/flagsmith-core.ts index fc5fce6..ab5ffe7 100644 --- a/flagsmith-core.ts +++ b/flagsmith-core.ts @@ -704,7 +704,7 @@ const Flagsmith = class { } if (!options?.skipAnalytics && !skipAnalytics) { - this.evaluateFlag(key, "VALUE"); + this.evaluateFlag(key, "VALUE", flag); } if (res === null && typeof options?.fallback !== 'undefined') { @@ -829,13 +829,13 @@ const Flagsmith = class { res = true; } if ((usingNewOptions && !options.skipAnalytics) || !options) { - this.evaluateFlag(key, "ENABLED"); + this.evaluateFlag(key, "ENABLED", flag); } if(this.sentryClient) { try { this.sentryClient.getIntegrationByName( "FeatureFlags", - )?.addFeatureFlag?.(key, res); + )?.addFeatureFlag?.(flag?.name || normalizeFlagKey(key), res); } catch (e) { console.error(e) } @@ -950,15 +950,20 @@ const Flagsmith = class { } } - private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED') => { + private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED', flag?: IFlagsmithFeature | null) => { + // The API resolves analytics by exact feature name, so always report the + // feature's own name rather than whatever casing the caller happened to use. + // Without this, getValue('My Flag') and getValue('my_flag') report as two + // separate features and neither resolves unless it matches the name exactly. + const analyticsKey = flag?.name || normalizeFlagKey(key); if (this.datadogRum) { if (!this.datadogRum!.client!.addFeatureFlagEvaluation) { console.error('Flagsmith: Your datadog RUM client does not support the function addFeatureFlagEvaluation, please update it.'); } else { if (method === 'VALUE') { - this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + key, this.getValue(key, {}, true)); + this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + analyticsKey, this.getValue(key, {}, true)); } else { - this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + key, this.hasFeature(key, true)); + this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + analyticsKey, this.hasFeature(key, true)); } } } @@ -968,10 +973,10 @@ const Flagsmith = class { if (!this.evaluationEvent[this.evaluationContext.environment.apiKey]) { this.evaluationEvent[this.evaluationContext.environment.apiKey] = {}; } - if (this.evaluationEvent[this.evaluationContext.environment.apiKey][key] === undefined) { - this.evaluationEvent[this.evaluationContext.environment.apiKey][key] = 0; + if (this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] === undefined) { + this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] = 0; } - this.evaluationEvent[this.evaluationContext.environment.apiKey][key] += 1; + this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] += 1; } this.updateEventStorage(); @@ -1010,8 +1015,11 @@ const Flagsmith = class { this.log(`Flagsmith: trackExposureEvent called for "${featureName}" without an identity; call identify() (optionally with transient: true) or pass opts.identifier. No exposure recorded.`); return; } + // As with evaluateFlag, report the feature's own name so that exposures for the + // same flag aggregate regardless of the casing the caller used. + const key = normalizeFlagKey(featureName); this.eventProcessor.trackExposureEvent({ - featureName, + featureName: this.flags?.[key]?.name || key, identifier, value: opts?.value ?? null, traits: resolveTraitValues(opts?.traits ?? this.evaluationContext.identity?.traits), diff --git a/test/feature-name-casing.test.ts b/test/feature-name-casing.test.ts index d55eade..5be6919 100644 --- a/test/feature-name-casing.test.ts +++ b/test/feature-name-casing.test.ts @@ -1,5 +1,5 @@ import { IInitConfig } from '../types'; -import { getFlagsmith } from './test-constants'; +import { delay, getFlagsmith } from './test-constants'; const mixedCaseFlags = [ { @@ -14,6 +14,14 @@ const mixedCaseFlags = [ }, ]; +const instances: any[] = []; + +afterEach(() => { + // enableAnalytics starts an interval that nothing public tears down. + instances.forEach((flagsmith) => clearInterval(flagsmith.analyticsInterval)); + instances.length = 0; +}); + async function initWithMixedCaseFlags(config: Partial = {}) { const { flagsmith, initConfig, mockFetch } = getFlagsmith(config); mockFetch.mockImplementation(async (url: string) => { @@ -26,9 +34,15 @@ async function initWithMixedCaseFlags(config: Partial = {}) { throw new Error('Please mock the call to ' + url); }); await flagsmith.init(initConfig); + instances.push(flagsmith); return { flagsmith, mockFetch }; } +const postedAnalytics = (mockFetch: jest.Mock) => { + const call = mockFetch.mock.calls.find(([url]: [string]) => url.includes('analytics/flags')); + return call ? JSON.parse(call[1].body) : null; +}; + describe('feature name casing', () => { test('exposes the original feature name on each flag', async () => { const { flagsmith } = await initWithMixedCaseFlags(); @@ -44,4 +58,29 @@ describe('feature name casing', () => { expect(flagsmith.getValue('myfeatureflag')).toBe('on'); expect(flagsmith.hasFeature('MYFEATUREFLAG')).toBe(true); }); + + test('records analytics against the canonical feature name, whatever casing the caller used', async () => { + const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true }); + await delay(1); // evaluationEvent is restored from storage asynchronously + + flagsmith.getValue('myfeatureflag'); + flagsmith.getValue('MyFeatureFlag'); + flagsmith.hasFeature('MYFEATUREFLAG'); + + // @ts-ignore internal, normally driven by an interval + await flagsmith.analyticsFlags(); + expect(postedAnalytics(mockFetch)).toEqual({ MyFeatureFlag: 3 }); + }); + + test('buckets unknown flags under the normalised key rather than the caller casing', async () => { + const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true }); + await delay(1); + + flagsmith.getValue('Does Not Exist'); + flagsmith.getValue('does_not_exist'); + + // @ts-ignore internal, normally driven by an interval + await flagsmith.analyticsFlags(); + expect(postedAnalytics(mockFetch)).toEqual({ does_not_exist: 2 }); + }); });