From aa80fd20ef7c617902a0406da4cc7b8fa7356d2d Mon Sep 17 00:00:00 2001 From: Dave Date: Tue, 12 Mar 2024 08:46:03 -0400 Subject: [PATCH] [Snap v3 CAPI] Additional SnapV3 connector tweaks (#1913) * Update logic for selecting the app or pixel id based upon the event_conversion_type * Trim whitespace from string data. Add tests * add fallback logic for appOrPixelID computation * add validation for num_items parsing * check if the authtoken is empty string and convert to undefined * fallback to content_ids.length when parsing number_items fails * a few simplifications --------- Co-authored-by: David Bordoley --- .../_tests_/capiV3tests.ts | 97 ++++++++++++++++++- .../reportConversionEvent/snap-capi-v3.ts | 29 ++++-- .../reportConversionEvent/utils.ts | 16 ++- 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/packages/destination-actions/src/destinations/snap-conversions-api/_tests_/capiV3tests.ts b/packages/destination-actions/src/destinations/snap-conversions-api/_tests_/capiV3tests.ts index e1cca58775..f77011f404 100644 --- a/packages/destination-actions/src/destinations/snap-conversions-api/_tests_/capiV3tests.ts +++ b/packages/destination-actions/src/destinations/snap-conversions-api/_tests_/capiV3tests.ts @@ -2,16 +2,17 @@ import nock from 'nock' import { createTestEvent, createTestIntegration } from '@segment/actions-core' import Definition from '../index' import { Settings } from '../generated-types' +import { buildRequestURL } from '../reportConversionEvent/snap-capi-v3' const testDestination = createTestIntegration(Definition) const timestamp = '2022-05-12T15:21:15.449Z' const settings: Settings = { snap_app_id: 'test123', - pixel_id: 'test123', - app_id: 'test123' + pixel_id: 'pixel123', + app_id: 'app123' } -const accessToken = 'test123' -const refreshToken = 'test123' +const accessToken = 'access123' +const refreshToken = 'refresh123' const testEvent = createTestEvent({ timestamp: timestamp, @@ -608,4 +609,92 @@ export const capiV3tests = () => ) expect(action_source).toBe('website') }) + + it('should always use the pixel id in settings for web events', async () => { + nock(/.*/).post(/.*/).reply(200) + const event = createTestEvent({ + ...testEvent, + properties: {} + }) + + const responses = await testDestination.testAction('reportConversionEvent', { + event, + settings, + useDefaultMappings: true, + auth: { + accessToken, + refreshToken + }, + features, + mapping: { + event_type: 'PURCHASE', + event_conversion_type: 'WEB' + } + }) + + expect(responses[0].url).toBe(buildRequestURL('pixel123', 'access123')) + }) + + it('should trim a pixel id with leading or trailing whitespace', async () => { + nock(/.*/).post(/.*/).reply(200) + const event = createTestEvent({ + ...testEvent, + properties: {} + }) + + const responses = await testDestination.testAction('reportConversionEvent', { + event, + settings: { + pixel_id: ' pixel123 ' + }, + useDefaultMappings: true, + auth: { + accessToken, + refreshToken + }, + features, + mapping: { + event_type: 'PURCHASE', + event_conversion_type: 'WEB' + } + }) + + expect(responses[0].url).toBe(buildRequestURL('pixel123', 'access123')) + }) + + it('should exclude number_items that is not a valid integer', async () => { + nock(/.*/).post(/.*/).reply(200) + const event = createTestEvent({ + ...testEvent, + properties: {} + }) + + const responses = await testDestination.testAction('reportConversionEvent', { + event, + settings: { + pixel_id: ' pixel123 ' + }, + useDefaultMappings: true, + auth: { + accessToken: ' access123 ', + refreshToken + }, + features, + mapping: { + event_type: 'PURCHASE', + event_conversion_type: 'WEB', + number_items: 'six' + } + }) + + expect(responses[0].url).toBe(buildRequestURL('pixel123', 'access123')) + + const body = JSON.parse(responses[0].options.body as string) + const { data } = body + expect(data.length).toBe(1) + + const { custom_data } = data[0] + + expect(custom_data).toBeUndefined() + }) }) diff --git a/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/snap-capi-v3.ts b/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/snap-capi-v3.ts index 34e410b4e4..20c4159c91 100644 --- a/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/snap-capi-v3.ts +++ b/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/snap-capi-v3.ts @@ -10,7 +10,8 @@ import { splitListValueToArray, raiseMisconfiguredRequiredFieldErrorIf, raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined, - emptyStringToUndefined + emptyStringToUndefined, + parseNumberSafe } from './utils' import { CURRENCY_ISO_4217_CODES } from '../snap-capi-properties' @@ -70,12 +71,15 @@ export const formatPayload = (payload: Payload, settings: Settings, isTest = tru brands: products.map((product) => product.brand ?? ''), num_items: products.length } - : { - content_ids: splitListValueToArray(payload.item_ids ?? ''), - content_category: splitListValueToArray(payload.item_category ?? ''), - brands: payload.brands, - num_items: payload.number_items - } + : (() => { + const content_ids = splitListValueToArray(payload.item_ids ?? '') + return { + content_ids, + content_category: splitListValueToArray(payload.item_category ?? ''), + brands: payload.brands, + num_items: parseNumberSafe(payload.number_items) ?? content_ids?.length + } + })() // FIXME: Ideally advertisers on iOS 14.5+ would pass the ATT_STATUS from the device. // However the field is required for app events, so hardcode the value to false (0) @@ -90,6 +94,8 @@ export const formatPayload = (payload: Payload, settings: Settings, isTest = tru '', // app package name '', // short version '', // long version + + // FIXME: extract from the user agent if available payload.os_version ?? '', // os version payload.device_model ?? '', // device model name '', // local @@ -162,9 +168,10 @@ export const validateAppOrPixelID = (settings: Settings, event_conversion_type: const { snap_app_id, pixel_id } = settings const snapAppID = emptyStringToUndefined(snap_app_id) const snapPixelID = emptyStringToUndefined(pixel_id) - const appOrPixelID = snapAppID ?? snapPixelID - raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined(appOrPixelID, 'Missing valid app or pixel ID') + // Some configurations specify both a snapPixelID and a snapAppID. In these cases + // check the conversion type to ensure that the right id is selected and used. + const appOrPixelID = event_conversion_type === 'WEB' ? snapPixelID : snapAppID raiseMisconfiguredRequiredFieldErrorIf( event_conversion_type === 'MOBILE_APP' && isNullOrUndefined(snapAppID), @@ -176,6 +183,8 @@ export const validateAppOrPixelID = (settings: Settings, event_conversion_type: `If event conversion type is "${event_conversion_type}" then Pixel ID must be defined` ) + raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined(appOrPixelID, 'Missing valid app or pixel ID') + return appOrPixelID } @@ -189,7 +198,7 @@ export const performSnapCAPIv3 = async ( ): Promise> => { const { payload, settings } = data const { event_conversion_type } = payload - const authToken = data.auth?.accessToken + const authToken = emptyStringToUndefined(data.auth?.accessToken) raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined(authToken, 'Missing valid auth token') diff --git a/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/utils.ts b/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/utils.ts index 1948026c8d..86f15ae7f2 100644 --- a/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/utils.ts +++ b/packages/destination-actions/src/destinations/snap-conversions-api/reportConversionEvent/utils.ts @@ -77,5 +77,17 @@ export const splitListValueToArray = (input: string): readonly string[] | undefi return result.length > 0 ? result : undefined } -export const emptyStringToUndefined = (v: string | undefined): string | undefined => - (v ?? '').length > 0 ? v : undefined +export const emptyStringToUndefined = (v: string | undefined): string | undefined => { + const trimmed = v?.trim() + return (trimmed ?? '').length > 0 ? trimmed : undefined +} + +export const parseNumberSafe = (v: string | number | undefined): number | undefined => { + if (Number.isSafeInteger(v)) { + return v as number + } else if (v != null) { + const parsed = Number.parseInt(String(v) ?? '') + return Number.isSafeInteger(parsed) ? parsed : undefined + } + return undefined +}