-
Notifications
You must be signed in to change notification settings - Fork 0
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
(ITE-146) Update Segment Engage Destination to handle onDelete events #11
Conversation
const formattedExternalIds = `["${userId}"]` | ||
|
||
const syncIds = [sha256hash(String(userId))] | ||
const formattedSyncIds = `[${syncIds.map((syncId: string) => `"${syncId}"`).join(', ')}]` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, we know that there will always only be one syncId, so why put it into an array and map it if the outcome will always just be "${syncId}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's correct, this was leftover logic from before, I will fix it
|
||
const mutation = `mutation { | ||
deleteProfilesWithExternalIds( | ||
externalIds: ${formattedExternalIds}, | ||
externalProvider: "${EXTERNAL_PROVIDER}", | ||
syncIds: ${formattedSyncIds} | ||
syncIds: ${syncId} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the brackets are preserved when you convert an array to a string, if you want that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should be called like syncId: "28test", so I adjusted the format to that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but what is the purpose of the syncId variable still being an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I'll fix it as well
Update Segment Engage Destination to handle onDelete events
A logic rewrite after the new graphQL API.
A summary of your pull request, including the what change you're making and why.
Testing
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.