Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DC-792] injects matchingKey in the perform block #2119

Merged
merged 3 commits into from
Jun 28, 2024
Merged

[DC-792] injects matchingKey in the perform block #2119

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34aeca4
injects matchingKey in the perform block
Jun 27, 2024
f872dc4
nudge package size limit
Jun 27, 2024
4717138
remove identifier fields check
Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions packages/core/src/__tests__/destination-kit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,33 @@ const destinationWithSyncMode: DestinationDefinition<JSONObject> = {
}
}

const destinationWithIdentifier: DestinationDefinition<JSONObject> = {
name: 'Actions Google Analytics 4',
mode: 'cloud',
actions: {
customEvent: {
title: 'Send a Custom Event',
description: 'Send events to a custom event in API',
defaultSubscription: 'type = "track"',
fields: {
userId: {
label: 'User ID',
description: 'The user ID',
type: 'string',
required: true,
category: 'identifier'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to understand the multiple identifiers case, suppose we had an additional identifier field 'email' here. Then we would automatically push a 'Record Matching' dropdown field that users would pick userID or email from, and depending on that the matchingKey would pull the correct value from the payload for the perform block? Is that a good understanding?

Copy link
Contributor Author

@konoufo konoufo Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to understand the multiple identifiers case, suppose we had an additional identifier field 'email' here. Then we would automatically push a 'Record Matching' dropdown field that users would pick userID or email from, and depending on that the matchingKey would pull the correct value from the payload for the perform block? Is that a good understanding?

@nick-Ag Exactly 💯 . So in the UI, the selected field out of all the identifier fields defined would have been stored in mapping['__segment_internal_matching_key'], which is exactly what matchingKey is set to.

}
},
perform: (_request, { matchingKey }) => {
return ['this is a test', matchingKey]
},
performBatch: (_request, { matchingKey }) => {
return ['this is a test', matchingKey]
}
}
}
}

const destinationWithDynamicFields: DestinationDefinition<JSONObject> = {
name: 'Actions Dynamic Fields',
mode: 'cloud',
Expand Down Expand Up @@ -432,6 +459,57 @@ describe('destination kit', () => {
}
])
})

test('should inject the matchingKey value in the perform handler', async () => {
const destinationTest = new Destination(destinationWithIdentifier)
const testEvent: SegmentEvent = { type: 'track' }
const testSettings = {
apiSecret: 'test_key',
subscription: {
subscribe: 'type = "track"',
partnerAction: 'customEvent',
mapping: {
__segment_internal_matching_key: 'userId',
userId: 'this-is-a-user-id'
}
}
}

const res = await destinationTest.onEvent(testEvent, testSettings)

expect(res).toEqual([
{ output: 'Mappings resolved' },
{ output: 'Payload validated' },
{
output: 'Action Executed',
data: ['this is a test', 'userId']
}
])
})

test('should inject the matchingKey value in the performBatch handler', async () => {
const destinationTest = new Destination(destinationWithIdentifier)
const testEvent: SegmentEvent = { type: 'track' }
const testSettings = {
subscription: {
subscribe: 'type = "track"',
partnerAction: 'customEvent',
mapping: {
__segment_internal_matching_key: 'userId',
userId: 'this-is-a-user-id'
}
}
}

const res = await destinationTest.onBatch([testEvent], testSettings)

expect(res).toEqual([
{
output: 'Action Executed',
data: ['this is a test', 'userId']
}
])
})
})

