Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[NoQA] Chore: Analytics - split SWITCH_REPORT event #46966

Merged
merged 8 commits into from
Aug 11, 2024
Merged

[NoQA] Chore: Analytics - split SWITCH_REPORT event #46966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b66f490
SWITCH_REPORT_THREAD_GO_BACK
kacper-mikolajczak Aug 7, 2024
ee733d8
SWITCH_REPORT_THREAD
kacper-mikolajczak Aug 7, 2024
5263c00
remove SWITCH_REPORT from ReportActionItemParentAction
kacper-mikolajczak Aug 7, 2024
a58504e
SWITCH_REPORT_FROM_PREVIEW
kacper-mikolajczak Aug 7, 2024
dfcd68e
fix linter
kacper-mikolajczak Aug 7, 2024
366d842
remove SWITCH_REPORT_THREAD_GO_BACK
kacper-mikolajczak Aug 7, 2024
b2f59d9
fix linter
kacper-mikolajczak Aug 8, 2024
15ec109
remove REPORT_INITIAL_RENDER
kacper-mikolajczak Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,8 @@ const CONST = {
HOMEPAGE_INITIAL_RENDER: 'homepage_initial_render',
REPORT_INITIAL_RENDER: 'report_initial_render',
SWITCH_REPORT: 'switch_report',
SWITCH_REPORT_FROM_PREVIEW: 'switch_report_from_preview',
SWITCH_REPORT_THREAD: 'switch_report_thread',
SIDEBAR_LOADED: 'sidebar_loaded',
LOAD_SEARCH_OPTIONS: 'load_search_options',
COLD: 'cold',
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ function ReportPreview({
<View style={[styles.chatItemMessage, containerStyles]}>
<PressableWithoutFeedback
onPress={() => {
Timing.start(CONST.TIMING.SWITCH_REPORT);
Timing.start(CONST.TIMING.SWITCH_REPORT_FROM_PREVIEW);
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(iouReportID));
}}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
Expand Down
3 changes: 0 additions & 3 deletions src/pages/home/report/ReportActionItemParentAction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import onyxSubscribe from '@libs/onyxSubscribe';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as Report from '@userActions/Report';
import Timing from '@userActions/Timing';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
Expand Down Expand Up @@ -135,7 +133,6 @@ function ReportActionItemParentAction({
// Pop the chat report screen before navigating to the linked report action.
Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.parentReportID ?? '-1', ancestor.reportAction.reportActionID));
}
Timing.start(CONST.TIMING.SWITCH_REPORT);
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
}
: undefined
}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemThread.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function ReportActionItemThread({numberOfReplies, icons, mostRecentReply, childR
<PressableWithSecondaryInteraction
onPress={() => {
Report.navigateToAndOpenChildReport(childReportID);
Timing.start(CONST.TIMING.SWITCH_REPORT);
Timing.start(CONST.TIMING.SWITCH_REPORT_THREAD);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, by the way, why do we start the timer after initiating a navigation? Shouldn't we call Timing.start before navigateToAndOpenChildReport? And could this be the reason for the high timings in the graph?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can discuss but its NAB for this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mountiny Do you think we can create another issue for this and discuss? Or if there is another related issue on analytics we can add this issue there and discuss.

}}
role={CONST.ROLE.BUTTON}
accessibilityLabel={`${numberOfReplies} ${replyText}`}
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/report/ReportActionsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,12 +448,13 @@ function ReportActionsView({
if (!ReportActionsView.initMeasured) {
Performance.markEnd(CONST.TIMING.OPEN_REPORT);
Performance.markEnd(CONST.TIMING.REPORT_INITIAL_RENDER);
Timing.end(CONST.TIMING.REPORT_INITIAL_RENDER);
ReportActionsView.initMeasured = true;
} else {
Performance.markEnd(CONST.TIMING.SWITCH_REPORT);
}
Timing.end(CONST.TIMING.SWITCH_REPORT, hasCachedActionOnFirstRender ? CONST.TIMING.WARM : CONST.TIMING.COLD);
Timing.end(CONST.TIMING.SWITCH_REPORT_THREAD);
Timing.end(CONST.TIMING.SWITCH_REPORT_FROM_PREVIEW);
}, [hasCachedActionOnFirstRender]);

// Check if the first report action in the list is the one we're currently linked to
Expand Down
2 changes: 0 additions & 2 deletions src/pages/home/report/ThreadDivider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import Navigation from '@libs/Navigation/Navigation';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import type {Ancestor} from '@libs/ReportUtils';
import variables from '@styles/variables';
import Timing from '@userActions/Timing';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';

Expand Down Expand Up @@ -48,7 +47,6 @@ function ThreadDivider({ancestor, isLinkDisabled = false}: ThreadDividerProps) {
) : (
<PressableWithoutFeedback
onPress={() => {
Timing.start(CONST.TIMING.SWITCH_REPORT);
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
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'));
Expand Down
Loading