Skip to content

Commit

Permalink
Snap misc fixes (#1900)
Browse files Browse the repository at this point in the history
* set the app_id to undefined if empty. Set advertiser_tracking_enabled to 0 if the app_id is defined

* add comment on advertiser_tracking_enabled field usage

* tweak app_data and extinfo build

* update box function to return undefined if the input string is empty or undefined instead of an empty array

* fix tests

* fix typo

* simplify action_source logic

* update tests to validate advertiser_tracking_enabled and changes to box function

* cleaner implementation of emptyStringtoUndefined

* simpler implementation of box

* remove duplicate util function

* fix function name

* log errors as critical

* add prefix to logger entries

---------

Co-authored-by: David Bordoley <dbordoley@snapchat.com>
  • Loading branch information
bordoley and David Bordoley authored Mar 5, 2024
1 parent b8439ca commit e0a5d6b
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export const capiV3tests = () =>
const { integration, event_name, event_time, user_data, custom_data, action_source, app_data } = data[0]
const { em, ph } = user_data
const { brands, content_category, content_ids, currency, num_items, value } = custom_data
const { app_id } = app_data

expect(integration).toBe('segment')
expect(event_name).toBe('PURCHASE')
Expand All @@ -93,7 +92,8 @@ export const capiV3tests = () =>
expect(currency).toBe('USD')
expect(value).toBe(15)
expect(action_source).toBe('website')
expect(app_id).toBe('test123')
// app_data is only defined when action_source is app
expect(app_data).toBeUndefined()

expect(brands).toEqual(['Hasbro', 'Mattel'])
expect(content_category).toEqual(['games', 'games'])
Expand Down Expand Up @@ -133,7 +133,6 @@ export const capiV3tests = () =>
data[0]
const { client_ip_address, client_user_agent, em, ph } = user_data
const { currency, value } = custom_data
const { app_id } = app_data

expect(integration).toBe('segment')
expect(event_name).toBe('PURCHASE')
Expand All @@ -148,7 +147,8 @@ export const capiV3tests = () =>
expect(currency).toBe('USD')
expect(value).toBe(15)
expect(action_source).toBe('website')
expect(app_id).toBe('test123')
// app_data is only defined when action_source is app
expect(app_data).toBeUndefined()
})

it('should fail web event without pixel_id', async () => {
Expand Down Expand Up @@ -234,7 +234,6 @@ export const capiV3tests = () =>
data[0]
const { client_ip_address, client_user_agent, em, ph } = user_data
const { currency, value } = custom_data
const { app_id } = app_data

expect(integration).toBe('segment')
expect(event_name).toBe('SAVE')
Expand All @@ -248,8 +247,10 @@ export const capiV3tests = () =>
expect(ph[0]).toBe('dc008fda46e2e64002cf2f82a4906236282d431c4f75e5b60bfe79fc48546383')
expect(currency).toBe('USD')
expect(value).toBe(15)
expect(action_source).toBe('other')
expect(app_id).toBe('test123')
expect(action_source).toBe('OFFLINE')

// App data is only defined for app events
expect(app_data).toBeUndefined()
})

it('should handle a mobile app event conversion type', async () => {
Expand Down Expand Up @@ -288,6 +289,7 @@ export const capiV3tests = () =>
data[0]
const { client_ip_address, client_user_agent, em, ph } = user_data
const { currency, value } = custom_data
const { extinfo, advertiser_tracking_enabled } = app_data

expect(integration).toBe('segment')
expect(event_name).toBe('SAVE')
Expand All @@ -302,7 +304,8 @@ export const capiV3tests = () =>
expect(currency).toBe('USD')
expect(value).toBe(15)
expect(action_source).toBe('app')
expect(app_data.extinfo).toEqual(['i2', '', '', '', '17.2', 'iPhone12,1', '', '', '', '', '', '', '', '', '', ''])
expect(extinfo).toEqual(['i2', '', '', '', '17.2', 'iPhone12,1', '', '', '', '', '', '', '', '', '', ''])
expect(advertiser_tracking_enabled).toBe(0)
})

it('should fail invalid currency', async () => {
Expand Down Expand Up @@ -393,7 +396,7 @@ export const capiV3tests = () =>
data[0]
const { client_ip_address, client_user_agent, em, ph } = user_data
const { currency, value } = custom_data
const { app_id } = app_data
const { app_id, advertiser_tracking_enabled } = app_data

expect(integration).toBe('segment')
expect(event_name).toBe('CUSTOM_EVENT_5')
Expand All @@ -409,6 +412,7 @@ export const capiV3tests = () =>
expect(value).toBe(15)
expect(action_source).toBe('app')
expect(app_id).toBe('123')
expect(advertiser_tracking_enabled).toBe(0)
})

it('should fail event missing all Snap identifiers', async () => {
Expand Down Expand Up @@ -472,12 +476,13 @@ export const capiV3tests = () =>
expect(data.length).toBe(1)

const { integration, event_name, event_time, user_data, action_source } = data[0]
const { em } = user_data
const { em, ph } = user_data

expect(integration).toBe('segment')
expect(event_name).toBe('PURCHASE')
expect(event_time).toBe(1652368875449)
expect(em[0]).toBe('cc779c04191c2e736d89e45c11339c8382832bcaf70383f7df94e3d08ba7a6d9')
expect(ph).toBeUndefined()
expect(action_source).toBe('website')
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const action: ActionDefinition<Settings, Payload> = {
// record. Instead log the errors so that we can identify issues and resolve them.

// FIXME: Should we add sampling here?
data.logger?.info(String(e))
data.logger?.crit(`snap-capi-v3\n\n${String(e)}`)
}
})()
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { Settings } from '../generated-types'
import {
box,
emptyObjectToUndefined,
emptyToUndefined,
hash,
hashEmailSafe,
isNullOrUndefined,
splitListValueToArray,
raiseMisconfiguredRequiredFieldErrorIf,
raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined
raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined,
emptyStringToUndefined
} from './utils'
import { CURRENCY_ISO_4217_CODES } from '../snap-capi-properties'

