Skip to content

Commit dcd6d16

Browse files
committed
fix review comments
1 parent f4e3a1f commit dcd6d16

File tree

3 files changed

+31
-31
lines changed

3 files changed

+31
-31
lines changed

lib/core/decision_service/cmab/cmab_client.spec.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ const mockErrorResponse = (statusCode: number) => Promise.resolve({
4343
const assertRequest = (
4444
call: number,
4545
mockRequestHandler: MockInstance<RequestHandler['makeRequest']>,
46-
experimentId: string,
46+
ruleId: string,
4747
userId: string,
4848
attributes: Record<string, any>,
4949
cmabUuid: string,
5050
) => {
5151
const [requestUrl, headers, method, data] = mockRequestHandler.mock.calls[call];
52-
expect(requestUrl).toBe(`https://prediction.cmab.optimizely.com/predict/${experimentId}`);
52+
expect(requestUrl).toBe(`https://prediction.cmab.optimizely.com/predict/${ruleId}`);
5353
expect(method).toBe('POST');
5454
expect(headers).toEqual({
5555
'Content-Type': 'application/json',
@@ -59,7 +59,7 @@ const assertRequest = (
5959
expect(parsedData.instances).toEqual([
6060
{
6161
visitorId: userId,
62-
experimentId,
62+
experimentId: ruleId,
6363
attributes: Object.keys(attributes).map((key) => ({
6464
id: key,
6565
value: attributes[key],
@@ -81,18 +81,18 @@ describe('DefaultCmabClient', () => {
8181
requestHandler,
8282
});
8383

84-
const experimentId = '123';
84+
const ruleId = '123';
8585
const userId = 'user123';
8686
const attributes = {
8787
browser: 'chrome',
8888
isMobile: true,
8989
};
9090
const cmabUuid = 'uuid123';
9191

92-
const variation = await cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid);
92+
const variation = await cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid);
9393

9494
expect(variation).toBe('var123');
95-
assertRequest(0, mockMakeRequest, experimentId, userId, attributes, cmabUuid);
95+
assertRequest(0, mockMakeRequest, ruleId, userId, attributes, cmabUuid);
9696
});
9797

9898
it('should retry fetch if retryConfig is provided', async () => {
@@ -110,20 +110,20 @@ describe('DefaultCmabClient', () => {
110110
},
111111
});
112112

113-
const experimentId = '123';
113+
const ruleId = '123';
114114
const userId = 'user123';
115115
const attributes = {
116116
browser: 'chrome',
117117
isMobile: true,
118118
};
119119
const cmabUuid = 'uuid123';
120120

121-
const variation = await cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid);
121+
const variation = await cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid);
122122

123123
expect(variation).toBe('var123');
124124
expect(mockMakeRequest.mock.calls.length).toBe(3);
125125
for(let i = 0; i < 3; i++) {
126-
assertRequest(i, mockMakeRequest, experimentId, userId, attributes, cmabUuid);
126+
assertRequest(i, mockMakeRequest, ruleId, userId, attributes, cmabUuid);
127127
}
128128
});
129129

@@ -157,15 +157,15 @@ describe('DefaultCmabClient', () => {
157157
},
158158
});
159159

160-
const experimentId = '123';
160+
const ruleId = '123';
161161
const userId = 'user123';
162162
const attributes = {
163163
browser: 'chrome',
164164
isMobile: true,
165165
};
166166
const cmabUuid = 'uuid123';
167167

168-
const fetchPromise = cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid);
168+
const fetchPromise = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid);
169169

170170
await exhaustMicrotasks();
171171
expect(mockMakeRequest.mock.calls.length).toBe(1);
@@ -205,7 +205,7 @@ describe('DefaultCmabClient', () => {
205205
expect(variation).toBe('var123');
206206
expect(mockMakeRequest.mock.calls.length).toBe(4);
207207
for(let i = 0; i < 4; i++) {
208-
assertRequest(i, mockMakeRequest, experimentId, userId, attributes, cmabUuid);
208+
assertRequest(i, mockMakeRequest, ruleId, userId, attributes, cmabUuid);
209209
}
210210
vi.useRealTimers();
211211
});
@@ -223,15 +223,15 @@ describe('DefaultCmabClient', () => {
223223
},
224224
});
225225

226-
const experimentId = '123';
226+
const ruleId = '123';
227227
const userId = 'user123';
228228
const attributes = {
229229
browser: 'chrome',
230230
isMobile: true,
231231
};
232232
const cmabUuid = 'uuid123';
233233

234-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toThrow();
234+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toThrow();
235235
expect(mockMakeRequest.mock.calls.length).toBe(6);
236236
});
237237

@@ -248,15 +248,15 @@ describe('DefaultCmabClient', () => {
248248
},
249249
});
250250

