From caab39bececcb7533afd77d3e000c6aa0c9dabc9 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Fri, 9 Aug 2024 15:50:50 +0200 Subject: [PATCH] move getAttributes to firebase module, add mocks for tests --- .../ReportActionItem/ReportPreview.tsx | 3 +- src/libs/Firebase/index.native.ts | 31 +++++++++++++------ src/libs/Firebase/types.ts | 1 + src/libs/Firebase/utils.ts | 18 ----------- .../Navigation/AppNavigator/AuthScreens.tsx | 3 +- .../createCustomBottomTabNavigator/TopBar.tsx | 3 +- src/libs/OptionsListUtils.ts | 3 +- src/libs/actions/App.ts | 5 ++- src/pages/home/ReportScreen.tsx | 3 +- .../report/ReportActionItemParentAction.tsx | 3 +- .../home/report/ReportActionItemThread.tsx | 3 +- src/pages/home/report/ThreadDivider.tsx | 3 +- .../SidebarScreen/BaseSidebarScreen.tsx | 5 ++- tests/perf-test/BaseOptionsList.perf-test.tsx | 5 +++ tests/perf-test/ChatFinderPage.perf-test.tsx | 5 +++ tests/perf-test/OptionsListUtils.perf-test.ts | 5 +++ tests/unit/OptionsListUtilsTest.ts | 5 +++ 17 files changed, 55 insertions(+), 49 deletions(-) delete mode 100644 src/libs/Firebase/utils.ts diff --git a/src/components/ReportActionItem/ReportPreview.tsx b/src/components/ReportActionItem/ReportPreview.tsx index de4753d05710..6d326b2d9eb9 100644 --- a/src/components/ReportActionItem/ReportPreview.tsx +++ b/src/components/ReportActionItem/ReportPreview.tsx @@ -21,7 +21,6 @@ import useThemeStyles from '@hooks/useThemeStyles'; import ControlSelection from '@libs/ControlSelection'; import * as CurrencyUtils from '@libs/CurrencyUtils'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; -import getAttributes from '@libs/Firebase/utils'; import Navigation from '@libs/Navigation/Navigation'; import * as PolicyUtils from '@libs/PolicyUtils'; import * as ReceiptUtils from '@libs/ReceiptUtils'; @@ -374,7 +373,7 @@ function ReportPreview({ { - Timing.start(CONST.TIMING.SWITCH_REPORT, getAttributes()); + Timing.start(CONST.TIMING.SWITCH_REPORT); Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(iouReportID)); }} onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} diff --git a/src/libs/Firebase/index.native.ts b/src/libs/Firebase/index.native.ts index 15333c369866..a54b9f39d3a5 100644 --- a/src/libs/Firebase/index.native.ts +++ b/src/libs/Firebase/index.native.ts @@ -2,12 +2,14 @@ import crashlytics from '@react-native-firebase/crashlytics'; import perf from '@react-native-firebase/perf'; import * as Environment from '@libs/Environment/Environment'; +import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; +import * as ReportUtils from '@libs/ReportUtils'; import * as SessionUtils from '@libs/SessionUtils'; -import type {Log, StartTrace, StopTrace, TraceMap} from './types'; +import type {FirebaseAttributes, Log, StartTrace, StopTrace, TraceMap} from './types'; const traceMap: TraceMap = {}; -const startTrace: StartTrace = (customEventName, attributes) => { +const startTrace: StartTrace = (customEventName) => { const start = global.performance.now(); if (Environment.isDevelopment()) { return; @@ -17,17 +19,14 @@ const startTrace: StartTrace = (customEventName, attributes) => { return; } - const session = SessionUtils.getSession(); + const attributes = getAttributes(); perf() .startTrace(customEventName) .then((trace) => { - trace.putAttribute('accountID', session?.accountID?.toString() ?? 'N/A'); - if (attributes) { - Object.entries(attributes).forEach(([name, value]) => { - trace.putAttribute(name, value); - }); - } + Object.entries(attributes).forEach(([name, value]) => { + trace.putAttribute(name, value); + }); traceMap[customEventName] = { trace, start, @@ -60,6 +59,20 @@ const log: Log = (action: string) => { crashlytics().log(action); }; +function getAttributes(): FirebaseAttributes { + const session = SessionUtils.getSession(); + + const accountId = session?.accountID?.toString() ?? 'N/A'; + const reportsLength = ReportUtils.getAllReportsLength().toString(); + const personalDetailsLength = PersonalDetailsUtils.getPersonalDetailsLength().toString(); + + return { + accountId, + reportsLength, + personalDetailsLength, + }; +} + export default { startTrace, stopTrace, diff --git a/src/libs/Firebase/types.ts b/src/libs/Firebase/types.ts index 5dda391d1894..a30017999225 100644 --- a/src/libs/Firebase/types.ts +++ b/src/libs/Firebase/types.ts @@ -9,6 +9,7 @@ type StartTrace = (customEventName: string, attributes?: FirebaseAttributes) => type StopTrace = (customEventName: string) => void; type Log = (action: string) => void; type FirebaseAttributes = { + accountId: string; personalDetailsLength: string; reportsLength: string; }; diff --git a/src/libs/Firebase/utils.ts b/src/libs/Firebase/utils.ts deleted file mode 100644 index 28fa29ffc80d..000000000000 --- a/src/libs/Firebase/utils.ts +++ /dev/null @@ -1,18 +0,0 @@ -import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; -import * as ReportUtils from '@libs/ReportUtils'; -import type {FirebaseAttributes} from './types'; - -/** - * Gets attributes for firebase trace - */ -function getAttributes(): FirebaseAttributes { - const reportsLength = ReportUtils.getAllReportsLength().toString(); - const personalDetailsLength = PersonalDetailsUtils.getPersonalDetailsLength().toString(); - - return { - reportsLength, - personalDetailsLength, - }; -} - -export default getAttributes; diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.tsx b/src/libs/Navigation/AppNavigator/AuthScreens.tsx index 9d64ab2c8f5e..64816562a507 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.tsx +++ b/src/libs/Navigation/AppNavigator/AuthScreens.tsx @@ -11,7 +11,6 @@ import useStyleUtils from '@hooks/useStyleUtils'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import {READ_COMMANDS} from '@libs/API/types'; -import getAttributes from '@libs/Firebase/utils'; import HttpUtils from '@libs/HttpUtils'; import KeyboardShortcut from '@libs/KeyboardShortcut'; import Log from '@libs/Log'; @@ -211,7 +210,7 @@ function AuthScreens() { let initialReportID: string | undefined; const isInitialRender = useRef(true); if (isInitialRender.current) { - Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER, getAttributes()); + Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); const currentURL = getCurrentUrl(); if (currentURL) { diff --git a/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar.tsx b/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar.tsx index 671a680a62cf..0f4c41b9cbfc 100644 --- a/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar.tsx +++ b/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar.tsx @@ -10,7 +10,6 @@ import WorkspaceSwitcherButton from '@components/WorkspaceSwitcherButton'; import useLocalize from '@hooks/useLocalize'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; -import getAttributes from '@libs/Firebase/utils'; import Navigation from '@libs/Navigation/Navigation'; import Performance from '@libs/Performance'; import SignInButton from '@pages/home/sidebar/SignInButton'; @@ -66,7 +65,7 @@ function TopBar({breadcrumbLabel, activeWorkspaceID, shouldDisplaySearch = true} accessibilityLabel={translate('sidebarScreen.buttonFind')} style={[styles.flexRow, styles.mr2, styles.touchableButtonImage]} onPress={Session.checkIfActionIsAllowed(() => { - Timing.start(CONST.TIMING.CHAT_FINDER_RENDER, getAttributes()); + Timing.start(CONST.TIMING.CHAT_FINDER_RENDER); Performance.markStart(CONST.TIMING.CHAT_FINDER_RENDER); Navigation.navigate(ROUTES.CHAT_FINDER); })} diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index f313f4a1a2c3..b9be3ffac761 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -42,7 +42,6 @@ import times from '@src/utils/times'; import Timing from './actions/Timing'; import * as ErrorUtils from './ErrorUtils'; import filterArrayByMatch from './filterArrayByMatch'; -import getAttributes from './Firebase/utils'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; import * as Localize from './Localize'; @@ -2078,7 +2077,7 @@ function getOptions( * Build the options for the Search view */ function getSearchOptions(options: OptionList, searchValue = '', betas: Beta[] = []): Options { - Timing.start(CONST.TIMING.LOAD_SEARCH_OPTIONS, getAttributes()); + Timing.start(CONST.TIMING.LOAD_SEARCH_OPTIONS); Performance.markStart(CONST.TIMING.LOAD_SEARCH_OPTIONS); const optionList = getOptions(options, { betas, diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts index c9133d6af63c..2a12b679972a 100644 --- a/src/libs/actions/App.ts +++ b/src/libs/actions/App.ts @@ -11,7 +11,6 @@ import type {GetMissingOnyxMessagesParams, HandleRestrictedEventParams, OpenAppP import {SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types'; import * as Browser from '@libs/Browser'; import {buildEmojisTrie} from '@libs/EmojiTrie'; -import getAttributes from '@libs/Firebase/utils'; import Log from '@libs/Log'; import getCurrentUrl from '@libs/Navigation/currentUrl'; import Navigation from '@libs/Navigation/Navigation'; @@ -265,7 +264,7 @@ function reconnectApp(updateIDFrom: OnyxEntry = 0) { // // - Look through the local report actions and reports to find the most recently modified report action or report. // - We send this to the server so that it can compute which new chats the user needs to see and return only those as an optimization. - Timing.start(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION, getAttributes()); + Timing.start(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION); params.mostRecentReportActionLastModified = ReportActionsUtils.getMostRecentReportActionLastModified(); Timing.end(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION, '', 500); @@ -294,7 +293,7 @@ function finalReconnectAppAfterActivatingReliableUpdates(): Promise { Report.navigateToAndOpenChildReport(childReportID); - Timing.start(CONST.TIMING.SWITCH_REPORT, getAttributes()); + Timing.start(CONST.TIMING.SWITCH_REPORT); }} role={CONST.ROLE.BUTTON} accessibilityLabel={`${numberOfReplies} ${replyText}`} diff --git a/src/pages/home/report/ThreadDivider.tsx b/src/pages/home/report/ThreadDivider.tsx index d28c8220ba90..5b3fe6e5986c 100644 --- a/src/pages/home/report/ThreadDivider.tsx +++ b/src/pages/home/report/ThreadDivider.tsx @@ -8,7 +8,6 @@ import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; -import getAttributes from '@libs/Firebase/utils'; import Navigation from '@libs/Navigation/Navigation'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import type {Ancestor} from '@libs/ReportUtils'; @@ -49,7 +48,7 @@ function ThreadDivider({ancestor, isLinkDisabled = false}: ThreadDividerProps) { ) : ( { - Timing.start(CONST.TIMING.SWITCH_REPORT, getAttributes()); + Timing.start(CONST.TIMING.SWITCH_REPORT); const isVisibleAction = ReportActionsUtils.shouldReportActionBeVisible(ancestor.reportAction, ancestor.reportAction.reportActionID ?? '-1'); // Pop the thread report screen before navigating to the chat report. Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID ?? '-1')); diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx index a4614d2e530f..edc8dfb3cb3a 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx @@ -7,7 +7,6 @@ import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; import {updateLastAccessedWorkspace} from '@libs/actions/Policy/Policy'; import * as Browser from '@libs/Browser'; -import getAttributes from '@libs/Firebase/utils'; import TopBar from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar'; import Navigation from '@libs/Navigation/Navigation'; import Performance from '@libs/Performance'; @@ -20,7 +19,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; * Function called when a pinned chat is selected. */ const startTimer = () => { - Timing.start(CONST.TIMING.SWITCH_REPORT, getAttributes()); + Timing.start(CONST.TIMING.SWITCH_REPORT); Performance.markStart(CONST.TIMING.SWITCH_REPORT); }; @@ -32,7 +31,7 @@ function BaseSidebarScreen() { useEffect(() => { Performance.markStart(CONST.TIMING.SIDEBAR_LOADED); - Timing.start(CONST.TIMING.SIDEBAR_LOADED, getAttributes()); + Timing.start(CONST.TIMING.SIDEBAR_LOADED); }, []); useEffect(() => { diff --git a/tests/perf-test/BaseOptionsList.perf-test.tsx b/tests/perf-test/BaseOptionsList.perf-test.tsx index dc5768610861..3ef7be6127af 100644 --- a/tests/perf-test/BaseOptionsList.perf-test.tsx +++ b/tests/perf-test/BaseOptionsList.perf-test.tsx @@ -11,6 +11,11 @@ type BaseOptionsListWrapperProps = { canSelectMultipleOptions?: boolean; }; +jest.mock('@src/libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); + describe('[BaseOptionsList]', () => { function BaseOptionsListWrapper({canSelectMultipleOptions = false}: BaseOptionsListWrapperProps) { const [selectedIds, setSelectedIds] = useState([]); diff --git a/tests/perf-test/ChatFinderPage.perf-test.tsx b/tests/perf-test/ChatFinderPage.perf-test.tsx index 4346977a1cd0..98d84b3eb035 100644 --- a/tests/perf-test/ChatFinderPage.perf-test.tsx +++ b/tests/perf-test/ChatFinderPage.perf-test.tsx @@ -71,6 +71,11 @@ jest.mock('@react-navigation/native', () => { }; }); +jest.mock('@src/libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); + jest.mock('@src/components/withNavigationFocus', () => (Component: ComponentType) => { function WithNavigationFocus(props: WithNavigationFocusProps) { return ( diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index 16522297a416..695c96e92d18 100644 --- a/tests/perf-test/OptionsListUtils.perf-test.ts +++ b/tests/perf-test/OptionsListUtils.perf-test.ts @@ -73,6 +73,11 @@ jest.mock('@react-navigation/native', () => { }; }); +jest.mock('@src/libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); + const options = OptionsListUtils.createOptionList(personalDetails, reports); /* GetOption is the private function and is never called directly, we are testing the functions which call getOption with different params */ diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 34a0a9af7383..73632329a230 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -13,6 +13,11 @@ import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; type PersonalDetailsList = Record; +jest.mock('@src/libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); + describe('OptionsListUtils', () => { // Given a set of reports with both single participants and multiple participants some pinned and some not const REPORTS: OnyxCollection = {