Skip to content

Commit

Permalink
fix(analytics): restore existing logic for user traits (#3294)
Browse files Browse the repository at this point in the history
### Description

Follow up for #3292, which was changing the behavior for the `identify`
call.
It's hard to tell if it would have had a real impact, but for instance
the removed properties would have been missing from the CleverTap user
profiles.
Which we could argue is probably fine for `appVersion` as the CleverTap
lib already adds this internally.
But just to be on the safe side with this, I restored the previous
behavior.

### Test plan

- Tests updated 
- Added types to `getCurrentUserTraits` to ensure changes to
`react-native-device-info` won't bring back the initial bug fixed by
#3292.

### Related issues

N/A

### Backwards compatibility

Yes

Co-authored-by: Tom McGuire <Mcgtom10@gmail.com>
Co-authored-by: Satish Ravi <satish.ravi@valoraapp.com>
Co-authored-by: Kathy Luo <kathyluo18@gmail.com>
  • Loading branch information
4 people authored Dec 28, 2022
1 parent e4109f0 commit 44132ef
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 16 deletions.
1 change: 1 addition & 0 deletions __mocks__/react-native-device-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mockRNDeviceInfo.getBundleId.mockImplementation(() => 'org.celo.mobile.debug')
mockRNDeviceInfo.getVersion.mockImplementation(() => '0.0.1')
mockRNDeviceInfo.getBuildNumber.mockImplementation(() => '1')
mockRNDeviceInfo.getUniqueId.mockImplementation(() => Promise.resolve('abc-def-123'))
mockRNDeviceInfo.getUniqueIdSync.mockImplementation(() => 'abc-def-123')
mockRNDeviceInfo.getDeviceId.mockImplementation(() => 'someDeviceId')
mockRNDeviceInfo.getBrand.mockImplementation(() => 'someBrand')
mockRNDeviceInfo.getModel.mockImplementation(() => 'someModel')
Expand Down
31 changes: 28 additions & 3 deletions src/analytics/ValoraAnalytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ jest.mock('@segment/analytics-react-native-firebase', () => ({}))
jest.mock('react-native-permissions', () => ({}))
jest.mock('@sentry/react-native', () => ({ init: jest.fn() }))
jest.mock('src/redux/store', () => ({ store: { getState: jest.fn() } }))
jest.mock('src/config', () => ({
// @ts-expect-error
...jest.requireActual('src/config'),
STATSIG_API_KEY: 'statsig-key',
}))
jest.mock('statsig-react-native')

const mockDeviceId = 'abc-def-123' // mocked in __mocks__/react-native-device-info.ts (but importing from that file causes weird errors)
const expectedSessionId = '205ac8350460ad427e35658006b409bbb0ee86c22c57648fe69f359c2da648'
Expand Down Expand Up @@ -91,7 +97,6 @@ const state = getMockStoreData({
pincodeType: PincodeType.CustomPin,
},
})
mockStore.getState.mockImplementation(() => state)

