fix: skip-segment-if-environment-flag-and-moved-flagsmith-id-to-id

Zaimwa9 · Zaimwa9 · commit a5ad4f8d52c7 · 2025-10-30T15:48:49.000+01:00
diff --git a/flagsmith-engine/evaluation/evaluationContext/mappers.ts b/flagsmith-engine/evaluation/evaluationContext/mappers.ts
@@ -5,7 +5,10 @@ import {
     GenericEvaluationContext,
     EnvironmentContext,
     IdentityContext,
-    SegmentSource
+    SegmentSource,
+    CustomFeatureMetadata,
+    SegmentsWithMetadata,
+    CustomSegmentMetadata
 } from '../models.js';
 import { EnvironmentModel } from '../../environments/models.js';
 import { IdentityModel } from '../../identities/models.js';
@@ -17,9 +20,13 @@ import { uuidToBigInt } from '../../features/util.js';
 export function getEvaluationContext(
     environment: EnvironmentModel,
     identity?: IdentityModel,
-    overrideTraits?: TraitModel[]
+    overrideTraits?: TraitModel[],
+    isEnvironmentEvaluation: boolean = false
 ): GenericEvaluationContext {
     const environmentContext = mapEnvironmentModelToEvaluationContext(environment);
+    if (isEnvironmentEvaluation) {
+        return environmentContext;
+    }
     const identityContext = identity
         ? mapIdentityModelToIdentityContext(identity, overrideTraits)
         : undefined;
@@ -40,7 +47,7 @@ function mapEnvironmentModelToEvaluationContext(
         name: environment.project.name
     };
 
-    const features: FeaturesWithMetadata = {};
+    const features: FeaturesWithMetadata<CustomFeatureMetadata> = {};
     for (const fs of environment.featureStates) {
         const variants =
             fs.multivariateFeatureStateValues?.length > 0
@@ -59,12 +66,12 @@ function mapEnvironmentModelToEvaluationContext(
             variants,
             priority: fs.featureSegment?.priority,
             metadata: {
-                flagsmithId: fs.feature.id
+                id: fs.feature.id
             }
         };
     }
 
-    const segmentOverrides: Segments = {};
+    const segmentOverrides: SegmentsWithMetadata<CustomSegmentMetadata> = {};
     for (const segment of environment.project.segments) {
         segmentOverrides[segment.id.toString()] = {
             key: segment.id.toString(),
@@ -79,18 +86,18 @@ function mapEnvironmentModelToEvaluationContext(
                           value: fs.getValue(),
                           priority: fs.featureSegment?.priority,
                           metadata: {
-                              flagsmithId: fs.feature.id
+                              id: fs.feature.id
                           }
                       }))
                     : [],
             metadata: {
                 source: SegmentSource.API,
-                flagsmithId: segment.id
+                id: segment.id
             }
         };
     }
 
-    let identityOverrideSegments: Segments = {};
+    let identityOverrideSegments: SegmentsWithMetadata<CustomSegmentMetadata> = {};
     if (environment.identityOverrides && environment.identityOverrides.length > 0) {
         identityOverrideSegments = mapIdentityOverridesToSegments(environment.identityOverrides);
     }
@@ -140,8 +147,10 @@ function mapSegmentRuleModelToRule(rule: any): any {
     };
 }
 
-function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Segments {
-    const segments: Segments = {};
+function mapIdentityOverridesToSegments(
+    identityOverrides: IdentityModel[]
+): SegmentsWithMetadata<CustomSegmentMetadata> {
+    const segments: SegmentsWithMetadata<CustomSegmentMetadata> = {};
     const featuresToIdentifiers = new Map<string, { identifiers: string[]; overrides: any[] }>();
 
     for (const identity of identityOverrides) {
@@ -158,7 +167,7 @@ function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Seg
             value: fs.getValue(),
             priority: -Infinity,
             metadata: {
-                flagsmithId: fs.feature.id
+                id: fs.feature.id
             }
         }));
 