251-
const experimentId = '123';
251+
const ruleId = '123';
252252
const userId = 'user123';
253253
const attributes = {
254254
browser: 'chrome',
255255
isMobile: true,
256256
};
257257
const cmabUuid = 'uuid123';
258258

259-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toThrow();
259+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toThrow();
260260
expect(mockMakeRequest.mock.calls.length).toBe(6);
261261
});
262262

@@ -270,15 +270,15 @@ describe('DefaultCmabClient', () => {
270270
requestHandler,
271271
});
272272

273-
const experimentId = '123';
273+
const ruleId = '123';
274274
const userId = 'user123';
275275
const attributes = {
276276
browser: 'chrome',
277277
isMobile: true,
278278
};
279279
const cmabUuid = 'uuid123';
280280

281-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toThrow();
281+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toThrow();
282282
expect(mockMakeRequest.mock.calls.length).toBe(1);
283283
});
284284

@@ -292,15 +292,15 @@ describe('DefaultCmabClient', () => {
292292
requestHandler,
293293
});
294294

295-
const experimentId = '123';
295+
const ruleId = '123';
296296
const userId = 'user123';
297297
const attributes = {
298298
browser: 'chrome',
299299
isMobile: true,
300300
};
301301
const cmabUuid = 'uuid123';
302302

303-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toMatchObject(
303+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toMatchObject(
304304
new OptimizelyError('CMAB_FETCH_FAILED', 500),
305305
);
306306
});
@@ -321,15 +321,15 @@ describe('DefaultCmabClient', () => {
321321
requestHandler,
322322
});
323323

324-
const experimentId = '123';
324+
const ruleId = '123';
325325
const userId = 'user123';
326326
const attributes = {
327327
browser: 'chrome',
328328
isMobile: true,
329329
};
330330
const cmabUuid = 'uuid123';
331331

332-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toMatchObject(
332+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toMatchObject(
333333
new OptimizelyError('INVALID_CMAB_RESPONSE'),
334334
);
335335
});
@@ -344,14 +344,14 @@ describe('DefaultCmabClient', () => {
344344
requestHandler,
345345
});
346346

347-
const experimentId = '123';
347+
const ruleId = '123';
348348
const userId = 'user123';
349349
const attributes = {
350350
browser: 'chrome',
351351
isMobile: true,
352352
};
353353
const cmabUuid = 'uuid123';
354354

355-
await expect(cmabClient.fetchVariation(experimentId, userId, attributes, cmabUuid)).rejects.toThrow('error');
355+
await expect(cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid)).rejects.toThrow('error');
356356
});
357357
});

lib/core/decision_service/cmab/cmab_client.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import { BackoffController } from "../../../utils/repeater/repeater";
2525
import { Producer } from "../../../utils/type";
2626

2727
export interface CmabClient {
28-
fetchVariation(
29-
experimentId: string,
28+
fetchDecision(
29+
ruleId: string,
3030
userId: string,
3131
attributes: UserAttributes,
3232
cmabUuid: string,
@@ -54,13 +54,13 @@ export class DefaultCmabClient implements CmabClient {
5454
this.retryConfig = config.retryConfig;
5555
}
5656

57-
async fetchVariation(
58-
experimentId: string,
57+
async fetchDecision(
58+
ruleId: string,
5959
userId: string,
6060
attributes: UserAttributes,
6161
cmabUuid: string,
6262
): Promise<string> {
63-
const url = sprintf(CMAB_PREDICTION_ENDPOINT, experimentId);
63+
const url = sprintf(CMAB_PREDICTION_ENDPOINT, ruleId);
6464

6565
const cmabAttributes = Object.keys(attributes).map((key) => ({
6666
id: key,
@@ -72,7 +72,7 @@ export class DefaultCmabClient implements CmabClient {
7272
instances: [
7373
{
7474
visitorId: userId,
75-
experimentId,
75+
experimentId: ruleId,
7676
attributes: cmabAttributes,
7777
cmabUUID: cmabUuid,
7878
}

lib/message/error_message.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export const ODP_EVENT_MANAGER_STOPPED = "ODP event manager stopped before it co
106106
export const DATAFILE_MANAGER_FAILED_TO_START = 'Datafile manager failed to start';
107107
export const UNABLE_TO_ATTACH_UNLOAD = 'unable to bind optimizely.close() to page unload event: "%s"';
108108
export const UNABLE_TO_PARSE_AND_SKIPPED_HEADER = 'Unable to parse & skipped header item';
109-
export const CMAB_FETCH_FAILED = 'CMAB variation fetch failed with status: %s';
109+
export const CMAB_FETCH_FAILED = 'CMAB decision fetch failed with status: %s';
110110
export const INVALID_CMAB_FETCH_RESPONSE = 'Invalid CMAB fetch response';
111111

112112
export const messages: string[] = [];

0 commit comments

Comments
 (0)