feat: Migrate setUserAttribute#689
Conversation
…arams, naming) - SingularKit: key/value/user naming, null checks for Java interop - AppsFlyerKit, Braze AppboyKit: consistent UserAttributeListener overrides Made-with: Cursor
PR SummaryMedium Risk Overview Kits are updated to implement the new Written by Cursor Bugbot for commit 50415b5. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
kits/braze/braze-38/src/main/kotlin/com/mparticle/kits/AppboyKit.kt
Outdated
Show resolved
Hide resolved
- KitIntegration: declare onSetUserAttribute on BaseAttributeListener; remove from UserAttributeListener - KitManagerImpl: dispatch scalar and CSV list paths via BaseAttributeListener; avoid double calls for dual-interface kits - AttributeListener-only kits: delegate onSetUserAttribute to setUserAttribute - AttributeListenerTestKit: implement onSetUserAttribute Made-with: Cursor
…tribute - Drop setUserAttribute from KitIntegration.AttributeListener; kits keep setUserAttribute as a regular method (no override). - KitManagerImpl: use BaseAttributeListener.onSetUserAttribute for scalar and joined list paths; prefer UserAttributeListener when both interfaces apply. - AttributeListenerTestKit: rename callback to setUserAttributeCallback; update DataplanBlockingUserTests. Made-with: Cursor
| if (provider instanceof KitIntegration.AttributeListener) { | ||
| ((KitIntegration.AttributeListener) provider).setUserAttribute(key, newValue); | ||
| if (provider instanceof KitIntegration.BaseAttributeListener listener) { | ||
| listener.onSetUserAttribute(key, newValue, FilteredMParticleUser.getInstance(mpid, provider)); |
There was a problem hiding this comment.
onSetUserAttribute bypasses disabled and filter guards in increment
High Severity
In incrementUserAttribute, the onSetUserAttribute call on BaseAttributeListener at line 743 sits outside the braceless if (!provider.isDisabled() && shouldForwardAttribute(...)) guard at line 739. That guard only controls the nested onIncrementUserAttribute call. As a result, onSetUserAttribute is dispatched to every BaseAttributeListener kit regardless of whether the provider is disabled or the attribute is filtered. Compare this to the properly guarded setUserAttribute(provider, key, value, mpid) private helper and removeUserAttribute, which both wrap their calls inside the full check. Additionally, the old code scoped this unguarded path to AttributeListener only, but the change to BaseAttributeListener now also dispatches to all UserAttributeListener kits (e.g. Braze, Singular), which never received this call during increments before.
Additional Locations (1)
…serAttribute - Move scalar attribute logic from removed setUserAttribute helpers into onSetUserAttribute (or private helpers shared with setAllUserAttributes). - Braze: applyScalarUserAttribute for onSet + initial sync; update unit tests to call onSetUserAttribute with a mocked FilteredMParticleUser. - AttributeListenerTestKit: inline callback/onAttributeReceived in onSetUserAttribute. Made-with: Cursor
…r for list attrs Re-run both branches when a kit implements both interfaces; UserAttributeListener join path calls UserAttributeListener.onSetUserAttribute again. Made-with: Cursor
Split applyScalarUserAttribute into applyScalarUserAttributeOnUser plus focused helpers for age, email/push subscribe, gender, subscription groups, and unmapped custom keys (braze-38 through braze-41). Made-with: Cursor
AttributeListener no longer has setUserAttribute; assert BaseAttributeListener onSetUserAttribute with joined value and FilteredMParticleUser. Made-with: Cursor
FilteredMParticleUser.getInstance returns null in JVM unit tests; Mockito any(Class) does not match null. Use isNull() for the third onSetUserAttribute arg. Made-with: Cursor
| s: String, | ||
| o: Any, | ||
| filteredMParticleUser: FilteredMParticleUser, | ||
| key: String?, |
There was a problem hiding this comment.
Why is this key nullable?
- Use String? and Any? for parameters matching Java Object/null call paths - Add early returns when key or value is null where needed - CleverTap: rename parameters to key/value; use mappedKey/mappedValue for mutations Made-with: Cursor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| user: FilteredMParticleUser, | ||
| ) { | ||
| if (key == null) { | ||
| return |
There was a problem hiding this comment.
Missing null guard for value in LeanplumKit
Low Severity
The onSetUserAttribute override widens value from Any (non-null) to Any? (nullable), but the null guard on line 221 only checks key == null, not value == null. Every other kit in this PR guards against null value, yet here a null value is silently forwarded into setAttributesAndCheckId. The old non-nullable value: Any parameter prevented this; the new nullable value: Any? parameter without a corresponding guard is inconsistent with the PR's stated null-safe contract.
Third parameter is user: FilteredMParticleUser? to match KitManagerImpl and FilteredMParticleUser.getInstance. GA/GA4 use parameter name user. Made-with: Cursor
|
3 participants
Background
Kits that implement
UserAttributeListenershould handleonSetUserAttributewith the same null-safe contract as other kits and forward string values to existingsetUserAttributelogic where appropriate.What Has Changed
onSetUserAttributeuseskey/value/usernaming, nullable parameters, and guards before callingsetUserAttribute.UserAttributeListeneroverrides for consistent Kotlin signatures and null checks.Screenshots/Video
N/A
Checklist
Additional Notes
Single commit on top of
workstation/6.0-Release.