feat: onboard linkedin audience destination #1758
Conversation
ItsSudip
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
AchuthaSourabhC,
ruchiramoitra,
shrouti1507,
sandeepdsvs,
krishna2020 and
1abhishekpandey
as code owners
WalkthroughThree new JSON configuration files have been added for the LinkedIn Audience destination. The Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/configurations/destinations/linkedin_audience/db-config.json (2)
1-3: Correct the capitalization in the display name.The display name "Linkedin Audience" should have consistent capitalization.
Apply this change:
- "displayName": "Linkedin Audience", + "displayName": "LinkedIn Audience",
28-30: Consider adding more details about the beta status.The configuration correctly indicates that the LinkedIn Audience integration is in beta. Consider adding more information about the beta status, such as known limitations or an expected timeline for full release, to help users understand what to expect.
For example, you could expand the options section like this:
"options": { "isBeta": true, "betaInfo": "This integration is in beta. Some features may be limited or subject to change. Expected full release: Q4 2024." }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/configurations/destinations/linkedin_audience/db-config.json (1 hunks)
- src/configurations/destinations/linkedin_audience/ui-config.json (1 hunks)
🧰 Additional context used
🔇 Additional comments (6)
src/configurations/destinations/linkedin_audience/db-config.json (2)
21-26: Verify the empty "secretKeys" array.The "secretKeys" array is currently empty. Please confirm if this is intentional or if there are any secret keys that should be included for the LinkedIn Audience integration.
#!/bin/bash # Description: Check other LinkedIn configurations for secret keys patterns. # Test: Search for LinkedIn configurations with non-empty secretKeys rg -g '*.json' -C 5 '"secretKeys":\s*\[(?!\s*\])' src/configurations/destinations/linkedin*
13-20: Verify supported message types for warehouse sources.The configuration specifies supported message types for cloud sources but not for warehouse sources. Please confirm if this is intentional or if warehouse sources should also support certain message types.
src/configurations/destinations/linkedin_audience/ui-config.json (4)
1-35: LGTM: Overall structure is well-organized.The JSON structure is valid and follows a logical organization for UI configuration. The use of a "baseTemplate" array allows for potential future expansion if needed.
4-14: Verify the purpose of the empty fields array in the initial setup.The initial setup section is well-structured with a clear title and guidance note. However, there's an empty fields array that might be intended for future use.
Could you confirm if the empty fields array is intentional? If so, consider adding a comment explaining its future purpose. If not, you may want to remove it to keep the configuration concise.
15-30: Verify the purpose of the empty fields array in the connection mode section.The connection mode section is well-structured with a clear title, informative note, and helpful link. The use of an icon enhances the user interface. However, there's an empty fields array that might be intended for future use.
Could you confirm if the empty fields array is intentional? If so, consider adding a comment explaining its future purpose. If not, you may want to remove it to keep the configuration concise.
11-11: Clarify the absence of field definitions in the configuration.The configuration currently lacks any field definitions, which is unusual for a UI configuration. This might indicate that the configuration is incomplete or that fields are defined elsewhere.
Could you please clarify:
- Are field definitions intentionally omitted from this file?
- If so, where are the actual field definitions located?
- If not, should we add the necessary field definitions to complete the configuration?
Also applies to: 27-27
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/configurations/destinations/linkedin_audience/ui-config.json (1)
5-6: Enhance setup guidance noteThe current note "Update your connection setting here" is too generic. Consider providing more specific guidance about what settings users need to configure for LinkedIn Audience integration.
- "note": "Update your connection setting here", + "note": "Configure your LinkedIn Audience connection settings, including authentication and data mapping preferences",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/configurations/destinations/linkedin_audience/db-config.json(1 hunks)src/configurations/destinations/linkedin_audience/ui-config.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/configurations/destinations/linkedin_audience/db-config.json
🔇 Additional comments (3)
src/configurations/destinations/linkedin_audience/ui-config.json (3)
1-40: Security review passed
The configuration file:
- Contains no exposed credentials or sensitive information
- Uses only public documentation URLs
- Follows secure configuration practices
34-38: Clarify the purpose of non-visible SDK template
The SDK template section is marked as "not visible in the ui". This raises a few questions:
- If it's not visible, what's the purpose of this section?
- Should this configuration be moved to a different file if it's not UI-related?
- Are there any specific SDK settings required for LinkedIn Audience integration?
1-40: Verify if empty field configurations are intentional
The configuration contains multiple empty field arrays (fields: [] or fields: [{}]). This might indicate incomplete configuration for:
- Initial setup fields (line 11)
- Connection mode fields (line 27)
- SDK settings fields (line 37)
Please confirm if this is intentional or if these sections need to be populated with specific field configurations.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1758 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 53 53
Branches 7 7
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
src/configurations/destinations/linkedin_audience/schema.json (3)
1-5: Consider adding recommended JSON Schema attributes.The schema could benefit from additional metadata attributes for better documentation and validation:
title: To provide a human-readable namedescription: To document the purposeadditionalProperties: To control unknown properties{ "configSchema": { "$schema": "http://json-schema.org/draft-07/schema#", + "title": "LinkedIn Audience Configuration Schema", + "description": "Schema for validating LinkedIn Audience destination configuration including OneTrust and Ketch consent settings", "type": "object", + "additionalProperties": false, "properties": {
15-17: Document the string pattern constraints.The pattern
(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$is used multiple times but lacks documentation explaining the allowed formats:
- Handlebars templates with fallbacks:
{{...||...}}- Environment variables:
env.*- Regular strings (max 100 chars)
Add comments explaining the pattern:
"oneTrustCookieCategory": { "type": "string", + "description": "Accepts: 1) Handlebars templates with fallbacks {{template||fallback}}, 2) Environment variables env.*, or 3) Regular strings up to 100 characters", + "examples": ["{{category||Marketing}}", "env.COOKIE_CATEGORY", "Marketing"], "pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$" }Also applies to: 26-29, 44-46, 55-58
1-66: Consider schema reusability across destinations.Since consent management is likely a common requirement across destinations, consider:
- Moving common patterns to shared schema definitions
- Using
$refto reference these shared definitions- Creating a base consent schema that destinations can extend
This would improve maintainability and ensure consistent validation across destinations.
src/configurations/destinations/linkedin_audience/ui-config.json (2)
11-11: Remove empty placeholder object from fields arrayThe empty object in the fields array serves no purpose and should be removed.
-"fields": [{}] +"fields": []
146-146: Fix inconsistent apostrophe usageChange "ID's" to "IDs" for consistency with other labels in the file.
-"label": "Enter consent category ID's", +"label": "Enter consent category IDs",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/configurations/destinations/linkedin_audience/schema.json(1 hunks)src/configurations/destinations/linkedin_audience/ui-config.json(1 hunks)
🔇 Additional comments (4)
src/configurations/destinations/linkedin_audience/schema.json (1)
35-63: Apply the same validation improvements to Ketch consent purposes.
The structure mirrors OneTrust cookie categories and would benefit from the same validation enhancements.
src/configurations/destinations/linkedin_audience/ui-config.json (3)
116-116: Verify default consent provider setting
The default consent provider is set to "oneTrust". Please verify if this is the correct default for LinkedIn Audience integration.
Let's check default providers in other destinations:
#!/bin/bash
# Search for default consent provider configurations
rg -g "ui-config.json" -C 5 '"default": "oneTrust"' src/configurations/destinations/34-38: Clarify the purpose of SDK Template section
The SDK template section is marked as "not visible in the ui" and contains no fields. Please clarify:
- Is this section needed for LinkedIn Audience integration?
- If needed, what fields should be configured here?
Let's check if other destinations use this template:
#!/bin/bash
# Search for SDK template usage in other destinations
rg -g "ui-config.json" -C 5 '"sdkTemplate"' src/configurations/destinations/16-30:
Connection mode section requires configuration fields
The connection mode section appears incomplete. It includes a title and documentation link but lacks the necessary configuration fields for LinkedIn Audience connection settings.
Let's verify if other destination configurations have similar connection mode fields:
|
share screenshots |
What are the changes introduced in this PR?
We are onboarding a new destination named LinkedIn Audience
What is the related Linear task?
Resolves INT-45
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