feat: support new consent resolution strategy values #5798
Conversation
saikumarrs
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the new consent resolution strategy values ("all" and "any") alongside the existing "and" and "or". It also updates the processing logic to prioritize "allowed consent IDs", fall back to "denied consent IDs", and sanitize consent IDs before filtering events.
- Changed the type of AllowedConsentIDs from interface{} to []string.
- Modified the filtering logic in getConsentFilteredDestinations to accommodate new resolution strategies.
- Enhanced consent ID processing and sanitation in getConsentManagementInfo.
saikumarrs
force-pushed
the
feat.support-new-resolution-strategies-sdk-2944
branch
from
b9773d9 to
6a1283f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5798 +/- ##
==========================================
+ Coverage 76.86% 76.92% +0.06%
==========================================
Files 491 491
Lines 67207 67252 +45
==========================================
+ Hits 51658 51733 +75
+ Misses 12723 12699 -24
+ Partials 2826 2820 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for new consent resolution strategies ("all" and "any") and adjusts logic to prefer allowed consent IDs while handling legacy OneTrust CMP formats. Key changes include:
- Updated test expectations for destination filtering in processor_test.go.
- Refactored consent management parsing in consent.go to support both object and array formats for allowed consent IDs.
- Added sanitation of consent IDs before filtering events.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| processor/processor_test.go | Updated test expectations to match changes in the consent resolution logic. |
| processor/consent.go | Enhanced parsing and sanitation logic for consent IDs, supporting new and legacy formats. |
Comments suppressed due to low confidence (1)
processor/consent.go:74
- [nitpick] Adding inline comments within each case of the resolution strategy switch (for 'any'/'or' and 'all'/'and') could help clarify the intended logic differences and ease future changes.
switch finalResolutionStrategy {
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for new consent resolution strategy values ("all" and "any") and adjusts the consent filtering logic accordingly. Key changes include updated test expectations in processor_test.go and enhancements to consent handling in processor/consent.go to support both array and object formats for allowed consent IDs.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| processor/processor_test.go | Updated expected destination counts in tests to match new logic |
| processor/consent.go | Revised consent processing for allowed and denied consent IDs, including legacy support for OneTrust CMP |
Comments suppressed due to low confidence (1)
processor/processor_test.go:3656
- The updated test expectation reflects the new consent resolution logic. Please verify that these new expected values accurately capture the intended filtering behavior across all relevant cases.
).To(Equal(6)) // all except D6, D13, and D14
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for new consent resolution strategy values ("all" and "any") and adjusts the logic to prefer allowed consent IDs while handling both object and array formats for legacy OneTrust CMP data. Key changes include updated test expectation values for consent filtering, enhancements in parsing and sanitizing consent IDs, and revised resolution strategy logic.
- Update tests to reflect new resolution logic
- Enhance getConsentManagementInfo to support multiple types for AllowedConsentIDs and sanitize inputs
- Adjust consent filtering logic in getConsentFilteredDestinations per new strategies
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| processor/processor_test.go | Updated expected count values in tests to reflect the new consent filtering logic |
| processor/consent.go | Revised consent parsing and sanitization, and added support for new resolution strategies |
Comments suppressed due to low confidence (1)
processor/processor_test.go:3656
- Verify that the updated expected count values in the tests accurately reflect the new consent filtering logic and confirm that the comments remain consistent with the actual behavior.
).To(Equal(6)) // all except D6, D13, and D14
| // This is to support really old version of the JS SDK v3 that sent this data as an object | ||
| // for OneTrust provider. | ||
| // Handle AllowedConsentIDs based on its type (array or map) | ||
| switch val := consentManagementInfoFromEvent.AllowedConsentIDs.(type) { |
There was a problem hiding this comment.
[nitpick] Consider logging or handling cases where non-string values are encountered during conversion from []interface{} to []string, so that unexpected types are not silently ignored.
Copilot uses AI. Check for mistakes.
Description
Besides, "and" and "or" consent resolution strategies, we're now supporting the new "all" and "any" values. The UI changes will be rolled out very soon.
Also, the logic now prefers to use "allowed consent IDs" and falls back to "denied consent IDs" to align with the client side implementation. It's a bit tricky with allowed consent IDs as some older SDK versions send it as an object for OneTrust CMP.
So, we're handling both an object and array of strings.
Additionally, I've sanitized the consent IDs from the config and event before resolving them for filtering events to destinations.
Linear Ticket
https://linear.app/rudderstack/view/scrum-board-75b83c60706a5
Security