Skip to content

Commit 58367d3

Browse files
committed
[FSSDK-12813] Remove ticket references from code comments per cross-sdk guideline
1 parent 28ec399 commit 58367d3

3 files changed

Lines changed: 41 additions & 74 deletions

File tree

lib/event_processor/event_builder/log_event.spec.ts

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ import { Region } from '../../project_config/project_config';
2626

2727
describe('makeEventBatch', () => {
2828
it('should build a batch with single impression event when experiment and variation are defined', () => {
29-
// FSSDK-12813: campaign_id, experiment_id, variation_id, and entity_id
30-
// are normalized to numeric-strings on the wire. Test fixtures use valid
31-
// numeric IDs so the happy-path output matches expectations.
29+
// Fixtures use valid numeric IDs so the wire output matches the
30+
// post-normalization happy-path expectations.
3231
const impressionEvent: ImpressionEvent = {
3332
type: 'impression',
3433
timestamp: 69,
@@ -128,10 +127,9 @@ describe('makeEventBatch', () => {
128127
})
129128

130129
it('should build a batch with simlge impression event when experiment and variation are not defined', () => {
131-
// FSSDK-12813: When campaign_id, experiment_id, and variation_id are all
130+
// When campaign_id, experiment_id, and variation_id are all
132131
// missing/invalid, the normalized wire output is campaign_id=null,
133-
// variation_id=null, and entity_id MUST equal campaign_id byte-for-byte
134-
// (FR-009). experiment_id is left as-is.
132+
// variation_id=null, and entity_id mirrors campaign_id byte-for-byte.
135133
const impressionEvent: ImpressionEvent = {
136134
type: 'impression',
137135
timestamp: 69,
@@ -190,7 +188,6 @@ describe('makeEventBatch', () => {
190188
{
191189
campaign_id: null,
192190
experiment_id: "",
193-
// FSSDK-12813: empty/null variation_id normalizes to null.
194191
variation_id: null,
195192
metadata: {
196193
flag_key: 'flagKey1',
@@ -699,8 +696,7 @@ describe('makeEventBatch', () => {
699696
attributes: [{ entityId: 'attr1-id', key: 'attr1-key', value: 'attr1-value' }],
700697
},
701698

702-
// FSSDK-12813: Use numeric-string IDs so happy-path output is unchanged
703-
// after normalization.
699+
// Use numeric-string IDs so the happy-path wire output is unchanged.
704700
layer: {
705701
id: '11111',
706702
},
@@ -819,8 +815,8 @@ describe('makeEventBatch', () => {
819815

820816
describe('buildLogEvent', () => {
821817
it('should select the correct URL based on the event context region', () => {
822-
// FSSDK-12813: Use numeric-string IDs to avoid normalization side effects
823-
// unrelated to the URL-region behavior being tested here.
818+
// Use numeric-string IDs so normalization is a no-op here; this test
819+
// only covers URL-region behavior.
824820
const baseEvent: ImpressionEvent = {
825821
type: 'impression',
826822
timestamp: 69,
@@ -882,27 +878,24 @@ describe('buildLogEvent', () => {
882878
});
883879

884880
/**
885-
* FSSDK-12813: Decision-event ID normalization tests.
881+
* Decision-event ID normalization tests.
886882
*
887-
* These tests pin the normalization contract for decisions[].campaign_id,
883+
* Pins the normalization contract for decisions[].campaign_id,
888884
* decisions[].variation_id, and events[].entity_id on impression events:
889885
*
890-
* - campaign_id MUST be a non-empty string (any character content; IDs
891-
* may be opaque, e.g. "default-12345", "layer_abc"); if empty/null/
892-
* missing, fall back to experiment_id; if experiment_id is also empty/
893-
* null, emit null.
894-
* - variation_id MUST be a non-empty decimal-digit string OR null; any
895-
* invalid input (empty, whitespace, non-numeric) normalizes to null.
896-
* - events[].entity_id (impression only) follows the same rule as
897-
* campaign_id and MUST equal decisions[].campaign_id byte-for-byte.
898-
* - The rule applies uniformly across all decision types (experiment,
886+
* - campaign_id: non-empty string (opaque IDs allowed); fall back to
887+
* experiment_id when empty/null/missing; emit null when both are
888+
* empty/null.
889+
* - variation_id: non-empty decimal-digit string OR null; any invalid
890+
* input (empty, whitespace, non-numeric) normalizes to null.
891+
* - events[].entity_id (impression only) follows the campaign_id rule
892+
* and must equal decisions[].campaign_id byte-for-byte.
893+
* - Rule applies uniformly across all decision types (experiment,
899894
* feature-test, rollout, holdout) — no per-type branching.
900-
* - Conversion events derive entity_id from event.id and are left
901-
* unchanged.
902-
* - Event dispatch is never dropped or failed by normalization, and
903-
* normalization never logs.
895+
* - Conversion events derive entity_id from event.id and are unchanged.
896+
* - Normalization never drops, fails, or logs.
904897
*/
905-
describe('decision event ID normalization (FSSDK-12813)', () => {
898+
describe('decision event ID normalization', () => {
906899
const baseContext = {
907900
accountId: 'accountId',
908901
projectId: 'projectId',
@@ -963,41 +956,38 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
963956
});
964957

965958
it('preserves an opaque non-numeric layerId unchanged', () => {
966-
// FSSDK-12813: campaign_id contract is "any non-empty string". Opaque
967-
// IDs like "layer_abc" or "default-12345" pass through unchanged.
959+
// campaign_id contract is "any non-empty string"; opaque IDs like
960+
// "layer_abc" or "default-12345" pass through unchanged.
968961
const decision = getDecision(
969962
makeImpression({ layerId: 'layer_abc', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
970963
);
971964
expect(decision.campaign_id).toBe('layer_abc');
972965
});
973966

974967
it('preserves a layerId with a negative sign unchanged', () => {
975-
// FSSDK-12813: Any non-empty string is valid for campaign_id.
976968
const decision = getDecision(
977969
makeImpression({ layerId: '-123', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
978970
);
979971
expect(decision.campaign_id).toBe('-123');
980972
});
981973

982974
it('preserves a decimal-formatted layerId unchanged', () => {
983-
// FSSDK-12813: Any non-empty string is valid for campaign_id.
984975
const decision = getDecision(
985976
makeImpression({ layerId: '123.45', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
986977
);
987978
expect(decision.campaign_id).toBe('123.45');
988979
});
989980

990981
it('preserves an exponential-notation layerId unchanged', () => {
991-
// FSSDK-12813: Any non-empty string is valid for campaign_id.
992982
const decision = getDecision(
993983
makeImpression({ layerId: '1e5', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
994984
);
995985
expect(decision.campaign_id).toBe('1e5');
996986
});
997987

998988
it('preserves a whitespace-only layerId unchanged', () => {
999-
// FSSDK-12813: Whitespace is a non-empty string; only empty string,
1000-
// null, and undefined trigger the experiment_id fallback.
989+
// Whitespace is a non-empty string; only empty string, null, and
990+
// undefined trigger the experiment_id fallback.
1001991
const decision = getDecision(
1002992
makeImpression({ layerId: ' ', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
1003993
);
@@ -1019,7 +1009,7 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
10191009
});
10201010

10211011
it('substitutes an opaque experiment_id when layerId is null', () => {
1022-
// FSSDK-12813: experiment_id fallback also accepts any non-empty string.
1012+
// experiment_id fallback also accepts any non-empty string.
10231013
const decision = getDecision(
10241014
makeImpression({ layerId: null, experimentId: 'exp_42', variationId: '111', ruleType: 'experiment' })
10251015
);
@@ -1110,8 +1100,8 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
11101100

11111101
ruleTypes.forEach((ruleType) => {
11121102
it(`normalizes campaign_id identically for ruleType=${ruleType}`, () => {
1113-
// FSSDK-12813: null layerId triggers fallback to experiment_id; the
1114-
// fallback fires identically regardless of rule type.
1103+
// null layerId triggers fallback to experiment_id; the fallback
1104+
// fires identically regardless of rule type.
11151105
const decision = getDecision(
11161106
makeImpression({ layerId: null, experimentId: '67890', variationId: '111', ruleType })
11171107
);
@@ -1185,9 +1175,8 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
11851175
// ---------------------------------------------------------------------------
11861176
describe('byte-equivalent output (FR-008)', () => {
11871177
it('produces identical JSON for two identical event inputs', () => {
1188-
// FSSDK-12813: identical inputs must produce identical wire output.
1189-
// layerId here is a valid non-empty string (passes through unchanged);
1190-
// variationId is non-numeric (normalizes to null).
1178+
// layerId here is a valid non-empty string (passes through); the
1179+
// variationId is non-numeric and normalizes to null.
11911180
const e1 = makeImpression({
11921181
layerId: 'layer_abc',
11931182
experimentId: '67890',

lib/event_processor/event_builder/log_event.ts

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -161,41 +161,26 @@ function makeConversionSnapshot(conversion: ConversionEvent): Snapshot {
161161
}
162162

163163
/**
164-
* FSSDK-12813: Non-empty string validator used to normalize campaign_id and
165-
* entity_id (decision-event ID fields whose contract is "any non-empty
166-
* string"). IDs may be opaque, e.g. "default-12345" or "layer_abc".
167-
*
168-
* Returns true if value is a string of length >= 1. Any character content is
169-
* accepted. Empty string, non-string types, null, and undefined are invalid.
164+
* Non-empty string validator used to normalize campaign_id and entity_id.
165+
* IDs may be opaque, e.g. "default-12345" or "layer_abc".
170166
*/
171167
function isNonEmptyString(value: unknown): value is string {
172168
return typeof value === 'string' && value.length > 0;
173169
}
174170

175171
/**
176-
* FSSDK-12813: Numeric-string validator used to normalize variation_id.
177-
*
178-
* Returns true if value is a non-empty string consisting entirely of decimal
179-
* digits [0-9]. Leading zeros are allowed. Whitespace, negatives, decimals,
180-
* exponents, non-string types, null, and undefined are all invalid.
172+
* Numeric-string validator used to normalize variation_id. Returns true if
173+
* value is a non-empty string of decimal digits [0-9]. Leading zeros are
174+
* allowed.
181175
*/
182176
function isNumericString(value: unknown): value is string {
183177
return typeof value === 'string' && value.length > 0 && /^[0-9]+$/.test(value);
184178
}
185179

186180
/**
187-
* FSSDK-12813: Normalize a campaign_id / entity_id field.
188-
*
189-
* Rule (FR-001/FR-002, FR-009): if the provided id is a non-empty string
190-
* return it unchanged (any character content is accepted — IDs may be
191-
* opaque, e.g. "default-12345", "layer_abc"); otherwise substitute
192-
* experimentId when experimentId itself is a non-empty string. If neither
193-
* is valid (empty string, null, or undefined), return null so the wire
194-
* payload is byte-equivalent across SDKs.
195-
*
196-
* Applies uniformly to ALL decision types (experiment, feature test,
197-
* rollout, holdout) — there is no per-type branching here (FR-005).
198-
* Does not drop or fail event dispatch (FR-006) and does not log (FR-007).
181+
* Normalize a campaign_id / entity_id field. Returns the provided id when
182+
* it is a non-empty string; otherwise substitutes experimentId; otherwise
183+
* returns null so the wire payload is byte-equivalent across SDKs.
199184
*/
200185
function normalizeCampaignId(id: unknown, experimentId: unknown): string | null {
201186
if (isNonEmptyString(id)) {
@@ -208,13 +193,8 @@ function normalizeCampaignId(id: unknown, experimentId: unknown): string | null
208193
}
209194

210195
/**
211-
* FSSDK-12813: Normalize a variation_id field.
212-
*
213-
* Rule (FR-003/FR-004): variation_id retains the stricter numeric-string
214-
* contract. If the provided id is a non-empty numeric-string return it
215-
* unchanged; otherwise substitute null. Applies uniformly to ALL decision
216-
* types (FR-005). Does not drop or fail event dispatch (FR-006) and does
217-
* not log (FR-007).
196+
* Normalize a variation_id field. variation_id keeps the stricter
197+
* numeric-string contract — non-numeric / empty input normalizes to null.
218198
*/
219199
function normalizeVariationId(id: unknown): string | null {
220200
return isNumericString(id) ? id : null;
@@ -227,9 +207,8 @@ function makeDecisionSnapshot(event: ImpressionEvent): Snapshot {
227207
const variationId = variation?.id ?? ''
228208
const variationKey = variation ? variation.key : ''
229209

230-
// FSSDK-12813: Normalize decision-event IDs uniformly across all decision
231-
// types (experiment, feature test, rollout, holdout). entity_id on the
232-
// impression event MUST equal decisions[].campaign_id byte-for-byte (FR-009).
210+
// entity_id on the impression event mirrors decisions[].campaign_id
211+
// byte-for-byte so the two fields stay wire-equivalent.
233212
const normalizedCampaignId = normalizeCampaignId(layerId, experimentId);
234213
const normalizedVariationId = normalizeVariationId(variationId);
235214

lib/optimizely/index.tests.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6690,7 +6690,6 @@ describe('lib/optimizely', function() {
66906690
{
66916691
campaign_id: null,
66926692
experiment_id: '',
6693-
// FSSDK-12813: empty/non-numeric variation_id normalizes to null on the wire.
66946693
variation_id: null,
66956694
metadata: {
66966695
flag_key: 'test_feature',

0 commit comments

Comments
 (0)