feat: onboarding new destination accoil analytics #1928
Conversation
Co-authored-by: Manish Kumar <[email protected]>
Co-authored-by: Manish Kumar <[email protected]>
…ion-ui feat: accoil analytics ui
manish339k
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
ItsSudip,
sandeepdsvs,
krishna2020 and
1abhishekpandey
as code owners
WalkthroughThis pull request modifies several JSON configuration files for the Accoil Analytics destination. The Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 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: 0
🧹 Nitpick comments (9)
src/configurations/destinations/accoil_analytics/schema.json (1)
7-10: Regex Update forapiKeyValidation
The updated regex on line 9 now restricts valid API key values to alphanumeric characters and underscores (with a length of 1–80) while still allowing the templated ({{...||...}}) and environment variable formats. Please verify that this more restrictive pattern aligns with all production API key formats and that it doesn’t inadvertently reject any previously valid keys.test/data/validation/destinations/accoil_analytics.json (1)
1-364: Comprehensive Test Coverage for Destination Configurations
This new test suite provides broad coverage of configuration scenarios—including valid setups, missing required properties, and type/pattern mismatches—for properties such asapiKey,consentManagement,oneTrustCookieCategories, andketchConsentPurposes. The various cases ensure that the JSON schema validations work as expected. Consider adding brief inline comments or grouping markers within the file to clarify the intent behind each test case block for future maintainability.src/configurations/destinations/accoil_analytics/ui-config.json (7)
1-45: UI Configuration – Initial Setup Section
The "Initial setup" section is well-organized, offering a clear structure for connection settings. The API Key field now uses a regex (^[0-9a-zA-Z_]{1,80}$) that precisely matches the updated schema validation, which promotes consistency. A minor enhancement would be to expand the descriptive text in the "note" fields to further guide users on expected inputs.
29-43: UI Configuration – Connection Mode Group
The "Connection mode" group provides useful guidance with an external help link. Note that the"fields"array is empty—if this is intentional for future extension, adding a brief comment or placeholder could help maintain clarity for future maintainers.
46-58: UI Configuration – Configuration Settings Section
This section clearly separates consent settings from connection settings. Although the"groups"array is currently empty, it establishes a structured area for future consent-related configurations. Consider including a comment or placeholder if additional default groups are planned.
60-64: UI Configuration – SDK Template Section
ThesdkTemplateis defined for Web SDK settings, albeit marked as "not visible in the UI." Confirm if this section will be populated later or if it should remain as a structural placeholder for potential future features.
65-93: UI Configuration – OneTrust Consent Category IDs Field
The tag input for OneTrust consent category IDs is clearly defined with appropriate labels, regex validation, and placeholders. The default value initializes with an empty string—please ensure this is the desired behavior for user experience, or consider prompting users explicitly for input if necessary.
94-118: UI Configuration – Ketch Consent Purpose IDs Field
This field’s configuration mirrors that of the OneTrust input, which is good for consistency. Verify that the messaging and placeholder text clearly differentiate the two when used in the UI to avoid user confusion.
119-192: UI Configuration – Dynamic Custom Form for Consent Management
The dynamic custom form is well-structured, allowing end users to select a consent management provider and—if "custom" is chosen—to specify a resolution strategy. The dependencies (usingpreRequisites) correctly enforce that theresolutionStrategyis required when needed. A minor suggestion is to rephrase the label"the required consent logic"to something like"Required Consent Logic"for improved clarity and consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)test/data/validation/destinations/accoil_analytics.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)test/data/validation/destinations/accoil_analytics.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/configurations/destinations/accoil_analytics/db-config.json (1)
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:148-284
Timestamp: 2024-12-09T04:17:28.675Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated. Avoid suggesting changes to autogenerated files.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (1)
src/configurations/destinations/accoil_analytics/db-config.json (1)
1-122: Auto-Generated Configuration File Notice
This newly introduceddb-config.jsonappears to be auto-generated for Accoil Analytics. As per our guidelines (and previous learnings), avoid making direct manual modifications to autogenerated files. Instead, ensure that any required changes are made upstream in the source generation process to prevent them from being inadvertently overwritten.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1928 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 53 53
Branches 7 7
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/data/validation/destinations/accoil_analytics.json (1)
8-12: Simplify and Validate Error Message for Missing API Key
The second test case now uses an empty"config": {}to trigger a validation failure for the missingapiKey. Please verify that the error message[" must have required property 'apiKey'"]matches the expected output. Note the leading space in the error string; confirm that this formatting is intentional.
src/configurations/destinations/accoil_analytics/ui-config.json (1)
127-128: Refine Consent Category IDs Input Field
The"tagInput"field’s label and note have been updated to clarify that users should enter consent category IDs (using plural “IDs”) and emphasize their uniqueness. This improves clarity compared to the previous version. Please double-check that this new wording meets the desired UX style and consistency guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)test/data/validation/destinations/accoil_analytics.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(1 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(1 hunks)test/data/validation/destinations/accoil_analytics.json(1 hunks)src/configurations/destinations/accoil_analytics/db-config.json(1 hunks)src/configurations/destinations/accoil_analytics/schema.json(12 hunks)src/configurations/destinations/accoil_analytics/ui-config.json(2 hunks)test/data/validation/destinations/accoil_analytics.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/configurations/destinations/accoil_analytics/db-config.json
- test/data/validation/destinations/accoil_analytics.json
- src/configurations/destinations/accoil_analytics/db-config.json
- src/configurations/destinations/accoil_analytics/db-config.json
- src/configurations/destinations/accoil_analytics/ui-config.json
- src/configurations/destinations/accoil_analytics/schema.json
- src/configurations/destinations/accoil_analytics/db-config.json
🧰 Additional context used
🧠 Learnings (2)
src/configurations/destinations/accoil_analytics/db-config.json (2)
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:148-284
Timestamp: 2024-12-09T04:17:28.675Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated. Avoid suggesting changes to autogenerated files.
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:285-795
Timestamp: 2024-12-09T04:17:16.931Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated.
src/configurations/destinations/accoil_analytics/schema.json (3)
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:148-284
Timestamp: 2024-12-09T04:17:28.675Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated. Avoid suggesting changes to autogenerated files.
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:285-795
Timestamp: 2024-12-09T04:17:16.931Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated.
Learnt from: accoilmj
PR: rudderlabs/rudder-integrations-config#1807
File: src/configurations/destinations/accoil_analytics/schema.json:11-147
Timestamp: 2024-12-09T04:17:05.013Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated and should not be manually modified.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (3)
src/configurations/destinations/accoil_analytics/db-config.json (1)
1-121: New Destination Configuration – Verify Intentional Values and Consistency
This new configuration file for Accoil Analytics includes keys such as"ketchConsentPurposes"and still lists"warehouse"under both"supportedSourceTypes"and"supportedConnectionModes". Please confirm that:
- The inclusion of
"warehouse"is intentional despite previous indications to remove certain references.- The addition of
"ketchConsentPurposes"in"includeKeys"and within"destConfig"aligns with the overall destination configuration requirements.
Ensure consistency with the corresponding schema definitions inschema.json.src/configurations/destinations/accoil_analytics/schema.json (1)
1-861: Autogenerated Schema – No Manual Modifications Expected
This JSON schema file is autogenerated (as noted in the long-term learnings). Do not modify it manually.
If the new provider"iubenda"is intended to be supported (as seen in the UI config), please ensure that the underlying code generation process is updated accordingly so that the schema’s"consentManagement"section reflects the new option.src/configurations/destinations/accoil_analytics/ui-config.json (1)
84-87:❓ Verification inconclusive
Add New Consent Management Provider Option "iubenda"
The singleSelect field for the consent management provider now includes an option for"iubenda". Ensure that this new option is supported end-to-end—particularly, verify that the schema (or its generation process) is updated to accept"iubenda"as a valid value.
#!/bin/bash # Script to search the repository for references to "iubenda", particularly in context of allowed enum values for consent management providers. rg -iE '("enum"\s*:\s*\[[^\]]*("iubenda"))' -n .Confirmation Needed: End-to-End Support for "iubenda" Option
Thesrc/configurations/destinations/accoil_analytics/ui-config.jsonfile now includes the"iubenda"option. Please verify that the associated JSON schema (or its generation process) is updated to accept"iubenda"as a valid value throughout the system.
What are the changes introduced in this PR?
What is the related Linear task?
Resolves INT-2905
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
Bug Fixes
Tests