Skip to content

Conversation

joe-ayoub-segment
Copy link
Contributor

@joe-ayoub-segment joe-ayoub-segment commented Aug 27, 2025

PR to add FB Messenger support to Twilio Messaging Destination

Testing

Unit tests added
Testing in production with feature flag

Copy link
Contributor

New required fields detected

Warning

Your PR adds new required fields to an existing destination. Adding new required settings/mappings for a destination already in production requires updating existing customer destination configuration. Ignore this warning if this PR is for a new destination with no active customers in production.

The following required fields were added in this PR:

  • Destination: Twilio Messaging, Action Field(s):toMessengerUserId,fromFacebookPageId

Add these new fields as optional instead and assume default values in perform or performBatch block.

Copy link

codecov bot commented Aug 27, 2025

Codecov Report

❌ Patch coverage is 62.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.61%. Comparing base (300a848) to head (834e896).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...destinations/twilio-messaging/sendMessage/utils.ts 62.50% 2 Missing and 1 partial ⚠️
...ons/twilio-messaging/sendMessage/dynamic-fields.ts 71.42% 2 Missing ⚠️
...destinations/twilio-messaging/sendMessage/index.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (62.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3204      +/-   ##
==========================================
- Coverage   79.62%   79.61%   -0.02%     
==========================================
  Files        1177     1177              
  Lines       21703    21718      +15     
  Branches     4216     4225       +9     
==========================================
+ Hits        17282    17291       +9     
- Misses       3679     3684       +5     
- Partials      742      743       +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.

@joe-ayoub-segment joe-ayoub-segment marked this pull request as ready for review August 27, 2025 09:11
@joe-ayoub-segment joe-ayoub-segment requested a review from a team as a code owner August 27, 2025 09:11
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 adds Facebook Messenger support to the Twilio Messaging destination action. The changes enable sending messages through Facebook Messenger by introducing new channel options and required fields for Messenger-specific identifiers.

Key changes:

  • Added Facebook Messenger as a supported channel option with feature flag control
  • Introduced new required fields for Messenger user ID and Facebook page ID
  • Updated field validation logic to handle Messenger-specific requirements

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
utils.ts Added Messenger channel handling and Facebook page ID validation logic
index.ts Added dynamic channel field configuration
generated-types.ts Updated field names for Messenger user ID and Facebook page ID
fields.ts Made channel dynamic and added conditional requirements for Messenger fields
dynamic-fields.ts Added feature flag-controlled channel options and Messenger sender type
constants.ts Updated sender type constant from generic to Facebook-specific naming
index.test.ts Added comprehensive tests for Messenger functionality and error cases
dynamic-fields.test.ts Added tests for dynamic channel field behavior with feature flags

Comment on lines +87 to +88
{
fieldKey: 'channels',
Copy link
Preview

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The fieldKey 'channels' should be 'channel' (singular) to match the actual field name defined in the schema.

Suggested change
{
fieldKey: 'channels',
fieldKey: 'channel',

Copilot uses AI. Check for mistakes.

Comment on lines +147 to +148
{
fieldKey: 'channels',
Copy link
Preview

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The fieldKey 'channels' should be 'channel' (singular) to match the actual field name defined in the schema.

Suggested change
{
fieldKey: 'channels',
fieldKey: 'channel',

Copilot uses AI. Check for mistakes.

])
})

it('channel for Facebook Messenger channel option should hidden when flag enabled', async () => {
Copy link
Preview

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The test description contains a grammatical error. It should read 'should be hidden when flag is disabled' or 'should be hidden when flag is not enabled'.

Suggested change
it('channel for Facebook Messenger channel option should hidden when flag enabled', async () => {
it('channel for Facebook Messenger channel option should be hidden when flag is not enabled', async () => {

Copilot uses AI. Check for mistakes.

@varadarajan-tw
Copy link
Contributor

Just left couple of comments. Also, it seems like codecov/patch is failing because test coverage isn't sufficient enough. pls see if we can increase test coverage

@joe-ayoub-segment joe-ayoub-segment merged commit 3e68179 into main Sep 2, 2025
13 of 14 checks passed
@joe-ayoub-segment joe-ayoub-segment deleted the twilio-messaging-fb branch September 2, 2025 11:35
@joe-ayoub-segment
Copy link
Contributor Author

PR deployed

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