Expand All @@ -33,15 +33,22 @@ export const validatePayload = (payload: Payload): Payload => {

const eventConversionTypeToActionSource: { [k in string]?: string } = {
WEB: 'website',
MOBILE_APP: 'app'
MOBILE_APP: 'app',

// Use the snap event_conversion_type for offline events
OFFLINE: 'OFFLINE'
}

const iosAppIDRegex = new RegExp('^[0-9]+$')

export const formatPayload = (payload: Payload, settings: Settings, isTest = true): object => {
const action_source = eventConversionTypeToActionSource[payload.event_conversion_type] ?? 'other'
const app_id = emptyStringToUndefined(settings.app_id)

// event_conversion_type is a required parameter whose value is enforced as
// always OFFLINE, WEB, or MOBILE_APP, so in practice action_source will always have a value.
const action_source = eventConversionTypeToActionSource[payload.event_conversion_type]

const event_id = emptyToUndefined(payload.client_dedup_id)
const event_id = emptyStringToUndefined(payload.client_dedup_id)

// Removes all leading and trailing whitespace and converts all characters to lowercase.
const email = hashEmailSafe(payload.email?.replace(/\s/g, '').toLowerCase())
Expand Down Expand Up @@ -70,9 +77,44 @@ export const formatPayload = (payload: Payload, settings: Settings, isTest = tru
num_items: payload.number_items
}

const { app_id } = settings
// 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)
// for any events sent that include app_data.
const advertiser_tracking_enabled = !isNullOrUndefined(app_id) ? 0 : undefined
const extInfoVersion = iosAppIDRegex.test((app_id ?? '').trim()) ? 'i2' : 'a2'

// extinfo needs to be defined whenever app_data is included in the data payload
const extinfo = !isNullOrUndefined(app_id)
? [
extInfoVersion, // required per spec version must be a2 for Android, must be i2 for iOS
'', // app package name
'', // short version
'', // long version
payload.os_version ?? '', // os version
payload.device_model ?? '', // device model name
'', // local
'', // timezone abbr
'', // carrier
'', //screen width
'', // screen height
'', // screen density
'', // cpu core
'', // external storage size
'', // freespace in external storage size
'' // device time zone
]
: undefined

// Only set app data for app events
const app_data =
action_source === 'app'
? emptyObjectToUndefined({
app_id,
advertiser_tracking_enabled,
extinfo
})
: undefined

const result = {
data: [
{
Expand Down Expand Up @@ -100,37 +142,14 @@ export const formatPayload = (payload: Payload, settings: Settings, isTest = tru
content_ids,
currency: payload.currency,
num_items,
order_id: emptyToUndefined(payload.transaction_id),
order_id: emptyStringToUndefined(payload.transaction_id),
search_string: payload.search_string,
sign_up_method: payload.sign_up_method,
value: payload.price
}),

action_source,

app_data: emptyObjectToUndefined({
app_id,
extinfo: !isNullOrUndefined(payload.os_version ?? payload.device_model)
? [
extInfoVersion, // required per spec version must be a2 for Android, must be i2 for iOS
'', // app package name
'', // short version
'', // long version
payload.os_version ?? '', // os version
payload.device_model ?? '', // device model name
'', // local
'', // timezone abbr
'', // carrier
'', //screen width
'', // screen height
'', // screen density
'', // cpu core
'', // external storage size
'', // freespace in external storage size
'' // device time zone
]
: undefined
})
app_data
}
],
...(isTest ? { test_event_code: 'segment_test' } : {})
Expand All @@ -141,8 +160,8 @@ export const formatPayload = (payload: Payload, settings: Settings, isTest = tru

export const validateAppOrPixelID = (settings: Settings, event_conversion_type: string): string => {
const { snap_app_id, pixel_id } = settings
const snapAppID = emptyToUndefined(snap_app_id)
const snapPixelID = emptyToUndefined(pixel_id)
const snapAppID = emptyStringToUndefined(snap_app_id)
const snapPixelID = emptyStringToUndefined(pixel_id)
const appOrPixelID = snapAppID ?? snapPixelID

raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined(appOrPixelID, 'Missing valid app or pixel ID')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ export const transformProperty = (
export const hashEmailSafe = (email: string | undefined): string | undefined =>
isHashedEmail(String(email)) ? email : hash(email)

export const emptyToUndefined = (str: string | undefined): string | undefined =>
str != null && str === '' ? undefined : str

export const raiseMisconfiguredRequiredFieldErrorIf = (condition: boolean, message: string) => {
if (condition) {
throw new IntegrationError(message, 'Misconfigured required field', 400)
Expand All @@ -48,7 +45,8 @@ export const raiseMisconfiguredRequiredFieldErrorIfNullOrUndefined: S['raiseMisc
<T>(v: T | undefined, message: string): asserts v is T =>
raiseMisconfiguredRequiredFieldErrorIf(isNullOrUndefined(v), message)

export const box = <T>(v: T | undefined): readonly T[] => (!isNullOrUndefined(v) ? [v] : [])
export const box = (v: string | undefined): readonly string[] | undefined =>
(v ?? '').length > 0 ? [v as string] : undefined

export const emptyObjectToUndefined = (v: { [k in string]?: unknown }) => {
const properties = Object.getOwnPropertyNames(v)
Expand Down Expand Up @@ -78,3 +76,6 @@ 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

0 comments on commit e0a5d6b

Please sign in to comment.