[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 <dbordoley@snapchat.com>

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()
+    })
   })

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<ModifiedResponse<unknown>> => {
   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')
 

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
+}