// Disable __DEV__ so analytics is enabled
// @ts-ignore
Expand Down Expand Up @@ -142,11 +147,31 @@ describe('ValoraAnalytics', () => {
beforeEach(() => {
jest.clearAllMocks()
jest.unmock('src/analytics/ValoraAnalytics')
Statsig.initialize = jest.fn()
Statsig.updateUser = jest.fn()
jest.isolateModules(() => {
ValoraAnalytics = require('src/analytics/ValoraAnalytics').default
})
mockStore.getState.mockImplementation(() => state)
})

it('creates statsig client on initialization with wallet address as user id', async () => {
mockStore.getState.mockImplementation(() =>
getMockStoreData({ web3: { account: '0x1234ABC', mtwAddress: '0x0000' } })
)
await ValoraAnalytics.init()
expect(Statsig.initialize).toHaveBeenCalledWith(
'statsig-key',
{ userID: '0x1234abc' },
{ environment: { tier: 'development' }, overrideStableID: 'anonId' }
)
})

it('creates statsig client on initialization with null as user id if wallet address is not set', async () => {
mockStore.getState.mockImplementation(() => getMockStoreData({ web3: { account: undefined } }))
await ValoraAnalytics.init()
expect(Statsig.initialize).toHaveBeenCalledWith('statsig-key', null, {
environment: { tier: 'development' },
overrideStableID: 'anonId',
})
})

it('delays identify calls until async init has finished', async () => {
Expand Down
19 changes: 8 additions & 11 deletions src/analytics/ValoraAnalytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,17 @@ class ValoraAnalytics {
}

try {
const { accountAddress } = getCurrentUserTraits(store.getState())
const stasigUser = accountAddress
? {
userID: accountAddress,
}
: null
const { walletAddress } = getCurrentUserTraits(store.getState())
const statsigUser =
typeof walletAddress === 'string'
? {
userID: walletAddress,
}
: null

// getAnonymousId causes the e2e tests to fail
const overrideStableID = isE2EEnv ? E2E_TEST_STATSIG_ID : await Analytics.getAnonymousId()
await Statsig.initialize(STATSIG_API_KEY, stasigUser, {
await Statsig.initialize(STATSIG_API_KEY, statsigUser, {
// StableID should match Segment anonymousId
overrideStableID,
environment: STATSIG_ENV,
Expand Down Expand Up @@ -301,10 +302,6 @@ class ValoraAnalytics {
const prefixedSuperProps = Object.fromEntries(
Object.entries({
...traits,
deviceId: this.deviceInfo?.UniqueID,
appVersion: this.deviceInfo?.Version,
appBuildNumber: this.deviceInfo?.BuildNumber,
appBundleId: this.deviceInfo?.BundleId,
currentScreenId: this.currentScreenId,
prevScreenId: this.prevScreenId,
}).map(([key, value]) => [`s${key.charAt(0).toUpperCase() + key.slice(1)}`, value])
Expand Down
2 changes: 1 addition & 1 deletion src/analytics/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export function* updateUserTraits() {
const traits: ReturnType<typeof getCurrentUserTraits> = yield select(getCurrentUserTraits)
if (traits !== prevTraits) {
const { walletAddress } = traits
yield call([ValoraAnalytics, 'identify'], walletAddress, traits)
yield call([ValoraAnalytics, 'identify'], walletAddress as string | null, traits)
prevTraits = traits
}

Expand Down
4 changes: 4 additions & 0 deletions src/analytics/selectors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,14 @@ describe('getCurrentUserTraits', () => {
})
expect(getCurrentUserTraits(state)).toStrictEqual({
accountAddress: '0x0000000000000000000000000000000000007E57',
appBuildNumber: '1',
appBundleId: 'org.celo.mobile.debug',
appVersion: '0.0.1',
celoBalance: 0,
ceurBalance: 21,
countryCodeAlpha2: 'US',
cusdBalance: 10,
deviceId: 'abc-def-123',
deviceLanguage: 'en-US',
hasCompletedBackup: false,
hasVerifiedNumber: false,
Expand Down
9 changes: 8 additions & 1 deletion src/analytics/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getRegionCodeFromCountryCode } from '@celo/phone-utils'
import BigNumber from 'bignumber.js'
import DeviceInfo from 'react-native-device-info'
import * as RNLocalize from 'react-native-localize'
import { createSelector } from 'reselect'
import { defaultCountryCodeSelector, pincodeTypeSelector } from 'src/account/selectors'
Expand Down Expand Up @@ -41,7 +42,9 @@ export const getCurrentUserTraits = createSelector(
hasCompletedBackup,
pincodeType,
superchargeInfo
) => {
): // Enforce primitive types, TODO: check this using `satisfies` once we upgrade to TS >= 4.9
// so we don't need to erase the named keys
Record<string, string | boolean | number | null | undefined> => {
const coreTokensAddresses = new Set(coreTokens.map((token) => token?.address))
const tokensByUsdBalance = tokens.sort(sortByUsdBalance)

Expand Down Expand Up @@ -89,6 +92,10 @@ export const getCurrentUserTraits = createSelector(
localCurrencyCode,
hasVerifiedNumber,
hasCompletedBackup,
deviceId: DeviceInfo.getUniqueIdSync(),
appVersion: DeviceInfo.getVersion(),
appBuildNumber: DeviceInfo.getBuildNumber(),
appBundleId: DeviceInfo.getBundleId(),
pincodeType,
superchargingToken: superchargeInfo.superchargingTokenConfig?.tokenSymbol,
superchargingAmountInUsd: superchargeInfo.superchargeUsdBalance,
Expand Down

0 comments on commit 44132ef

Please sign in to comment.