diff --git a/flagsmith-engine/evaluation/models.ts b/flagsmith-engine/evaluation/models.ts
@@ -25,7 +25,7 @@ export enum SegmentSource {
 
 // Feature types
 export interface CustomFeatureMetadata extends FeatureMetadata {
-    flagsmithId: number;
+    id: number;
 }
 
 export interface FeatureContextWithMetadata<T extends FeatureMetadata = FeatureMetadata>
@@ -49,7 +49,7 @@ export type EvaluationResultFlags<T extends FeatureMetadata = FeatureMetadata> =
 
 // Segment types
 export interface CustomSegmentMetadata extends SegmentMetadata {
-    flagsmithId: number;
+    id?: number;
     source?: SegmentSource;
 }
 
@@ -68,7 +68,7 @@ export interface SegmentResultWithMetadata {
     metadata: CustomSegmentMetadata;
 }
 
-export type EvaluationResultSegments = EvaluationContextResult['segments'];
+export type EvaluationResultSegments = SegmentResultWithMetadata[];
 
 // Evaluation context types
 export interface GenericEvaluationContext<
diff --git a/flagsmith-engine/segments/models.ts b/flagsmith-engine/segments/models.ts
@@ -227,13 +227,13 @@ export class SegmentModel {
             if (segmentResult.metadata?.source === SegmentSource.IDENTITY_OVERRIDE) {
                 continue;
             }
-            const flagsmithId = segmentResult.metadata?.flagsmithId;
-            if (!flagsmithId) {
+            const segmentMetadataId = segmentResult.metadata?.id;
+            if (!segmentMetadataId) {
                 continue;
             }
-            const segmentContext = evaluationContext.segments[flagsmithId.toString()];
+            const segmentContext = evaluationContext.segments[segmentMetadataId.toString()];
             if (segmentContext) {
-                const segment = new SegmentModel(flagsmithId, segmentContext.name);
+                const segment = new SegmentModel(segmentMetadataId, segmentContext.name);
                 segment.rules = segmentContext.rules.map(rule => new SegmentRuleModel(rule.type));
                 segment.featureStates = SegmentModel.createFeatureStatesFromOverrides(
                     segmentContext.overrides || []
@@ -249,13 +249,13 @@ export class SegmentModel {
         if (!overrides) return [];
         return overrides
             .filter(override => {
-                const flagsmithId = override?.metadata?.flagsmithId;
-                return typeof flagsmithId === 'number';
+                const overrideMetadataId = override?.metadata?.id;
+                return typeof overrideMetadataId === 'number';
             })
             .map(override => {
-                const flagsmithId = override.metadata!.flagsmithId as number;
+                const overrideMetadataId = override.metadata!.id as number;
                 const feature = new FeatureModel(
-                    flagsmithId,
+                    overrideMetadataId,
                     override.name,
                     override.variants?.length && override.variants.length > 0
                         ? CONSTANTS.MULTIVARIATE
diff --git a/sdk/index.ts b/sdk/index.ts
@@ -25,6 +25,7 @@ import {
 } from './types.js';
 import { pino, Logger } from 'pino';
 import { getEvaluationContext } from '../flagsmith-engine/evaluation/evaluationContext/mappers.js';
+import { EvaluationContextWithMetadata } from '../flagsmith-engine/evaluation/models.js';
 
 export { AnalyticsProcessor, AnalyticsProcessorOptions } from './analytics.js';
 export { FlagsmithAPIError, FlagsmithClientError } from './errors.js';
@@ -282,7 +283,7 @@ export class Flagsmith {
         if (!context) {
             throw new FlagsmithClientError('Local evaluation required to obtain identity segments');
         }
-        const evaluationResult = getEvaluationResult(context);
+        const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
 
         return SegmentModel.fromSegmentResult(evaluationResult.segments, context);
     }
@@ -449,11 +450,11 @@ export class Flagsmith {
 
     private async getEnvironmentFlagsFromDocument(): Promise<Flags> {
         const environment = await this.getEnvironment();
-        const context = getEvaluationContext(environment);
+        const context = getEvaluationContext(environment, undefined, undefined, true);
         if (!context) {
             throw new FlagsmithClientError('Unable to get flags. No environment present.');
         }
-        const evaluationResult = getEvaluationResult(context);
+        const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
         const flags = Flags.fromEvaluationResult(evaluationResult);
 
         if (!!this.cache) {
@@ -481,7 +482,7 @@ export class Flagsmith {
         if (!context) {
             throw new FlagsmithClientError('Unable to get flags. No environment present.');
         }
-        const evaluationResult = getEvaluationResult(context);
+        const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
 
         const flags = Flags.fromEvaluationResult(
             evaluationResult,
diff --git a/sdk/models.ts b/sdk/models.ts
@@ -118,18 +118,18 @@ export class Flags {
     ): Flags {
         const flags: { [key: string]: Flag } = {};
         for (const flag of Object.values(evaluationResult.flags)) {
-            const flagsmithId = flag.metadata?.flagsmithId;
-            if (!flagsmithId) {
+            const flagMetadataId = flag.metadata?.id;
+            if (!flagMetadataId) {
                 throw new Error(
-                    `FlagResult metadata.flagsmithId is missing for feature "${flag.name}". ` +
+                    `FlagResult metadata.id is missing for feature "${flag.name}". ` +
                         `This indicates a bug in the SDK, please report it.`
                 );
             }
 
             flags[flag.name] = new Flag({
                 enabled: flag.enabled,
                 value: flag.value ?? null,
-                featureId: flagsmithId,
+                featureId: flagMetadataId,
                 featureName: flag.name,
                 reason: flag.reason
             });
diff --git a/tests/engine/unit/engine.test.ts b/tests/engine/unit/engine.test.ts
@@ -115,7 +115,7 @@ test('shouldApplyOverride with priority conflicts', () => {
                 enabled: true,
                 value: 'value',
                 priority: 5,
-                metadata: { flagsmithId: 1 }
+                metadata: { id: 1 }
             },
             segmentName: 'segment1'
         }
@@ -185,7 +185,7 @@ test('evaluateSegments handles segments with identity identifier matching', () =
                 name: 'feature1',
                 enabled: false,
                 value: 'default_value',
-                metadata: { flagsmithId: 1 }
+                metadata: { id: 1 }
             }
         }
     };
@@ -272,7 +272,7 @@ test('evaluateSegments handles priority conflicts correctly', () => {
                 name: 'feature1',
                 enabled: false,
                 value: 'default_value',
-                metadata: { flagsmithId: 1 }
+                metadata: { id: 1 }
             }
         }
     };
@@ -343,7 +343,7 @@ test('evaluateFeatures with multivariate evaluation', () => {
                     { value: 'variant_a', weight: 0, priority: 1 },
                     { value: 'variant_b', weight: 100, priority: 2 }
                 ],
-                metadata: { flagsmithId: 1 }
+                metadata: { id: 1 }
             }
         },
         identity: { key: 'test_user', identifier: 'test_user' },
diff --git a/tests/engine/unit/segments/segments_model.test.ts b/tests/engine/unit/segments/segments_model.test.ts
@@ -141,7 +141,7 @@ test('test_segment_rule_matching_function', () => {
 });
 
 test('test_fromSegmentResult_with_multiple_variants', () => {
-    const segmentResults = [{ name: 'test_segment', metadata: { flagsmithId: '1' } }];
+    const segmentResults = [{ name: 'test_segment', metadata: { id: '1' } }];
 
     const evaluationContext: EvaluationContext = {
         identity: {
@@ -159,7 +159,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => {
                 name: 'test_segment',
                 metadata: {
                     source: SegmentSource.API,
-                    flagsmithId: '1'
+                    id: '1'
                 },
                 rules: [
                     {
@@ -181,7 +181,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => {
                         value: 'default_value',
                         priority: 1,
                         metadata: {
-                            flagsmithId: 1
+                            id: 1
                         },
                         variants: [
                             { id: 1, value: 'variant_a', weight: 30 },
diff --git a/tests/sdk/flagsmith.test.ts b/tests/sdk/flagsmith.test.ts
@@ -520,7 +520,7 @@ test('get_user_agent_extracts_version_from_package_json', async () => {
     expect(userAgent).toBe(`flagsmith-nodejs-sdk/${packageJson.version}`);
 });
 
-test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missing', () => {
+test('Flags.fromEvaluationResult throws error when metadata.id is missing', () => {
     const evaluationResult = {
         flags: {
             test_feature: {
@@ -535,7 +535,7 @@ test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missi
     };
 
     expect(() => Flags.fromEvaluationResult(evaluationResult as any)).toThrow(
-        'FlagResult metadata.flagsmithId is missing for feature "test_feature". ' +
+        'FlagResult metadata.id is missing for feature "test_feature". ' +
             'This indicates a bug in the SDK, please report it.'
     );
 });

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ export enum SegmentSource {`
`25`	`25`
`26`	`26`	`// Feature types`
`27`	`27`	`export interface CustomFeatureMetadata extends FeatureMetadata {`
`28`		`- flagsmithId: number;`
	`28`	`+ id: number;`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`export interface FeatureContextWithMetadata<T extends FeatureMetadata = FeatureMetadata>`
`@@ -49,7 +49,7 @@ export type EvaluationResultFlags<T extends FeatureMetadata = FeatureMetadata> =`
`49`	`49`
`50`	`50`	`// Segment types`
`51`	`51`	`export interface CustomSegmentMetadata extends SegmentMetadata {`
`52`		`- flagsmithId: number;`
	`52`	`+ id?: number;`
`53`	`53`	`source?: SegmentSource;`
`54`	`54`	`}`
`55`	`55`
`@@ -68,7 +68,7 @@ export interface SegmentResultWithMetadata {`
`68`	`68`	`metadata: CustomSegmentMetadata;`
`69`	`69`	`}`
`70`	`70`
`71`		`-export type EvaluationResultSegments = EvaluationContextResult['segments'];`
	`71`	`+export type EvaluationResultSegments = SegmentResultWithMetadata[];`
`72`	`72`
`73`	`73`	`// Evaluation context types`
`74`	`74`	`export interface GenericEvaluationContext<`
Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,7 @@ test('shouldApplyOverride with priority conflicts', () => {`
`115`	`115`	`enabled: true,`
`116`	`116`	`value: 'value',`
`117`	`117`	`priority: 5,`
`118`		`- metadata: { flagsmithId: 1 }`
	`118`	`+ metadata: { id: 1 }`
`119`	`119`	`},`
`120`	`120`	`segmentName: 'segment1'`
`121`	`121`	`}`
`@@ -185,7 +185,7 @@ test('evaluateSegments handles segments with identity identifier matching', () =`
`185`	`185`	`name: 'feature1',`
`186`	`186`	`enabled: false,`
`187`	`187`	`value: 'default_value',`
`188`		`- metadata: { flagsmithId: 1 }`
	`188`	`+ metadata: { id: 1 }`
`189`	`189`	`}`
`190`	`190`	`}`
`191`	`191`	`};`
`@@ -272,7 +272,7 @@ test('evaluateSegments handles priority conflicts correctly', () => {`
`272`	`272`	`name: 'feature1',`
`273`	`273`	`enabled: false,`
`274`	`274`	`value: 'default_value',`
`275`		`- metadata: { flagsmithId: 1 }`
	`275`	`+ metadata: { id: 1 }`
`276`	`276`	`}`
`277`	`277`	`}`
`278`	`278`	`};`
`@@ -343,7 +343,7 @@ test('evaluateFeatures with multivariate evaluation', () => {`
`343`	`343`	`{ value: 'variant_a', weight: 0, priority: 1 },`
`344`	`344`	`{ value: 'variant_b', weight: 100, priority: 2 }`
`345`	`345`	`],`
`346`		`- metadata: { flagsmithId: 1 }`
	`346`	`+ metadata: { id: 1 }`
`347`	`347`	`}`
`348`	`348`	`},`
`349`	`349`	`identity: { key: 'test_user', identifier: 'test_user' },`