-
Notifications
You must be signed in to change notification settings - Fork 16
[MSDK-3955] Expose ConsentOrPay settings in React Native bridge for P… #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import com.usercentrics.sdk.v2.settings.data.CustomizationFont | |
| import com.usercentrics.sdk.v2.settings.data.FirstLayer | ||
| import com.usercentrics.sdk.v2.settings.data.PublishedApp | ||
| import com.usercentrics.sdk.v2.settings.data.SecondLayer | ||
| import com.usercentrics.sdk.v2.settings.data.ConsentOrPaySettings | ||
| import com.usercentrics.sdk.v2.settings.data.TCF2ChangedPurposes | ||
| import com.usercentrics.sdk.v2.settings.data.TCF2Settings | ||
| import com.usercentrics.sdk.v2.settings.data.UsercentricsCategory | ||
|
|
@@ -246,9 +247,17 @@ private fun TCF2Settings.serialize(): WritableMap { | |
| "changedPurposes" to changedPurposes?.serialize(), | ||
| "acmV2Enabled" to acmV2Enabled, | ||
| "selectedATPIds" to selectedATPIds, | ||
| "consentOrPay" to consentOrPay?.serialize(), | ||
| ).toWritableMap() | ||
| } | ||
|
|
||
| private fun ConsentOrPaySettings.serialize(): Map<String, Any?> = mapOf( | ||
| "enableConsentOrPay" to enableConsentOrPay, | ||
| "showTogglesForVendors" to showTogglesForVendors, | ||
| "publisherRestrictions" to publisherRestrictions, | ||
| "specialFeatures" to specialFeatures | ||
|
Comment on lines
+257
to
+258
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Severity Level: Major
|
||
| ) | ||
|
Comment on lines
+254
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [REFACTORING] The new private fun ConsentOrPaySettings.serialize() (lines 254-259) currently returns a Kotlin Map<String,Any?> while other serializer helpers in this file return WritableMap (or rely on .toWritableMap()). For consistency and to avoid subtle nested conversion bugs, make this return a WritableMap (e.g. build a map and call .toWritableMap()). Also explicitly convert publisherRestrictions and specialFeatures into JS-friendly structures (Map keys -> String, values -> primitives). Example: publisherRestrictions?.mapKeys { it.key.toString() }?.toMap() and then include .toWritableMap(). This reduces runtime surprises across Android RN bridge conversions. private fun ConsentOrPaySettings.serialize(): WritableMap = mapOf(
"enableConsentOrPay" to enableConsentOrPay,
"showTogglesForVendors" to showTogglesForVendors,
"publisherRestrictions" to publisherRestrictions
?.mapKeys { it.key.toString() }
?.toMap(),
"specialFeatures" to specialFeatures
?.mapKeys { it.key.toString() }
?.toMap(),
).toWritableMap() |
||
|
|
||
| private fun TCF2Settings.getResurfacePeriodCompat(): Int { | ||
| val intValue = runCatching { | ||
| javaClass.getMethod("getResurfacePeriod").invoke(this) as? Int | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,10 +236,21 @@ extension TCF2Settings { | |
| "acmV2Enabled": self.acmV2Enabled, | ||
| "selectedATPIds": self.selectedATPIds, | ||
| "resurfacePeriod": self.resurfacePeriod, | ||
| "consentOrPay": self.consentOrPay?.toDictionary() as Any, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [VALIDATION] You added mapping "consentOrPay": self.consentOrPay?.toDictionary() (line 239). Ensure the toDictionary() produces JS-bridge-safe types: convert KotlinBoolean to Bool (.boolValue) where needed and convert any Kotlin map/dictionary keys to Swift String keys before returning. If consentOrPay fields can be nil, keep the optional handling (as you already do) but confirm callers expect null vs empty object. extension TCF2Settings {
func toDictionary() -> NSDictionary {
return [
// ...existing fields...
"selectedATPIds": self.selectedATPIds,
"resurfacePeriod": self.resurfacePeriod,
"consentOrPay": self.consentOrPay?.toDictionary() as Any,
]
}
}
extension ConsentOrPaySettings {
func toDictionary() -> [String: Any] {
return [
"enableConsentOrPay": self.enableConsentOrPay.boolValue,
"showTogglesForVendors": self.showTogglesForVendors.boolValue,
"publisherRestrictions": self.publisherRestrictions as [String: Any],
"specialFeatures": self.specialFeatures as [String: Any],
]
}
} |
||
| ] | ||
| } | ||
| } | ||
|
|
||
| extension ConsentOrPaySettings { | ||
| func toDictionary() -> [String: Any] { | ||
| return [ | ||
| "enableConsentOrPay": self.enableConsentOrPay, | ||
| "showTogglesForVendors": self.showTogglesForVendors, | ||
| "publisherRestrictions": self.publisherRestrictions, | ||
| "specialFeatures": self.specialFeatures | ||
|
Comment on lines
+249
to
+250
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The Consent-or-Pay dictionaries are exposed directly to JS without converting keys/values to RN-safe JSON primitives. If these SDK maps use numeric keys or non-primitive value types, bridging to JavaScript can fail or produce unusable objects. Convert them into Severity Level: Major
|
||
| ] | ||
| } | ||
| } | ||
|
Comment on lines
+244
to
+253
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The new extension ConsentOrPaySettings.toDictionary() returns fields directly (lines 244-253). This may cause compile/runtime issues: Kotlin booleans from the KMP bindings are often KotlinBoolean and need to be converted with .boolValue (see pattern in ios/Extensions/TCFData+Dict.swift lines ~85-115 where consent?.boolValue is used). Also publisherRestrictions and specialFeatures may be Kotlin map types — convert them to native [String: Any] (map keys to String) and ensure values are JS-serializable. Update to mirror existing conversion patterns (use .boolValue for KotlinBoolean and explicit dictionary transforms) to avoid crashes or incorrect values in JS. extension ConsentOrPaySettings {
func toDictionary() -> [String: Any] {
return [
"enableConsentOrPay": self.enableConsentOrPay.boolValue,
"showTogglesForVendors": self.showTogglesForVendors.boolValue,
// Ensure keys are Strings and values are JSON-serializable
"publisherRestrictions": self.publisherRestrictions.reduce(into: [String: String]()) { result, entry in
if let key = entry.key as? String, let value = entry.value as? String {
result[key] = value
}
},
"specialFeatures": self.specialFeatures.reduce(into: [String: String]()) { result, entry in
if let key = entry.key as? String, let value = entry.value as? String {
result[key] = value
}
},
]
}
} |
||
|
|
||
| extension UsercentricsCustomization { | ||
| func toDictionary() -> NSDictionary { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[VALIDATION] You added "consentOrPay" to the TCF2Settings map (new line 250). Ensure the nested ConsentOrPaySettings data is serialized into a shape accepted by toWritableMap() (and ultimately the RN bridge) — specifically: 1) handle nulls safely (use consentOrPay?.let { ... } ), 2) normalize key types for nested maps (publisherRestrictions/specialFeatures) to string keys (JS requires string keys), and 3) convert nested structures to WritableMap/Array where applicable. Also update related Android unit-test expected maps so they include the new "consentOrPay" entry (see android/src/androidTest/java/com/usercentrics/reactnative/mock/GetCMPDataMock.kt expectedTCF2Settings around lines 448-509).