feat: onboarding customerio segment #1853
Conversation
shrouti1507
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
ruchiramoitra,
ItsSudip,
sandeepdsvs,
krishna2020 and
1abhishekpandey
as code owners
WalkthroughA new destination configuration for Customer.io Segment has been introduced, involving three JSON configuration files: a database configuration, a schema validation file, and a user interface configuration. These files collectively define the integration settings, connection parameters, and UI representation for the Customer.io Segment destination. The configuration supports advanced features like consent management, connection modes, and detailed validation rules for various input parameters. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1853 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 53 53
Branches 7 7
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/configurations/destinations/customerio_segment/db-config.json (1)
5-11: Consider adding data validation feature flagThe destination handles sensitive API keys and customer data. Consider adding the
validateDataFormatfeature flag for enhanced data validation."features": [ - "vdm-next" + "vdm-next", + "validateDataFormat" ],src/configurations/destinations/customerio_segment/schema.json (1)
74-80: Add pattern validation for resolutionStrategyThe
resolutionStrategyfield lacks pattern validation for environment variables.Add pattern validation:
"resolutionStrategy": { "type": "string", - "enum": ["and", "or"] + "enum": ["and", "or"], + "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(and|or)$" }src/configurations/destinations/customerio_segment/ui-config.json (2)
138-146: Simplify duplicate feature flag conditionsThe feature flags array contains duplicate entries with different values.
Simplify the conditions:
"featureFlags": [ - { - "configKey": "AMP_enable-gcm", - "value": false - }, - { - "configKey": "AMP_enable-gcm" - } + { + "configKey": "AMP_enable-gcm", + "value": false + } ],
22-22: Standardize placeholder formatThe placeholder format is inconsistent across fields. Some use "e.g:" while others use "e.g.".
Standardize the format:
- "placeholder": "e.g: 88f02580XXXXXXXcf18", + "placeholder": "e.g., 88f02580XXXXXXXcf18",Also applies to: 32-32, 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/configurations/destinations/customerio_segment/db-config.json(1 hunks)src/configurations/destinations/customerio_segment/schema.json(1 hunks)src/configurations/destinations/customerio_segment/ui-config.json(1 hunks)
| "ketchConsentPurposes" | ||
| ] | ||
| }, | ||
| "secretKeys": ["appApiKey", "apiKey"] |
There was a problem hiding this comment.
💡 Codebase verification
Add siteId to secretKeys array for consistent security configuration
Based on the UI configuration, siteId is marked with "secret": true in the UI, but is missing from the secretKeys array in the db-config.json. This inconsistency should be fixed by adding siteId to maintain proper security handling across the configuration.
src/configurations/destinations/customerio_segment/db-config.json: AddsiteIdto thesecretKeysarray
🔗 Analysis chain
Verify consistency of secret keys configuration
The secretKeys array includes appApiKey and apiKey but excludes siteId, which is marked as secret in the UI configuration.
Add siteId to the secretKeys array:
- "secretKeys": ["appApiKey", "apiKey"]
+ "secretKeys": ["appApiKey", "apiKey", "siteId"]Let me verify the consistency between secret keys configuration and UI fields by searching through the codebase.
Let me search specifically for UI configuration that marks the siteId as secret.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for UI configuration files related to customerio_segment
fd customerio_segment --type f
# Search for siteId configuration in the found files
rg "siteId" --type json
Length of output: 8732
Script:
#!/bin/bash
# Check UI configuration for customerio_segment
cat src/configurations/destinations/customerio_segment/ui-config.json
Length of output: 8861
| "type": "string", | ||
| "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,500})$" | ||
| }, |
There was a problem hiding this comment.
Fix inconsistent pattern length validation
The pattern for apiKey allows up to 500 characters, but the UI configuration limits it to 100 characters.
Update the pattern to match UI constraints:
- "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,500})$"
+ "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "type": "string", | |
| "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,500})$" | |
| }, | |
| "type": "string", | |
| "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$" | |
| }, |
| "note": "Settings > Account Settings > API Credentials > Track APP Keys > API KEY of the corresponding Site ID", | ||
| "configKey": "apiKey", | ||
| "regex": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$", | ||
| "regexErrorMessage": "Invalid Ad Account id", |
There was a problem hiding this comment.
Fix incorrect error messages
The error messages for apiKey and appApiKey incorrectly reference "Ad Account id".
Update the error messages:
- "regexErrorMessage": "Invalid Ad Account id",
+ "regexErrorMessage": "Invalid API Key",Also applies to: 41-41
| { | ||
| "type": "textInput", | ||
| "label": "App Secret", | ||
| "note": "Provide App Secret from your Facebook Developer App's basic settings.", |
There was a problem hiding this comment.
Remove incorrect field note
The note for appSecret incorrectly references Facebook Developer App settings.
Remove or update the note:
- "note": "Provide App Secret from your Facebook Developer App's basic settings.",
+ "note": "Provide the App Secret from your Customer.io settings.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "note": "Provide App Secret from your Facebook Developer App's basic settings.", | |
| "note": "Provide the App Secret from your Customer.io settings.", |
| "message": "Cloud mode for this destination will be deprecated soon. Please migrate to Facebook Conversions destination for cloud mode.", | ||
| "type": "info" |
There was a problem hiding this comment.
Update incorrect deprecation message
The deprecation message incorrectly references Facebook Conversions destination.
Fix the message:
- "message": "Cloud mode for this destination will be deprecated soon. Please migrate to Facebook Conversions destination for cloud mode.",
+ "message": "Cloud mode for this destination will be deprecated soon.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "message": "Cloud mode for this destination will be deprecated soon. Please migrate to Facebook Conversions destination for cloud mode.", | |
| "type": "info" | |
| "message": "Cloud mode for this destination will be deprecated soon.", | |
| "type": "info" |
What are the changes introduced in this PR?
Write a brief explainer on your code changes.
What is the related Linear task?
Resolves INT-XXX
Please explain the objectives of your changes below
Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new checks got introduced or modified in test suites. Please explain the changes.
N/A
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Documentation