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

Update MC contacts upsert to handle multiple identifiers #2032

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

msaunders-twilio
Copy link
Contributor

@msaunders-twilio msaunders-twilio commented May 13, 2024

Sendgrid Marketinc Campaigns (MC) recently updated our contacts system to allow for more identifier fields. Previously we required primary_email for each contact as the unique identifier. With the recent rollout of multiple identifiers, we added the identifier fields phone_number_id, external_id, and anonymous_id. Link to our API docs showing this change.

Now, only one of these four identifier fields is required to send a contact to Sendgrid MC. With this change, we also allow for the primary_email field to be null if one of the other three identifiers is included.

To accommodate for multiple identifiers in the MC action destination, I have made the following changes:

  • Added the new identifier fields phone_number_id, external_id, and anonymous_id
  • Made the primary_email field optional now
  • Added a check to validate the destination payload contains at least one of the four identifier fields

Testing

I tested this locally by sending sample events to my test MC account. I also added a few more test cases to our unit tests for this destionation.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [Segmenters] Tested in the staging environment

@joe-ayoub-segment
Copy link
Contributor

joe-ayoub-segment commented May 23, 2024

Hi @msaunders-twilio thanks for raising this PR.

Can you tell me where the input payload will come from for please?
Are they coming from a regular Connections Destination or Engage?

If so then the usual location for an anonymousId will be at the root of the payload.

'@path': '$.anonymousId'

Regarding phone_number_id, is this supposed to be the user's actual phone number, or something else?

If it is supposed to be the user's regular phone number then it will normally be ingested at $.traits.phone or $.context.traits.phone . Sometimes you'll see it in $.properties.phone.

Also can you explain what the external_id field is for please? What types of values are expected to populate this field?

Cheers,
Joe

@joe-ayoub-segment joe-ayoub-segment self-assigned this May 23, 2024
@msaunders-twilio
Copy link
Contributor Author

Hi @joe-ayoub-segment ,

anonymous_id and phone_number_id

Good to know. I will make the update to use the usual Segment fields by default.

external_id

This field is there for the user to put whatever id they want so long as it is unique. This might be an id they use for a user in their system.

@joe-ayoub-segment
Copy link
Contributor

hi @msaunders-twilio just another minor change.
Is this Destination in use yet?

@msaunders-twilio
Copy link
Contributor Author

hi @msaunders-twilio just another minor change. Is this Destination in use yet?

Yes, this destination is currently in use by customers.

@joe-ayoub-segment
Copy link
Contributor

joe-ayoub-segment commented May 27, 2024

hi @msaunders-twilio the mapping should be to $.anonymousId and not $.anonymous_id.
Think that was my bad from the previous comment.

@msaunders-twilio
Copy link
Contributor Author

Hi @joe-ayoub-segment 👋 Good catch! I updated that default field and am ready for another review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants