-
Notifications
You must be signed in to change notification settings - Fork 283
[MAIN][STRATCONN-6153] : [DV360] added feature flag for first party dv360 destination for upgrade version from v3 to v4 #3283
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
base: main
Are you sure you want to change the base?
Conversation
…360 version from v3 to v4
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3283 +/- ##
==========================================
+ Coverage 79.92% 80.00% +0.08%
==========================================
Files 1193 1202 +9
Lines 21898 22096 +198
Branches 4262 4352 +90
==========================================
+ Hits 17502 17679 +177
+ Misses 3643 3639 -4
- Partials 753 778 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR implements a feature flag migration from Google DV360 API v3 to v4 for the first-party DV360 destination. Google is deprecating the v3 API, requiring this upgrade to maintain functionality.
Key Changes
- Added feature flag
actions-first-party-dv360-version-update
to control v3/v4 API usage - Updated all API endpoints to conditionally use v4 (
firstPartyAndPartnerAudiences
) or v3 (firstAndThirdPartyAudiences
) based on feature flag - Comprehensive test coverage for both v3 and v4 API versions across all action types
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
File | Description |
---|---|
properties.ts |
Added feature flag constant definition |
index.ts |
Updated createAudience and getAudience to handle version-specific response fields |
functions.ts |
Modified API endpoint construction and response handling for v3/v4 compatibility |
addToAudContactInfo/index.ts |
Added features parameter to perform methods |
addToAudMobileDeviceId/index.ts |
Added features parameter to perform methods |
removeFromAudContactInfo/index.ts |
Added features parameter to perform methods |
removeFromAudMobileDeviceId/index.ts |
Added features parameter to perform methods |
Test files | Comprehensive test coverage for both API versions with feature flag scenarios |
}) | ||
|
||
describe('First-Party-dv360.removeToAudContactInfo', () => { | ||
describe('First-Party-dv360.addToAudContactInfo', () => { |
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.
The test describe block name should be 'removeFromAudContactInfo' to match the actual action being tested, not 'addToAudContactInfo'.
describe('First-Party-dv360.addToAudContactInfo', () => { | |
describe('First-Party-dv360.removeFromAudContactInfo', () => { |
Copilot uses AI. Check for mistakes.
.reply(200, { success: true }) | ||
|
||
const responses = await testDestination.testAction('removeFromAudContactInfo', { | ||
const responses = await testDestination.testAction('addToAudContactInfo', { |
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.
These test calls should use 'removeFromAudContactInfo' action instead of 'addToAudContactInfo' since this is the remove action test file.
const responses = await testDestination.testAction('addToAudContactInfo', { | |
const responses = await testDestination.testAction('removeFromAudContactInfo', { |
Copilot uses AI. Check for mistakes.
.reply(200, { success: true }) | ||
|
||
const responses = await testDestination.testAction('removeFromAudContactInfo', { | ||
const responses = await testDestination.testAction('addToAudContactInfo', { |
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.
These test calls should use 'removeFromAudContactInfo' action instead of 'addToAudContactInfo' since this is the remove action test file.
const responses = await testDestination.testAction('addToAudContactInfo', { | |
const responses = await testDestination.testAction('removeFromAudContactInfo', { |
Copilot uses AI. Check for mistakes.
.reply(200, { success: true }) | ||
|
||
const events = createBatchTestEvents(createContactList) | ||
const responses = await testDestination.testBatchAction('addToAudContactInfo', { |
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.
These test calls should use 'removeFromAudContactInfo' action instead of 'addToAudContactInfo' since this is the remove action test file.
Copilot uses AI. Check for mistakes.
.reply(200, { success: true }) | ||
|
||
const events = createBatchTestEvents(createContactList) | ||
const responses = await testDestination.testBatchAction('addToAudContactInfo', { |
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.
These test calls should use 'removeFromAudContactInfo' action instead of 'addToAudContactInfo' since this is the remove action test file.
Copilot uses AI. Check for mistakes.
|
||
expect(responses[0].options.body).toMatchInlineSnapshot( | ||
'"{\\"advertiserId\\":\\"1234567890\\",\\"removedContactInfoList\\":{\\"contactInfos\\":[{\\"hashedEmails\\":\\"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777\\",\\"hashedPhoneNumbers\\":\\"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8\\",\\"zipCodes\\":\\"12345\\",\\"hashedFirstName\\":\\"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a\\",\\"hashedLastName\\":\\"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f\\",\\"countryCode\\":\\"US\\"}],\\"consent\\":{\\"adUserData\\":\\"CONSENT_STATUS_GRANTED\\",\\"adPersonalization\\":\\"CONSENT_STATUS_GRANTED\\"}}}"' | ||
'"{\\"advertiserId\\":\\"1234567890\\",\\"addedContactInfoList\\":{\\"contactInfos\\":[{\\"hashedEmails\\":\\"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777\\",\\"hashedPhoneNumbers\\":\\"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8\\",\\"zipCodes\\":\\"12345\\",\\"hashedFirstName\\":\\"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a\\",\\"hashedLastName\\":\\"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f\\",\\"countryCode\\":\\"US\\"}],\\"consent\\":{\\"adUserData\\":\\"CONSENT_STATUS_GRANTED\\",\\"adPersonalization\\":\\"CONSENT_STATUS_GRANTED\\"}}}"' |
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.
The expected request body should contain 'removedContactInfoList' instead of 'addedContactInfoList' since this is testing the remove action.
Copilot uses AI. Check for mistakes.
expect(responses[0].options.body).toBe( | ||
'{"advertiserId":"1234567890","removedContactInfoList":{"contactInfos":[{"hashedEmails":"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777","hashedPhoneNumbers":"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8","zipCodes":"12345","hashedFirstName":"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a","hashedLastName":"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f","countryCode":"US"}],"consent":{"adUserData":"CONSENT_STATUS_GRANTED","adPersonalization":"CONSENT_STATUS_GRANTED"}}}' | ||
expect(responses[0].options.body).toMatchInlineSnapshot( | ||
'"{\\"advertiserId\\":\\"1234567890\\",\\"addedContactInfoList\\":{\\"contactInfos\\":[{\\"hashedEmails\\":\\"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777\\",\\"hashedPhoneNumbers\\":\\"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8\\",\\"zipCodes\\":\\"12345\\",\\"hashedFirstName\\":\\"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a\\",\\"hashedLastName\\":\\"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f\\",\\"countryCode\\":\\"US\\"}],\\"consent\\":{\\"adUserData\\":\\"CONSENT_STATUS_GRANTED\\",\\"adPersonalization\\":\\"CONSENT_STATUS_GRANTED\\"}}}"' |
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.
The expected request body should contain 'removedContactInfoList' instead of 'addedContactInfoList' since this is testing the remove action.
'"{\\"advertiserId\\":\\"1234567890\\",\\"addedContactInfoList\\":{\\"contactInfos\\":[{\\"hashedEmails\\":\\"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777\\",\\"hashedPhoneNumbers\\":\\"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8\\",\\"zipCodes\\":\\"12345\\",\\"hashedFirstName\\":\\"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a\\",\\"hashedLastName\\":\\"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f\\",\\"countryCode\\":\\"US\\"}],\\"consent\\":{\\"adUserData\\":\\"CONSENT_STATUS_GRANTED\\",\\"adPersonalization\\":\\"CONSENT_STATUS_GRANTED\\"}}}"' | |
'"{\\"advertiserId\\":\\"1234567890\\",\\"removedContactInfoList\\":{\\"contactInfos\\":[{\\"hashedEmails\\":\\"584c4423c421df49955759498a71495aba49b8780eb9387dff333b6f0982c777\\",\\"hashedPhoneNumbers\\":\\"422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8\\",\\"zipCodes\\":\\"12345\\",\\"hashedFirstName\\":\\"96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a\\",\\"hashedLastName\\":\\"799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f\\",\\"countryCode\\":\\"US\\"}],\\"consent\\":{\\"adUserData\\":\\"CONSENT_STATUS_GRANTED\\",\\"adPersonalization\\":\\"CONSENT_STATUS_GRANTED\\"}}}"' |
Copilot uses AI. Check for mistakes.
expect(requestBody.addedContactInfoList.contactInfos.length).toBe(2) | ||
expect(requestBody.addedContactInfoList.contactInfos[0].hashedEmails).toBeDefined() | ||
expect(requestBody.addedContactInfoList.contactInfos[1].hashedEmails).toBeDefined() |
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.
These assertions should check 'removedContactInfoList' instead of 'addedContactInfoList' since this is testing the remove action.
Copilot uses AI. Check for mistakes.
expect(requestBody.addedContactInfoList.contactInfos.length).toBe(2) | ||
expect(requestBody.addedContactInfoList.contactInfos[0].hashedEmails).toBeDefined() | ||
expect(requestBody.addedContactInfoList.contactInfos[1].hashedEmails).toBeDefined() |
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.
These assertions should check 'removedContactInfoList' instead of 'addedContactInfoList' since this is testing the remove action.
Copilot uses AI. Check for mistakes.
expect(requestBody.addedContactInfoList.contactInfos.length).toBe(2) | ||
expect(requestBody.addedContactInfoList.contactInfos[0].hashedEmails).toBeDefined() | ||
expect(requestBody.addedContactInfoList.contactInfos[1].hashedEmails).toBeDefined() |
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.
These assertions should check 'removedContactInfoList' instead of 'addedContactInfoList' since this is testing the remove action.
expect(requestBody.addedContactInfoList.contactInfos.length).toBe(2) | |
expect(requestBody.addedContactInfoList.contactInfos[0].hashedEmails).toBeDefined() | |
expect(requestBody.addedContactInfoList.contactInfos[1].hashedEmails).toBeDefined() | |
expect(requestBody.removedContactInfoList.contactInfos.length).toBe(2) | |
expect(requestBody.removedContactInfoList.contactInfos[0].hashedEmails).toBeDefined() | |
expect(requestBody.removedContactInfoList.contactInfos[1].hashedEmails).toBeDefined() |
Copilot uses AI. Check for mistakes.
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.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
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.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
}) | ||
|
||
describe('First-Party-dv360.removeToAudContactInfo', () => { | ||
describe('First-Party-dv360.removeFromAudContactInfo', () => { |
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.
The test description was corrected from 'removeToAudContactInfo' to 'removeFromAudContactInfo' to match the actual action name.
Copilot uses AI. Check for mistakes.
JIRA
https://twilio-engineering.atlassian.net/browse/STRATCONN-6153
A summary of your pull request, including the what change you're making and why.
Description
Testing
Please find the testing document attached here .
https://docs.google.com/document/d/1W8CsIjAtOyP27YrgfyHMLXSz5V2QvffG7t91wo05-QY/edit?tab=t.0
Flag created on production -
https://flagon.segment.com/families/centrifuge-destinations/gates/actions-first-party-dv360-version-update
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.