Skip to content

Commit

Permalink
refactor(education): remove unnecessary prop and enum (#5341)
Browse files Browse the repository at this point in the history
### Description

- Removed EmbeddedNavBar enum (only one value)
- Removed onboarding from EducationTopic
- Cleanup unused analytics events

### Test plan

CI

### Related issues

Part of ACT-1133

### Backwards compatibility

Yes

### Network scalability

N/A
  • Loading branch information
satish-ravi committed Apr 26, 2024
1 parent 31acd51 commit 52fb14d
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 103 deletions.
3 changes: 1 addition & 2 deletions src/account/AccountKeyEducation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { NativeStackScreenProps } from '@react-navigation/native-stack'
import React, { useEffect } from 'react'
import { useTranslation } from 'react-i18next'
import { Platform } from 'react-native'
import Education, { EducationTopic, EmbeddedNavBar } from 'src/account/Education'
import Education, { EducationTopic } from 'src/account/Education'
import { OnboardingEvents } from 'src/analytics/Events'
import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { BtnTypes } from 'src/components/Button'
Expand Down Expand Up @@ -34,7 +34,6 @@ export default function AccountKeyEducation(props: Props) {

return (
<Education
embeddedNavBar={EmbeddedNavBar.Close}
stepInfo={steps}
onFinish={onComplete}
finalButtonText={t('completeEducation')}
Expand Down
18 changes: 1 addition & 17 deletions src/account/Education.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ import { fireEvent, render } from '@testing-library/react-native'
import * as React from 'react'
import 'react-native'
import { Provider } from 'react-redux'
import { OnboardingEvents } from 'src/analytics/Events'
import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { navigateBack } from 'src/navigator/NavigationService'
import { createMockStore } from 'test/utils'
import Education, { EducationTopic, EmbeddedNavBar } from '../../src/account/Education'
import Education, { EducationTopic } from '../../src/account/Education'

jest.mock('src/analytics/ValoraAnalytics')

Expand All @@ -22,7 +20,6 @@ const educationProps = {
},
],
buttonText: 'next',
embeddedNavBar: EmbeddedNavBar.Close,
finalButtonText: BUTTON_TEXT,
onFinish: jest.fn(),
}
Expand All @@ -48,17 +45,4 @@ describe('Education', () => {
fireEvent.press(edu.getByTestId('Education/CloseIcon'))
expect(navigateBack).toBeCalled()
})

it('onboarding step info analytics event fires', () => {
// Onboarding analytics event
const onboardingProps = { ...educationProps }
onboardingProps.stepInfo[0].topic = EducationTopic.onboarding
render(<Education {...educationProps} />)
expect(ValoraAnalytics.track).toHaveBeenCalledWith(
OnboardingEvents.onboarding_education_step_impression,
{
step: 0,
}
)
})
})
55 changes: 8 additions & 47 deletions src/account/Education.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useRef, useState } from 'react'
import React, { useRef, useState } from 'react'
import {
Image,
ImageSourcePropType,
Expand All @@ -25,13 +25,7 @@ import fontStyles from 'src/styles/fonts'
import progressDots from 'src/styles/progressDots'
import variables from 'src/styles/variables'

// TODO: clean this up
export enum EmbeddedNavBar {
Close = 'Close',
}

export enum EducationTopic {
onboarding = 'onboarding',
backup = 'backup',
celo = 'celo',
}
Expand All @@ -46,7 +40,6 @@ interface EducationStep {
}

export type Props = NativeSafeAreaViewProps & {
embeddedNavBar: EmbeddedNavBar | null
stepInfo: EducationStep[]
buttonType?: BtnTypes
buttonText: string
Expand All @@ -60,7 +53,6 @@ export type Props = NativeSafeAreaViewProps & {
const Education = (props: Props) => {
const {
style,
embeddedNavBar,
stepInfo,
buttonType = BtnTypes.SECONDARY,
buttonText,
Expand All @@ -73,9 +65,6 @@ const Education = (props: Props) => {
} = props

const [currentIndex, setCurrentIndex] = useState(0)
// This variable tracks the last scrolled to carousel screen, so that impression
// events are not dispatched twice for the same carousel screen
const lastViewedIndex = useRef(-1)
// Scroll View Ref for button clicks
const scrollViewRef = useRef<ScrollView>(null)

Expand All @@ -97,28 +86,11 @@ const Education = (props: Props) => {
currentStep: currentIndex,
direction: direction,
})
} else if (topic === EducationTopic.onboarding) {
ValoraAnalytics.track(OnboardingEvents.onboarding_education_scroll, {
currentStep: currentIndex,
direction: direction,
})
}

setCurrentIndex(Math.round(event.nativeEvent.contentOffset.x / variables.width))
}

useEffect(() => {
const { topic } = stepInfo[currentIndex]
if (stepInfo.length > 0 && lastViewedIndex.current < currentIndex) {
lastViewedIndex.current = currentIndex
}
if (topic === EducationTopic.onboarding) {
ValoraAnalytics.track(OnboardingEvents.onboarding_education_step_impression, {
step: currentIndex,
})
}
}, [currentIndex])

if (!stepInfo.length) {
// No Steps, no slider
return null
Expand All @@ -145,26 +117,15 @@ const Education = (props: Props) => {
: scrollViewRef.current?.scrollTo({ x: variables.width * (currentIndex + 1), animated: true })
}

const renderEmbeddedNavBar = () => {
switch (embeddedNavBar) {
case EmbeddedNavBar.Close:
return (
<View style={styles.top} testID="Education/top">
<TopBarIconButton
testID={`Education/${currentIndex === 0 ? 'Close' : 'Back'}Icon`}
onPress={goBack}
icon={currentIndex === 0 ? <Times /> : <BackChevron color={colors.black} />}
/>
</View>
)
default:
return null
}
}

return (
<SafeAreaView testID="Education" style={[styles.root, style]} {...passThroughProps}>
{renderEmbeddedNavBar()}
<View style={styles.top} testID="Education/top">
<TopBarIconButton
testID={`Education/${currentIndex === 0 ? 'Close' : 'Back'}Icon`}
onPress={goBack}
icon={currentIndex === 0 ? <Times /> : <BackChevron color={colors.black} />}
/>
</View>
<View style={styles.container}>
<ScrollView
ref={scrollViewRef}
Expand Down
3 changes: 1 addition & 2 deletions src/account/GoldEducation.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect } from 'react'
import { useTranslation } from 'react-i18next'
import { Platform } from 'react-native'
import Education, { EducationTopic, EmbeddedNavBar } from 'src/account/Education'
import Education, { EducationTopic } from 'src/account/Education'
import { setGoldEducationCompleted } from 'src/account/actions'
import { celoEducationCompletedSelector } from 'src/account/selectors'
import { OnboardingEvents } from 'src/analytics/Events'
Expand Down Expand Up @@ -38,7 +38,6 @@ export default function GoldEducation() {

return (
<Education
embeddedNavBar={EmbeddedNavBar.Close}
stepInfo={stepInfo}
onFinish={onFinish}
finalButtonType={BtnTypes.PRIMARY}
Expand Down
6 changes: 0 additions & 6 deletions src/analytics/Events.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ export enum AppEvents {
}

export enum HomeEvents {
hamburger_tapped = 'hamburger_tapped',
account_circle_tapped = 'account_circle_tapped',
drawer_navigation = 'drawer_navigation',
drawer_address_copy = 'drawer_address_copy',
profile_address_copy = 'profile_address_copy',
notification_scroll = 'notification_scroll',
notification_impression = 'notification_impression',
Expand Down Expand Up @@ -133,9 +130,6 @@ export enum KeylessBackupEvents {
}

export enum OnboardingEvents {
onboarding_education_scroll = 'onboarding_education_scroll',
onboarding_education_step_impression = 'onboarding_education_step_impression',

create_account_start = 'create_account_start',

restore_account_start = 'restore_account_start',
Expand Down
14 changes: 0 additions & 14 deletions src/analytics/Properties.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,8 @@ interface AppEventsProperties {
}

interface HomeEventsProperties {
[HomeEvents.hamburger_tapped]: undefined
[HomeEvents.account_circle_tapped]: undefined
[HomeEvents.drawer_navigation]: {
navigateTo: string
}
[HomeEvents.drawer_address_copy]: undefined
[HomeEvents.profile_address_copy]: undefined

[HomeEvents.notification_scroll]: {
// TODO: Pass in notificationType and make param required
notificationType?: NotificationType
Expand Down Expand Up @@ -295,14 +289,6 @@ interface KeylessBackupEventsProperties {
}

interface OnboardingEventsProperties {
[OnboardingEvents.onboarding_education_scroll]: {
currentStep: number
direction: ScrollDirection
}
[OnboardingEvents.onboarding_education_step_impression]: {
step: number
}

[OnboardingEvents.create_account_start]: undefined

[OnboardingEvents.restore_account_start]: undefined
Expand Down
20 changes: 10 additions & 10 deletions src/analytics/ValoraAnalytics.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createClient } from '@segment/analytics-react-native'
import { PincodeType } from 'src/account/reducer'
import { HomeEvents } from 'src/analytics/Events'
import { OnboardingEvents } from 'src/analytics/Events'
import ValoraAnalyticsModule from 'src/analytics/ValoraAnalytics'
import { store } from 'src/redux/store'
import { getDefaultStatsigUser, getDynamicConfigParams, getFeatureGate } from 'src/statsig'
Expand Down Expand Up @@ -240,24 +240,24 @@ describe('ValoraAnalytics', () => {
})

it('delays track calls until async init has finished', async () => {
ValoraAnalytics.track(HomeEvents.drawer_navigation, { navigateTo: 'somewhere' })
ValoraAnalytics.track(OnboardingEvents.pin_invalid, { error: 'some error' })
expect(mockSegmentClient.track).not.toHaveBeenCalled()

await ValoraAnalytics.init()
// Now that init has finished track should have been called
expect(mockSegmentClient.track).toHaveBeenCalledTimes(1)
expect(mockSegmentClient.track).toHaveBeenCalledWith(HomeEvents.drawer_navigation, {
expect(mockSegmentClient.track).toHaveBeenCalledWith(OnboardingEvents.pin_invalid, {
...defaultProperties,
navigateTo: 'somewhere',
error: 'some error',
})

// And now test that track calls go trough directly
mockSegmentClient.track.mockClear()
ValoraAnalytics.track(HomeEvents.drawer_navigation, { navigateTo: 'somewhere else' })
ValoraAnalytics.track(OnboardingEvents.pin_invalid, { error: 'some error' })
expect(mockSegmentClient.track).toHaveBeenCalledTimes(1)
expect(mockSegmentClient.track).toHaveBeenCalledWith(HomeEvents.drawer_navigation, {
expect(mockSegmentClient.track).toHaveBeenCalledWith(OnboardingEvents.pin_invalid, {
...defaultProperties,
navigateTo: 'somewhere else',
error: 'some error',
})
})

Expand Down Expand Up @@ -288,11 +288,11 @@ describe('ValoraAnalytics', () => {

it('adds super properties to all tracked events', async () => {
await ValoraAnalytics.init()
ValoraAnalytics.track(HomeEvents.drawer_navigation, { navigateTo: 'somewhere else' })
ValoraAnalytics.track(OnboardingEvents.pin_invalid, { error: 'some error' })
expect(mockSegmentClient.track).toHaveBeenCalledTimes(1)
expect(mockSegmentClient.track).toHaveBeenCalledWith(HomeEvents.drawer_navigation, {
expect(mockSegmentClient.track).toHaveBeenCalledWith(OnboardingEvents.pin_invalid, {
...defaultProperties,
navigateTo: 'somewhere else',
error: 'some error',
})
})

Expand Down
5 changes: 0 additions & 5 deletions src/analytics/docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ export const eventDocs: Record<AnalyticsEventType, string> = {
[AppEvents.multichain_beta_opt_out]: `When the user taps the No Thanks button on the multichain beta screen`,
[AppEvents.multichain_beta_contact_support]: `When the user taps the Contact Support button on the multichain beta screen`,
[AppEvents.handle_deeplink]: `When a deeplink that leads into the app is detected and handled`,
[HomeEvents.hamburger_tapped]: ``,
[HomeEvents.account_circle_tapped]: `When the account circle used in the tab navigation is tapped`,
[HomeEvents.drawer_navigation]: ``,
[HomeEvents.drawer_address_copy]: ``,
[HomeEvents.profile_address_copy]: `When a user copies their wallet address from the profile screen`,
[HomeEvents.notification_scroll]: ``,
[HomeEvents.notification_impression]: `When the notification appears on the user screen for the first time. Note that the format of the notificationId property for user generated notifications is $NotificationType/$Id, where the $Id can be filtered out with a fuzzy string match.`,
Expand Down Expand Up @@ -161,8 +158,6 @@ export const eventDocs: Record<AnalyticsEventType, string> = {
[KeylessBackupEvents.cab_phone_verification_help_go_back]: `When a user is on the Help bottom sheet on CAB phone verification screen, and they hit go back`,
[KeylessBackupEvents.cab_restore_mnemonic_not_found]: `When a user is restoring from CAB and the mnemonic is not found. Meaning one or more of the user's keyshares/auth methods were incorrect`,
[KeylessBackupEvents.cab_setup_hashed_keyshares]: `When a user is setting up CAB, the hashed keyshares from their phone and email are saved`,
[OnboardingEvents.onboarding_education_scroll]: ``,
[OnboardingEvents.onboarding_education_step_impression]: ``,
[OnboardingEvents.create_account_start]: ``,
[OnboardingEvents.restore_account_start]: ``,
[OnboardingEvents.restore_account_cancel]: ``,
Expand Down

0 comments on commit 52fb14d

Please sign in to comment.