describe('refresh token', () => {
Expand Down
42 changes: 25 additions & 17 deletions packages/core/src/destination-kit/action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,13 @@ const isSyncMode = (value: unknown): value is SyncMode => {
return syncModeTypes.find((validValue) => value === validValue) !== undefined
}

const INTERNAL_HIDDEN_FIELDS = ['__segment_internal_sync_mode', '__segment_internal_matching_key']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! more tidy than before 👏

const removeInternalHiddenFields = (mapping: JSONObject): JSONObject => {
return Object.keys(mapping).reduce((acc, key) => {
return INTERNAL_HIDDEN_FIELDS.includes(key) ? acc : { ...acc, [key]: mapping[key] }
}, {})
}

/**
* Action is the beginning step for all partner actions. Entrypoints always start with the
* MapAndValidateInput step.
Expand Down Expand Up @@ -265,18 +272,16 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
// TODO cleanup results... not sure it's even used
const results: Result[] = []

// Remove internal hidden fields
const mapping: JSONObject = removeInternalHiddenFields(bundle.mapping)

// Resolve/transform the mapping with the input data
let payload = transform(bundle.mapping, bundle.data) as Payload
let payload = transform(mapping, bundle.data) as Payload
results.push({ output: 'Mappings resolved' })

// Remove empty values (`null`, `undefined`, `''`) when not explicitly accepted
payload = removeEmptyValues(payload, this.schema, true) as Payload

// Remove internal hidden field
if (bundle.mapping && '__segment_internal_sync_mode' in bundle.mapping) {
delete payload['__segment_internal_sync_mode']
}

// Validate the resolved payload against the schema
if (this.schema) {
const schemaKey = `${this.destinationName}:${this.definition.title}`
Expand All @@ -297,6 +302,10 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =

const syncMode = this.definition.syncMode ? bundle.mapping?.['__segment_internal_sync_mode'] : undefined

const matchingKey = Object.values(this.definition.fields).find((field) => field.category === 'identifier')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to remove this check to simplify? Can we just make an attempt to pass bundle.mapping?.['__segment_internal_matching_key'] ... I think will be undefined if the destination does not support record matching anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to remove this check to simplify? Can we just make an attempt to pass bundle.mapping?.['__segment_internal_matching_key'] ... I think will be undefined if the destination does not support record matching anyways.

Yeah I guess I was trying to be defensive lol. By the way, do you think there should be validation to make sure that the value in bundle.mapping?.['__segment_internal_matching_key'] is one of the identifier fields?
I'm on the fence. On one hand the UI is unlikely to cause invalid values but on another hand, things happen e.g someone can save a bad mapping via PAPI?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered removing the condition to simplify future changes. If we decide to change the field category used as the indicator for matching keys, fewer parts of the codebase will need to be updated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also no need for validation since even if someone creates a __segment_internal_matching_key with bad value it won't cause any significant issues in the perform block

? bundle.mapping?.['__segment_internal_matching_key']
: undefined

// Construct the data bundle to send to an action
const dataBundle = {
rawData: bundle.data,
Expand All @@ -312,7 +321,8 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
stateContext: bundle.stateContext,
audienceSettings: bundle.audienceSettings,
hookOutputs,
syncMode: isSyncMode(syncMode) ? syncMode : undefined
syncMode: isSyncMode(syncMode) ? syncMode : undefined,
matchingKey: matchingKey ? String(matchingKey) : undefined
}

// Construct the request client and perform the action
Expand All @@ -329,16 +339,10 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
throw new IntegrationError('This action does not support batched requests.', 'NotImplemented', 501)
}

let payloads = transformBatch(bundle.mapping, bundle.data) as Payload[]
// Remove internal hidden fields
const mapping: JSONObject = removeInternalHiddenFields(bundle.mapping)

// Remove internal hidden field
if (bundle.mapping && '__segment_internal_sync_mode' in bundle.mapping) {
for (const payload of payloads) {
if (payload) {
delete payload['__segment_internal_sync_mode']
}
}
}
let payloads = transformBatch(mapping, bundle.data) as Payload[]

// Validate the resolved payloads against the schema
if (this.schema) {
Expand Down Expand Up @@ -373,6 +377,9 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =

if (this.definition.performBatch) {
const syncMode = this.definition.syncMode ? bundle.mapping?.['__segment_internal_sync_mode'] : undefined
const matchingKey = Object.values(this.definition.fields).find((field) => field.category === 'identifier')
? bundle.mapping?.['__segment_internal_matching_key']
: undefined
const data = {
rawData: bundle.data,
rawMapping: bundle.mapping,
Expand All @@ -387,7 +394,8 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
transactionContext: bundle.transactionContext,
stateContext: bundle.stateContext,
hookOutputs,
syncMode: isSyncMode(syncMode) ? syncMode : undefined
syncMode: isSyncMode(syncMode) ? syncMode : undefined,
matchingKey: matchingKey ? String(matchingKey) : undefined
}
const output = await this.performRequest(this.definition.performBatch, data)
results[0].data = output as JSONObject
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/destination-kit/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export interface ExecuteInput<
page?: string
/** The subscription sync mode */
syncMode?: SyncMode
/** The key for the action's field used to match data between Segment and the Destination */
matchingKey?: string
/** The data needed in OAuth requests */
readonly auth?: AuthTokens
/**
Expand Down
Loading