[PE-102] Add Snap Audience destination

[tcgilbert]

This pull request adds a new Snap Audience destination.

• planning doc

Testing

• ✅ Added unit tests for new functionality
• ✅ Tested end-to-end using the local server

[github-actions]

New required fields detected

Warning

Your PR adds new required fields to an existing destination. Adding new required settings/mappings for a destination already in production requires updating existing customer destination configuration. Ignore this warning if this PR is for a new destination with no active customers in production.

The following required fields were added in this PR:

• Destination: Snap Audiences (Actions), Settings:settings, Fields:ad_account_id
• Destination: Snap Audiences (Actions), Action:syncAudience, Fields:external_audience_id,audienceKey,traits_or_props,schema_type,enable_batching

Add these new fields as optional instead and assume default values in perform or performBatch block.

[joe-ayoub-segment]

Thanks for the PR @tcgilbert !

I did a quick review but would like to catch up in person to walk through the details. Most of my comments are 'nits':

1. customAudienceName or audienceName fields - how will this work in the UI? WIll the customer only see the customAudienceName field? If so, what value populates the audienceName field in the createAudience function?

2. Nit: Maybe we could define types for the request and responses for the createAudience and getAudience functions? It's not something I've done in the past, but have started doing it recently and it helps with code clarify.

3. Should we go with oauth-managed or oauth2? oauth-managed is where the oauth config is directly added into the Developer Portal. oauth2 request that we manually add these values into chamber.

4. Nit: We could simplify the code by throwing errors directly from the validateAndExtractIdentifier function.

5. Nit: normalizeAndHashEmail and normalizeAndHashMobileId do the same thing. Could be merged together.

6. Nit: A nice pattern to use is to have a single function which is able to handle both input from perform() and performBatch. Essentially the code will handle an array of payloads regardless of whether or not it was called from perform() or performBatch(). This means there's only one code path to maintain and test. You could still check for multiple payloads and throw a batchErrorMessage if you like.

7. Nit: We could defined a type for request and responses for adding/removing users from an Audience.

8. I'm not sure if I understand how collecting the advertising_id will work? The default mapping is to context.device.advertisingId but then we're also asking the customer to configure ID enrichment for ios.idfa and android.idfa ?

[tcgilbert]

• customAudienceName or audienceName fields - how will this work in the UI? WIll the customer only see the customAudienceName field? If so, what value populates the audienceName field in the createAudience function?

This just gives the option for the user to use a name that is different from the Segment Audience name if they want. Tbh not sure if that is a valid need, so okay with removing.

• Nit: Maybe we could define types for the request and responses for the createAudience and getAudience functions? It's not something I've done in the past, but have started doing it recently and it helps with code clarify.

Done.

• Should we go with oauth-managed or oauth2? oauth-managed is where the oauth config is directly added into the Developer Portal. oauth2 request that we manually add these values into chamber.

Was thinking oauth2 since we created it

• Nit: We could simplify the code by throwing errors directly from the validateAndExtractIdentifier function.

Have a couple questions on this as this function is used in batching too. Can talk it through

• Nit: normalizeAndHashEmail and normalizeAndHashMobileId do the same thing. Could be merged together.

Done

• Nit: A nice pattern to use is to have a single function which is able to handle both input from perform() and performBatch. Essentially the code will handle an array of payloads regardless of whether or not it was called from perform() or performBatch(). This means there's only one code path to maintain and test. You could still check for multiple payloads and throw a batchErrorMessage if you like.

Yeah this is good feedback. I've updated to use this format

• Nit: We could defined a type for request and responses for adding/removing users from an Audience.

done

• I'm not sure if I understand how collecting the advertising_id will work? The default mapping is to context.device.advertisingId but then we're also asking the customer to configure ID enrichment for ios.idfa and android.idfa ?

This has to do with the . notation used for properties, can talk through what would be ideal here

[codecov]

Codecov Report

Attention: Patch coverage is 87.80488% with 10 lines in your changes missing coverage. Please review.

Project coverage is 77.99%. Comparing base (5a5f52b) to head (8152922).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
...n-actions/src/destinations/snap-audiences/index.ts	63.63%	6 Missing and 2 partials ⚠️
.../destinations/snap-audiences/syncAudience/utils.ts	95.34%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2474       +/-   ##
===========================================
+ Coverage   33.18%   77.99%   +44.80%     
===========================================
  Files          14      985      +971     
  Lines         693    17141    +16448     
  Branches      109     3226     +3117     
===========================================
+ Hits          230    13369    +13139     
- Misses        463     2700     +2237     
- Partials        0     1072     +1072     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.