[TikTok Audiences] - new syncAudience action#3732
[TikTok Audiences] - new syncAudience action#3732joe-ayoub-segment wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new syncAudience action to the TikTok Audiences destination intended to support both audience additions and removals via audienceMembership, aligning with the broader “single sync action” pattern used by other audience destinations.
Changes:
- Introduces
syncAudienceaction implementation (single + batch) for TikTok Audiences. - Registers the new action in the TikTok Audiences destination.
- Adds a new feature flag constant and a new test file under the
syncAudienceaction folder.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/tiktok-audiences/syncAudience/index.ts | New action implementation for syncing adds/removes using audienceMembership. |
| packages/destination-actions/src/destinations/tiktok-audiences/syncAudience/generated-types.ts | Generated payload type for the new action. |
| packages/destination-actions/src/destinations/tiktok-audiences/syncAudience/tests/index.test.ts | Adds tests under the new action, but currently targets the wrong action. |
| packages/destination-actions/src/destinations/tiktok-audiences/index.ts | Registers syncAudience in the destination’s actions map. |
| packages/core/src/flags.ts | Adds a TikTok audience membership feature flag constant. |
| const action: ActionDefinition<Settings, Payload, AudienceSettings> = { | ||
| title: 'Sync Audience', | ||
| description: 'Sync an Engage Audience to a TikTok Audience Segment.', | ||
| defaultSubscription: 'type = "track"', |
There was a problem hiding this comment.
defaultSubscription is currently type = "track", but audienceMembership is resolved for both track and identify Engage audience events (see resolveAudienceMembership behavior). As written, this action will never run for identify-based audience syncs. Update the subscription to include identify (e.g., type = "track" or type = "identify") to support all audience membership sources.
| defaultSubscription: 'type = "track"', | |
| defaultSubscription: 'type = "track" or type = "identify"', |
| if(!Array.isArray(audienceMembership)){ | ||
| throw new PayloadValidationError('Audience Memberships must be an array') | ||
| } | ||
| if(audienceMembership.length !== payload.length){ | ||
| throw new PayloadValidationError('Audience Memberships length must match payloads length') | ||
| } | ||
| if (audienceMembership.some((membership) => typeof membership !== 'boolean')) { | ||
| throw new PayloadValidationError('Audience Membership must be a boolean'); | ||
| } |
There was a problem hiding this comment.
In batch mode, audienceMembership items can legally be undefined (the core type is boolean | undefined). The current validation typeof membership !== 'boolean' will throw for any undefined entry, failing the entire batch even if only some payloads are non-audience events. Consider allowing undefined and either (a) returning a MultiStatusResponse that marks those specific indices as payload validation failures, or (b) filtering them out explicitly with a clear error path.
| if (addPayloads && addPayloads.length > 0) { | ||
| statsClient?.incr('addToAudience', 1, statsTag) | ||
| await processPayload(request, audienceSettings, addPayloads, 'add') | ||
| } | ||
|
|
||
| if (deletePayloads && deletePayloads.length > 0) { | ||
| statsClient?.incr('removeFromAudience', 1, statsTag) | ||
| await processPayload(request, audienceSettings, deletePayloads, 'delete') | ||
| } | ||
|
|
||
| return {} // what to return here? | ||
| } |
There was a problem hiding this comment.
performBatch currently ignores the responses from the add/delete processPayload calls and always returns {} (with a TODO). In @segment/actions-core, returning a plain object causes the framework to assume the whole batch succeeded (200), which can incorrectly mark failures as successes (and it can’t represent partial failures between the add and delete fan-out). Return a proper batch result (ideally a MultiStatusResponse like other syncAudience implementations) and propagate errors from each request to the correct payload indices.
| describe('TiktokAudiences.addToAudience', () => { | ||
| it('should succeed if audience id is valid', async () => { | ||
| nock(`${BASE_URL}${TIKTOK_AUDIENCES_API_VERSION}`).post('/segment/mapping/', updateUsersRequestBody).reply(200) | ||
|
|
||
| const r = await testDestination.testAction('addToAudience', { | ||
| auth, | ||
| event, | ||
| settings: {}, | ||
| useDefaultMappings: true, |
There was a problem hiding this comment.
This new test file lives under syncAudience/__tests__, but the suite is named TiktokAudiences.addToAudience and all tests call testAction('addToAudience', ...), which means the new syncAudience action isn’t being tested at all (and these cases appear duplicated from tiktok-audiences/addToAudience/__tests__/index.test.ts). Update the tests to target syncAudience and cover both audienceMembership=true (add) and audienceMembership=false (delete), including mixed-membership performBatch behavior.
| @@ -0,0 +1,101 @@ | |||
| import { ActionDefinition, PayloadValidationError, } from '@segment/actions-core' | |||
There was a problem hiding this comment.
The import has an extra trailing comma inside the braces ({ ActionDefinition, PayloadValidationError, }), which is invalid TypeScript and will fail compilation. Remove the dangling comma/whitespace and consider matching the existing pattern in this destination (type-only ActionDefinition import, and import IntegrationError from the same statement).
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (15.09%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3732 +/- ##
==========================================
- Coverage 80.94% 80.66% -0.29%
==========================================
Files 1637 1388 -249
Lines 31963 28007 -3956
Branches 7096 6055 -1041
==========================================
- Hits 25872 22591 -3281
+ Misses 5112 4456 -656
+ Partials 979 960 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| describe('TiktokAudiences.addToAudience', () => { | ||
| it('should succeed if audience id is valid', async () => { | ||
| nock(`${BASE_URL}${TIKTOK_AUDIENCES_API_VERSION}`).post('/segment/mapping/', updateUsersRequestBody).reply(200) | ||
|
|
||
| const r = await testDestination.testAction('addToAudience', { | ||
| auth, |
There was a problem hiding this comment.
This test file lives under syncAudience/__tests__ but the suite still targets addToAudience (describe name and testAction('addToAudience', ...)). As written it duplicates the existing addToAudience tests and does not exercise the new syncAudience action (including the audienceMembership add/delete fan-out behavior). Consider rewriting these tests to call testBatchAction('syncAudience', ...) (or unit-test syncAudience/functions.ts) and removing the duplicated addToAudience coverage here.
| const payloads = Array.from(payloadMap.values()) | ||
| const advertiserId = audienceSettings.advertiserId | ||
| const idSchema = getIDSchema(payloads[0]) | ||
| const batchData = extractUsers(payloads) | ||
| const TikTokApiClient = new TikTokAudiences(request, advertiserId) | ||
|
|
||
| return TikTokApiClient.batchUpdate({ | ||
| advertiser_ids: [advertiserId], | ||
| action, |
There was a problem hiding this comment.
audienceSettings.advertiserId is used to construct the TikTok client and request body, but it isn't validated. If advertiserId is missing/undefined, this will send advertiser_ids: [undefined] (and may construct an invalid client), leading to hard-to-debug API errors. Add an explicit validation that audienceSettings.advertiserId is a non-empty string and fail/mark errors accordingly before calling batchUpdate.
| for (const payload of payloads) { | ||
| const userIds: Record<string, unknown>[] = [] | ||
| const audienceIds = [payload.external_audience_id] | ||
|
|
||
| if (payload.send_email) { | ||
| if (payload.email) { | ||
| const normalized = normalizeEmail(payload.email) | ||
| userIds.push({ id: isHashedInformation(normalized) ? normalized : hash(normalized), audience_ids: audienceIds }) |
There was a problem hiding this comment.
external_audience_id is treated as present when building audienceIds, but the field is not required and validate() never checks it. If it's missing, the code will send audience_ids: [undefined] to TikTok. Add validation to ensure payload.external_audience_id is a non-empty string (and return a per-item validation error in batch mode).
| if (!audienceSettings) { | ||
| return returnAllErrors(multiStatusResponse, payloads, 400, 'Bad Request: no audienceSettings found.', isBatch) | ||
| } |
There was a problem hiding this comment.
When audienceSettings is missing, this throws a PayloadValidationError. In the existing TikTok audience actions the same condition is treated as an integration/request-data issue and throws IntegrationError('Bad Request: no audienceSettings found.', 'INVALID_REQUEST_DATA', 400) (see tiktok-audiences/addToAudience/index.ts:37-39 and removeFromAudience/index.ts:37-39). Aligning the error type/code here will make error reporting consistent across actions.
| const advertiserId = audienceSettings.advertiserId | ||
| const idSchema = getIDSchema(payloads[0]) | ||
| const batchData = extractUsers(payloads) | ||
| const TikTokApiClient = new TikTokAudiences(request, advertiserId) | ||
|
|
There was a problem hiding this comment.
audienceSettings.advertiserId is optional in the generated type, but this code uses it unconditionally to construct the TikTok client and request body. If advertiserId is unset, you'll send advertiser_ids: [undefined] and call the API with an invalid advertiser id. Add an explicit validation (and for batch, set per-item errors) when audienceSettings.advertiserId is missing.
This PR adds a new syncAudience action to the TikTok Audiences destination that handles both audience additions and removals in a single action, replacing the need to use separate addToAudience and removeFromAudience actions.
It leverages actions-core audienceMembership array to fan out a single batch into separate add and remove requests.
Testing
Added multiple unit tests to cover new functionality.
To be tested in Staging.
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.
Security Review
Please ensure sensitive data is properly protected in your integration.
type: 'password'New Destination Checklist
verioning-info.tsfile. example