Skip to content

Commit 4fdbd61

Browse files
bkboothclaude
andcommitted
refactor(audience): address PR review — buildBase helper, consent payload fix
Review feedback from Natalie on PR #2830: 1. Extract buildBase() helper in Pixel class to deduplicate messageId, eventTimestamp, anonymousId, surface, and context fields across page(), identify(), fireSessionStart(), and fireSessionEnd(). 2. Fix consent PUT payload: rename `consentLevel` to `status` and add `source` field to match backend OAS. Accept source as a parameter to createConsentManager() so pixel passes 'pixel' and web SDK can pass 'sdk'. 3. Add comment on direct fetch usage in consent (different method/payload than httpSend's BatchPayload POST). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6c90efc commit 4fdbd61

4 files changed

Lines changed: 33 additions & 32 deletions

File tree

packages/audience/core/src/consent.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,19 @@ beforeEach(() => {
2626
describe('createConsentManager', () => {
2727
it('defaults to none when no initial level provided', () => {
2828
const queue = createMockQueue();
29-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev');
29+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel');
3030
expect(manager.level).toBe('none');
3131
});
3232

3333
it('uses the initial level when provided', () => {
3434
const queue = createMockQueue();
35-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'anonymous');
35+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous');
3636
expect(manager.level).toBe('anonymous');
3737
});
3838

3939
it('upgrades consent without modifying queue', () => {
4040
const queue = createMockQueue();
41-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'none');
41+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'none');
4242

4343
manager.setLevel('anonymous');
4444
expect(manager.level).toBe('anonymous');
@@ -48,7 +48,7 @@ describe('createConsentManager', () => {
4848

4949
it('purges queue on downgrade to none', () => {
5050
const queue = createMockQueue();
51-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'full');
51+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'full');
5252

5353
manager.setLevel('none');
5454
expect(manager.level).toBe('none');
@@ -61,7 +61,7 @@ describe('createConsentManager', () => {
6161

6262
it('strips userId on downgrade from full to anonymous', () => {
6363
const queue = createMockQueue();
64-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'full');
64+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'full');
6565

6666
manager.setLevel('anonymous');
6767
expect(manager.level).toBe('anonymous');
@@ -77,7 +77,7 @@ describe('createConsentManager', () => {
7777

7878
it('fires PUT to consent endpoint on level change', () => {
7979
const queue = createMockQueue();
80-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'none');
80+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'none');
8181

8282
manager.setLevel('anonymous');
8383

@@ -89,13 +89,14 @@ describe('createConsentManager', () => {
8989
'Content-Type': 'application/json',
9090
'x-immutable-publishable-key': 'pk_test',
9191
}),
92+
body: JSON.stringify({ anonymousId: 'anon-1', status: 'anonymous', source: 'pixel' }),
9293
}),
9394
);
9495
});
9596

9697
it('does nothing when setting the same level', () => {
9798
const queue = createMockQueue();
98-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'anonymous');
99+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous');
99100

100101
manager.setLevel('anonymous');
101102
expect(queue.purge).not.toHaveBeenCalled();
@@ -107,7 +108,7 @@ describe('createConsentManager', () => {
107108
Object.defineProperty(navigator, 'doNotTrack', { value: '1', configurable: true });
108109

109110
const queue = createMockQueue();
110-
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev');
111+
const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel');
111112
expect(manager.level).toBe('none');
112113

113114
Object.defineProperty(navigator, 'doNotTrack', { value: '0', configurable: true });

packages/audience/core/src/consent.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export function createConsentManager(
3030
publishableKey: string,
3131
anonymousId: string,
3232
environment: Environment,
33+
source: string,
3334
initialLevel?: ConsentLevel,
3435
): ConsentManager {
3536
const dntDetected = detectDoNotTrack();
@@ -39,7 +40,10 @@ export function createConsentManager(
3940

4041
function notifyBackend(level: ConsentLevel): void {
4142
const url = `${getBaseUrl(environment)}${CONSENT_PATH}`;
42-
const payload = { anonymousId, consentLevel: level };
43+
const payload = { anonymousId, status: level, source };
44+
// Uses fetch directly rather than httpSend because this is a PUT
45+
// to a different endpoint with a different payload shape than the
46+
// message ingest POST that httpSend is designed for.
4347
fetch(url, {
4448
method: 'PUT',
4549
headers: {

packages/audience/pixel/src/pixel.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ jest.mock('@imtbl/audience-core', () => ({
4545
}),
4646
getOrCreateSession: (...args: unknown[]) => mockGetOrCreateSession(...args),
4747
createConsentManager: jest.fn().mockImplementation(
48-
(_queue: unknown, _key: unknown, _anonId: unknown, _env: unknown, level?: string) => {
48+
(_queue: unknown, _key: unknown, _anonId: unknown, _env: unknown, _source: unknown, level?: string) => {
4949
let current = level ?? 'none';
5050
return {
5151
get level() { return current; },

packages/audience/pixel/src/pixel.ts

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export class Pixel {
9292
key,
9393
this.anonymousId,
9494
environment,
95+
'pixel',
9596
consentLevel,
9697
);
9798

@@ -112,15 +113,10 @@ export class Pixel {
112113
const { sessionId, isNew } = getOrCreateSession(this.domain);
113114
this.refreshSession(sessionId, isNew);
114115
const attribution = collectAttribution();
115-
const context = collectContext('@imtbl/pixel', PIXEL_VERSION);
116116

117117
const message: PageMessage = {
118+
...this.buildBase(),
118119
type: 'page',
119-
messageId: generateId(),
120-
eventTimestamp: getTimestamp(),
121-
anonymousId: this.anonymousId,
122-
surface: 'pixel',
123-
context,
124120
properties: {
125121
...attribution,
126122
sessionId,
@@ -138,15 +134,10 @@ export class Pixel {
138134
this.userId = userId;
139135
const { sessionId, isNew } = getOrCreateSession(this.domain);
140136
this.refreshSession(sessionId, isNew);
141-
const context = collectContext('@imtbl/pixel', PIXEL_VERSION);
142137

143138
const message: IdentifyMessage = {
139+
...this.buildBase(),
144140
type: 'identify',
145-
messageId: generateId(),
146-
eventTimestamp: getTimestamp(),
147-
anonymousId: this.anonymousId,
148-
surface: 'pixel',
149-
context,
150141
userId,
151142
traits: {
152143
...traits,
@@ -186,13 +177,9 @@ export class Pixel {
186177
if (!this.canTrack()) return;
187178

188179
const message: TrackMessage = {
180+
...this.buildBase(),
189181
type: 'track',
190182
eventName: 'session_start',
191-
messageId: generateId(),
192-
eventTimestamp: getTimestamp(),
193-
anonymousId: this.anonymousId,
194-
surface: 'pixel',
195-
context: collectContext('@imtbl/pixel', PIXEL_VERSION),
196183
properties: { sessionId },
197184
userId: this.consent!.level === 'full' ? this.userId : undefined,
198185
};
@@ -208,13 +195,9 @@ export class Pixel {
208195
: undefined;
209196

210197
const message: TrackMessage = {
198+
...this.buildBase(),
211199
type: 'track',
212200
eventName: 'session_end',
213-
messageId: generateId(),
214-
eventTimestamp: getTimestamp(),
215-
anonymousId: this.anonymousId,
216-
surface: 'pixel',
217-
context: collectContext('@imtbl/pixel', PIXEL_VERSION),
218201
properties: {
219202
sessionId: this.sessionId,
220203
duration,
@@ -247,6 +230,19 @@ export class Pixel {
247230
}
248231
}
249232

233+
// -- Helpers ------------------------------------------------------------
234+
235+
// eslint-disable-next-line class-methods-use-this
236+
private buildBase() {
237+
return {
238+
messageId: generateId(),
239+
eventTimestamp: getTimestamp(),
240+
anonymousId: this.anonymousId,
241+
surface: 'pixel' as const,
242+
context: collectContext('@imtbl/pixel', PIXEL_VERSION),
243+
};
244+
}
245+
250246
// -- Guards -------------------------------------------------------------
251247

252248
private canTrack(): boolean {

0 commit comments

Comments
 (0)