Skip to content

Conversation

Prithviraj-rathore-segment
Copy link
Contributor

@Prithviraj-rathore-segment Prithviraj-rathore-segment commented Sep 30, 2025

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

  1. This PR includes changes for migration from v3 to v4 for first party DV360 action destination with feature flag and API_VERSION AND CANARY_API_VERSION with code refactoring .
    Code changes are done based on this comment
    [MAIN][STRATCONN-6153] : [DV360] added feature flag for first party dv360 destination for upgrade version from v3 to v4  #3283 (review)

  2. Updated the unit test cases to v4 from v3 in first party action destination with CANARY_API_VERSION .

  3. Google is depreciating the v3 version of dv360 hence the changes are done .

Reason of doing changes

Screenshot 2025-09-30 at 12 19 49 PM Screenshot 2025-09-30 at 12 22 05 PM

Testing

Please find the testing document attached here .
https://docs.google.com/document/d/1GIxrjRqm5ExkAC_1uNZwCBNNi6Jv9_gfC2xmQQ1JOx0/edit?tab=t.0

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.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.08%. Comparing base (7710428) to head (1c703b4).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3325      +/-   ##
==========================================
+ Coverage   80.03%   80.08%   +0.05%     
==========================================
  Files        1202     1203       +1     
  Lines       22105    22136      +31     
  Branches     4355     4360       +5     
==========================================
+ Hits        17691    17728      +37     
+ Misses       3637     3630       -7     
- Partials      777      778       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@Copilot Copilot AI left a 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 refactors the first-party DV360 destination to support API version migration from v3 to v4 using feature flags. The changes introduce structured API version management through constants and centralized functions while maintaining backward compatibility.

Key changes:

  • Introduces API_VERSION, CANARY_API_VERSION constants and getApiVersion() function for centralized version management
  • Updates feature flag name from actions-first-party-dv360-version-update to first-party-dv360-canary-version
  • Refactors endpoint URL construction into reusable helper functions

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/destination-actions/src/destinations/first-party-dv360/functions.ts Added version constants, centralized API version logic, and refactored endpoint construction functions
packages/destination-actions/src/destinations/first-party-dv360/index.ts Updated to use new version management functions and feature flag name
packages/destination-actions/src/destinations/first-party-dv360/tests/index.test.ts Updated test descriptions and feature flag references to reflect new naming
packages/destination-actions/src/destinations/first-party-dv360/addToAudContactInfo/tests/index.test.ts Updated test descriptions and feature flag references
packages/destination-actions/src/destinations/first-party-dv360/addToAudMobileDeviceId/tests/index.test.ts Updated test descriptions and feature flag references
packages/destination-actions/src/destinations/first-party-dv360/removeFromAudContactInfo/tests/index.test.ts Updated test descriptions and feature flag references

Copy link
Contributor

@Copilot Copilot AI left a 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 6 out of 6 changed files in this pull request and generated no new comments.

Copy link
Contributor

@Copilot Copilot AI left a 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 7 out of 7 changed files in this pull request and generated 1 comment.

Comment on lines +17 to +18
tags?.push(`version:${version}`)
statsClient?.incr('dv360_api_version', 1, tags)
Copy link
Preview

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function modifies the tags array parameter by reference, which could cause side effects for callers. Consider creating a copy of the tags array before modification: const newTags = [...(tags || []), \version:${version}`]and usenewTags` for the stats call.

Suggested change
tags?.push(`version:${version}`)
statsClient?.incr('dv360_api_version', 1, tags)
const newTags = [...(tags || []), `version:${version}`]
statsClient?.incr('dv360_api_version', 1, newTags)

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@Copilot Copilot AI left a 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 7 out of 7 changed files in this pull request and generated no new comments.

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

Successfully merging this pull request may close these issues.

2 participants