Skip to content

Conversation

Prithviraj-rathore-segment
Copy link
Contributor

JIRA

https://twilio-engineering.atlassian.net/browse/STRATCONN-6111

A summary of your pull request, including the what change you're making and why.

Description

  1. This PR includes changes for braze browser destination for devicePropertyAllowList field in settings .
  2. Pass devicePropertyAllowList field in SDK initialization only in case of at least one non empty string is added from UI side .
  3. This change is done as per request done by Melio customer and for more information please refer above jira ticket.

Testing

Please find the testing document attached here .
https://docs.google.com/document/d/1h54Whz9omf3Kkz3Lg2b6vPBhy60PWROIbmB84mZ_n6Y/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 Aug 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.57%. Comparing base (67a677e) to head (a4e46cb).
⚠️ Report is 24 commits behind head on main.

❌ Your project status has failed because the head coverage (32.57%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3218       +/-   ##
===========================================
- Coverage   79.62%   32.57%   -47.06%     
===========================================
  Files        1177       14     -1163     
  Lines       21703      703    -21000     
  Branches     4220      118     -4102     
===========================================
- Hits        17282      229    -17053     
+ Misses       3679      474     -3205     
+ Partials      742        0      -742     

☔ 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 improves the Braze destination's handling of the devicePropertyAllowList setting by only passing it to the Braze SDK initialization when it contains at least one non-empty string value. This prevents the SDK from receiving empty arrays or arrays with only empty strings.

Key changes:

  • Added conditional logic to filter out empty devicePropertyAllowList arrays
  • Updated Braze SDK initialization to use the filtered configuration
  • Added unit tests to verify the devicePropertyAllowList handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/browser-destinations/destinations/braze/src/index.ts Added filtering logic for devicePropertyAllowList and updated SDK initialization to use filtered config
packages/browser-destinations/destinations/braze/src/tests/initialization.test.ts Added unit test to verify devicePropertyAllowList is properly passed to Braze SDK initialization

@joe-ayoub-segment joe-ayoub-segment merged commit b48eac1 into main Sep 9, 2025
13 of 14 checks passed
@joe-ayoub-segment joe-ayoub-segment deleted the STRATCONN-6111-braze-web-destination branch September 9, 2025 11:17
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.

5 participants