feat(nocodes): add custom variables and forceSendProperties bridges#61
Conversation
Bumps QonversionSandwich to 7.9.0 to consume new Sandwich APIs.
Adds public DeferredPurchasesListener API and forwards QonversionEventsListener.onDeferredPurchaseCompleted from Sandwich through deferredPurchaseCompletedEvent on both Android and iOS.
NickSxti
left a comment
NickSxti
left a comment
There was a problem hiding this comment.
Cross-SDK comparison review
Compared this PR against the sibling deferred-purchase rollouts: react-native-sdk #423, flutter-sdk #436, cordova-plugin #152 (+ follow-ups #153–#156), and unity-sdk #305. Architectural direction is right - using the native Sandwich onDeferredPurchaseCompleted event matches what every other late-family SDK ended up doing, and avoids the JS-correlation limitations called out in DEV-643. Below are the gaps relative to family conventions.
Blockers
1. Listener accumulates on repeated set*Listener calls - src/internal/QonversionInternal.ts
Every call to setDeferredPurchasesListener adds a fresh addListener without removing the previous one. The constructor calls it once if the builder configured a listener; if the user later calls the runtime setter to replace it, both fire on every event. Same latent bug already in setEntitlementsUpdateListener.
Sister SDKs avoid this explicitly:
- RN #423 uses an idempotent subscription flag (
deferredPurchaseEventSubscribed) and reassigns the listener field - Cordova #152 subscribes once in the constructor and reads a mutable field
RN ships explicit tests for this: replaces previous listener when setting a new one and subscribes to onDeferredPurchaseCompleted only once. Both would fail here.
2. EntitlementsUpdateListener is not deprecated
Every sibling marks the old listener @deprecated:
| Repo | Interface | API setter | Builder setter | Config field |
|---|---|---|---|---|
| RN #423 | yes | yes | yes | yes |
| Cordova #152 | yes | yes | yes | - |
| this PR | no | no | no | no |
DEV-665 explicitly deprecated QEntitlementsUpdateListener upstream; consumers won't see the migration signal without matching JSDoc here.
3. No tests added
RN #423 added 254 lines covering listener replacement, single-subscription, and backward-compat scenarios. The two bugs above are exactly what those tests catch. Adding at least the "replaces previous listener" and "subscribes only once" tests would prevent regression.
Medium
4. Self-referential JSDoc - already fixed in cordova-plugin #154
QonversionApi.ts:268-275 carries the pre-fix wording word-for-word:
"You may set the listener both after Qonversion SDK initializing with
{@link QonversionApi.setDeferredPurchasesListener}and..."
That {@link} points to the very method whose docstring it lives in. cordova-plugin #154 fixed this to point at QonversionConfigBuilder.setDeferredPurchasesListener for the during-init path.
5. forceSendProperties swallows errors - QonversionPlugin.kt:114-117, QonversionPlugin.swift:142-146
Both platforms call.resolve() unconditionally inside the Sandwich callback. Compare to peer methods using call.toResultListener() / getDefaultCompletion(call). If Sandwich's forceSendProperties callback can pass a failure, JS sees a successful resolve while properties weren't flushed - exactly the failure mode the API is trying to prevent.
6. customVariables cross-platform inconsistency - NoCodesPlugin.kt, NoCodesPlugin.swift
- Android:
json.optString(key, null)coerces -5becomes"5",truebecomes"true" - iOS:
as? [String: String]returnsnilfor the whole map if any value isn't already a String
JS type is Record<string, string> but runtime safety nets diverge. A user passing {count: 5} works on Android and disappears on iOS.
7. PR scope - three independent features bundled
Every other SDK shipped deferred-purchases as a focused PR. This bundles deferred-purchases + forceSendProperties + NoCodes customVariables because they share the Sandwich 7.7.0 → 7.9.0 bump. Title only mentions the first two; deferred-purchases is buried. Cordova still needed 4 follow-up PRs even with focused scope, so the cost of splitting is bounded.
Low
8. Constructor parameter ordering
RN and Cordova group listeners adjacent: entitlementsUpdateListener, deferredPurchasesListener, proxyUrl, kidsMode. This PR appends at the end: ..., proxyUrl, kidsMode, deferredPurchasesListener. Cosmetic, but the family has a convention.
9. Sandwich version skip 7.7.0 → 7.9.0
Worth a one-line note in the PR description on what 7.8.0 contained.
10. No plugin version bump
sdkVersion = "1.4.0" and the podspec/build.gradle versions are unchanged despite a public-API expansion (forceSendProperties, setDeferredPurchasesListener, customVariables, new exported interface).
Cordova follow-up applicability
| Cordova fix | Applies here? |
|---|---|
| #153 rename Android arg | No (handler arg names are platform-specific by convention) |
| #154 JSDoc self-ref | Yes - bug present (item 4) |
| #155 channel registration order | No (Cordova-specific timing; Capacitor's addListener doesn't have this coupling) |
| #156 plugin.xml cleanup | No (Capacitor uses ES exports; already correct) |
Recommended pre-merge
Items 1, 2, and 3 (idempotent subscription, deprecation, two tests) bring this in line with the family. Items 4-7 are quality-of-life. Items 8-10 are cosmetic.
Architectural direction is right - just polish to match the established cross-SDK conventions before merge.
setEntitlementsUpdateListener and setDeferredPurchasesListener used to call QonversionNative.addListener every time, so each invocation stacked another native subscription on top of the previous one. Repeated calls left every prior listener still firing. Subscribe to each native event exactly once and dispatch to a stored listener field that the setter overwrites. Mirrors the fix shipped in react-native-sdk #423. Adds a jest setup (ts-jest, no extra config beyond preset) and four tests covering the once-only subscription and the replacement semantics for both listeners.
…rchasesListener docs - mark EntitlementsUpdateListener interface and its setters as @deprecated, pointing users to DeferredPurchasesListener (matches RN/Cordova SDKs) - fix self-referential JSDoc on QonversionApi.setDeferredPurchasesListener to point to QonversionConfigBuilder.setDeferredPurchasesListener for the init-time setter (mirrors cordova-plugin #154) - reorder QonversionConfig constructor and field declarations so deferredPurchasesListener sits next to entitlementsUpdateListener, matching the RN/Cordova family ordering
customVariables (Record<string, string> per JS contract):
- Android NoCodesPlugin: replace optString(key, null) with
(json.opt(key) as? String) so non-string entries (numbers, booleans,
arrays, objects) are skipped instead of coerced to strings. Matches
the iOS guard direction.
- iOS NoCodesPlugin: replace whole-map `as? [String: String]` with
per-entry filter. Previously a single non-string value caused the
entire map to be dropped to nil; now non-string entries are silently
skipped and string entries pass through.
- Both platforms now: pass nil through, accept {}, accept {a:"x"},
drop non-string entries.
Version bump 1.4.0 -> 1.5.0 (minor; expands public API with NoCodes
customVariables and forceSendProperties bridges):
- package.json
- src/internal/QonversionInternal.ts sdkVersion constant
The podspec reads from package.json so it picks up the bump
automatically. android/build.gradle has versionName "1.0" but that is
just AAR metadata, not the plugin's published version, so no change
there.
forceSendProperties error propagation: investigated and left as-is.
QonversionSandwich 7.9.0 callback signatures on both platforms are
purely () -> Unit/Void with no error parameter:
- Android: fun forceSendProperties(callback: () -> Unit)
(sandwich-sdk/android/sandwich/.../QonversionSandwich.kt:265)
Underlying QonversionEmptyCallback exposes only onComplete().
- iOS: func forceSendProperties(_ completion: @escaping () -> Void)
(sandwich-sdk/ios/sandwich/QonversionSandwich.swift:311)
There is no error to propagate; switching to the toResultListener /
getDefaultCompletion peer pattern would not match the Sandwich API.
The current `call.resolve()` on completion is correct.
Package A's tests were written against the original constructor signature where kidsMode preceded deferredPurchasesListener. Package B reordered constructor params so deferredPurchasesListener sits adjacent to entitlementsUpdateListener (matching RN/Cordova family ordering). The two changes were correct in isolation; this updates the test fixture to match the merged signature.
- Restore deferredPurchasesListener as the trailing constructor arg in QonversionConfig so the public signature stays semver-compatible with 1.4.0. Builder + test fixture realigned to the original arg order. - Revert package version 1.5.0 -> 1.4.0 and sdkVersion constant; the bump will happen at release time, not on the feature branch.
2 participants
Bumps QonversionSandwich to 7.9.0 to consume new Sandwich APIs.