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.

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-789] adds subscription sync modes support in destination-kit #2065

Merged
merged 4 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
83 changes: 83 additions & 0 deletions packages/core/src/__tests__/destination-kit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,40 @@ const destinationWithOptions: DestinationDefinition<JSONObject> = {
}
}

const destinationWithSyncMode: 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: {},
syncMode: {
default: 'add',
description: 'Select the sync mode for the subscription',
label: 'Sync Mode',
choices: [
{
label: 'Insert',
value: 'add'
},
{
label: 'Delete',
value: 'delete'
}
]
},
perform: (_request, { syncMode }) => {
return ['this is a test', syncMode]
},
performBatch: (_request, { syncMode }) => {
return ['this is a test', syncMode]
}
}
}
}

describe('destination kit', () => {
describe('event validations', () => {
test('should return `invalid subscription` when sending an empty subscribe', async () => {
Expand Down Expand Up @@ -275,6 +309,55 @@ describe('destination kit', () => {
{ output: 'Action Executed', data: ['this is a test', {}] }
])
})

test('should inject the syncMode value in the perform handler', async () => {
const destinationTest = new Destination(destinationWithSyncMode)
const testEvent: SegmentEvent = { type: 'track' }
const testSettings = {
apiSecret: 'test_key',
subscription: {
subscribe: 'type = "track"',
partnerAction: 'customEvent',
mapping: {
__segment_internal_sync_mode: 'add'
}
}
}

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

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

test('should inject the syncMode value in the performBatch handler', async () => {
const destinationTest = new Destination(destinationWithSyncMode)
const testEvent: SegmentEvent = { type: 'track' }
const testSettings = {
apiSecret: 'test_key',
subscription: {
subscribe: 'type = "track"',
partnerAction: 'customEvent',
mapping: {
__segment_internal_sync_mode: 'add'
}
}
}

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

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

describe('refresh token', () => {
Expand Down
41 changes: 38 additions & 3 deletions packages/core/src/destination-kit/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@ import { InputData, Features, transform, transformBatch } from '../mapping-kit'
import { fieldsToJsonSchema } from './fields-to-jsonschema'
import { Response } from '../fetch'
import type { ModifiedResponse } from '../types'
import type { DynamicFieldResponse, InputField, RequestExtension, ExecuteInput, Result } from './types'
import type {
DynamicFieldResponse,
InputField,
RequestExtension,
ExecuteInput,
Result,
SyncMode,
SyncModeDefinition
} from './types'
import { syncModeTypes } from './types'
import { NormalizedOptions } from '../request-client'
import type { JSONSchema4 } from 'json-schema'
import { validateSchema } from '../schema-validation'
Expand Down Expand Up @@ -93,6 +102,9 @@ export interface ActionDefinition<
NonNullable<GeneratedActionHookBundle[K]>['inputs']
>
}

/** The sync mode setting definition. This enables subscription sync mode selection when subscribing to this action. */
syncMode?: SyncModeDefinition
}

export const hookTypeStrings = ['onMappingSave', 'retlOnMappingSave'] as const
Expand Down Expand Up @@ -171,6 +183,10 @@ interface ExecuteBundle<T = unknown, Data = unknown, AudienceSettings = any, Act
stateContext?: StateContext
}

const isSyncMode = (value: unknown): value is SyncMode => {
return syncModeTypes.find((validValue) => value === validValue) !== undefined
}

/**
* Action is the beginning step for all partner actions. Entrypoints always start with the
* MapAndValidateInput step.
Expand Down Expand Up @@ -246,6 +262,11 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
// 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 @@ -264,6 +285,8 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
}
}

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

Choose a reason for hiding this comment

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

Will __segment_internal_sync_mode show up in the payload as well? I think we can possibly filter it from the payload 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it would show up. Ok let's get rid of it.


// Construct the data bundle to send to an action
const dataBundle = {
rawData: bundle.data,
Expand All @@ -278,7 +301,8 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
transactionContext: bundle.transactionContext,
stateContext: bundle.stateContext,
audienceSettings: bundle.audienceSettings,
hookOutputs
hookOutputs,
syncMode: isSyncMode(syncMode) ? syncMode : undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to implement a similar logic for executeBatch as well.

}

// Construct the request client and perform the action
Expand All @@ -297,6 +321,15 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =

let payloads = transformBatch(bundle.mapping, bundle.data) as Payload[]

// 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']
}
}
}

// Validate the resolved payloads against the schema
if (this.schema) {
const schema = this.schema
Expand Down Expand Up @@ -329,6 +362,7 @@ 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 data = {
rawData: bundle.data,
rawMapping: bundle.mapping,
Expand All @@ -342,7 +376,8 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings =
dataFeedCache: bundle.dataFeedCache,
transactionContext: bundle.transactionContext,
stateContext: bundle.stateContext,
hookOutputs
hookOutputs,
syncMode: isSyncMode(syncMode) ? syncMode : undefined
}
const output = await this.performRequest(this.definition.performBatch, data)
results[0].data = output as JSONObject
Expand Down
48 changes: 45 additions & 3 deletions packages/core/src/destination-kit/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export interface ExecuteInput<
hookOutputs?: Partial<Record<ActionHookType, ActionHookOutputs>>
/** The page used in dynamic field requests */
page?: string
/** The subscription sync mode */
syncMode?: SyncMode
/** The data needed in OAuth requests */
readonly auth?: AuthTokens
/**
Expand Down Expand Up @@ -191,18 +193,35 @@ export interface InputField extends InputFieldJSONSchema {
depends_on?: DependsOnConditions
}

/** Base interface for conditions */
interface BaseCondition {
operator: 'is' | 'is_not'
}

/**
* A single condition defining whether a field should be shown.
* A single condition defining whether a field should be shown based on the value of the field specified by `fieldKey`.
* fieldKey: The field key in the fields object to look at
* operator: The operator to use when comparing the field value
* value: The value we expect that field to have, if undefined, we will match based on whether the field contains a value or not
*/
export interface Condition {
export interface FieldCondition extends BaseCondition {
fieldKey: string
operator: 'is' | 'is_not'
type?: 'field' // optional for backwards compatibility
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be worth it to make type required now since there are only a few places using it that you'd have to update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting. I found that I'd need to make changes in app and this repo. And that's just what I could find quickly. Is it possible to break compatibility anywhere else? Can you help round up all the places this could break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

value: Omit<FieldValue, 'Directive'> | Array<Omit<FieldValue, 'Directive'>> | undefined
}

/**
* A single condition defining whether a field should be shown based on the current sync mode.
* operator: The operator to use when comparing the sync mode
* value: The value to compare against, if undefined, we will match based on whether a sync mode is set or not
*/
export interface SyncModeCondition extends BaseCondition {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering how this will be used, is there some example code of a destination that implements SyncModeCondition? Will it be something like 'if condition.type == 'syncMode' && condition.value === "add"' -> do not show some field that is irrelevant to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering how this will be used, is there some example code of a destination that implements SyncModeCondition? Will it be something like 'if condition.type == 'syncMode' && condition.value === "add"' -> do not show some field that is irrelevant to add?

Good question! There is no example code as of now. This is a completely new thing. And it's going to be used exactly as you described to hide fields in app e.g Action editor. Any concerns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also it's good to point out that the type itself might not be used directly outside this module so maybe an export is not justified?

type: 'syncMode'
value: SyncMode
}

export type Condition = FieldCondition | SyncModeCondition

/**
* If match is not set, it will default to 'all'
* If match = 'any', then meeting any of the conditions defined will result in the field being shown.
Expand Down Expand Up @@ -263,3 +282,26 @@ export type Deletion<Settings, Return = any> = (
request: RequestClient,
data: ExecuteInput<Settings, DeletionPayload>
) => MaybePromise<Return>

/** The supported sync mode values */
export const syncModeTypes = ['add', 'update', 'upsert', 'delete'] as const
export type SyncMode = typeof syncModeTypes[number]

export interface SyncModeOption {
/** The human-readable label for this option */
label: string
/** The value of this option */
value: SyncMode
}

/** An action sync mode definition */
export interface SyncModeDefinition {
/** The default sync mode that will be selected */
default: SyncMode
/** The human-readable label for this setting */
label: string
/** The human-friendly description of the setting */
description: string
/** The available sync mode choices */
choices: SyncModeOption[]
}
Loading