HIGH: [Reliability] [$250] Two OpenReports when opening a room

[quinthar]

Problem:

Room switching is very slow, including a lot of API calls that seem redundant or unnecessary. Our 1:1:1 principle isn't being followed. Switching to a new room makes:

• Two identical calls to OpenReport (should make 1)
• Four identical calls to GetNewerMessages (should make zero, as OpenReport should return what we need)

It is doing 6 separate API calls, even though in theory it should only make one call to OpenReport.

Solution:

Audit room opening to figure out why switching to a room does so many unnecessary API calls. Here's a list of the network connections from switching to the #exfy-roadmap room for the first time:

Here are the payloads:

OpenReport #1:

reportID: 868417741568295
reportActionID: 
emailList: 
accountIDList: 
parentReportActionID: 0
clientLastReadTime: 2024-03-10 23:34:02.144
appversion: 1.4.62-6
apiRequestType: write
pusherSocketID: 698458.778938
shouldRetry: true
canCancel: true
authToken: F0D774A253499AC56DE79A0F2455EE2CD57762EC9B74121B02D82340CFC275BCDE2332022ACCB4C26E6343463B62AE53D60F9A5B798AB8B18063E99885BB9F729E2ABDA84A847AEDBD160E007D36E8337149981E6EE5394E3189A1BA62C8C160605D5C91386A164C5B749846CAC14FD12DC36E27559D8DD65BE14E1C848805AEABE60B2F8709B9FCCBBB28382129A9B2C6068C77752392B8F4F402A2F5D80193D2C2277657926FF85029C4DB00293DACA086F1697D16B397065DA4910D496AB236BD3EE25718C588D552119A5AA4F8FC31D563F4EF1096E73B141A26DF17B9A887B766FEF514D0A9E0CEEBF80E5446FF1C5EA845119582399F391C8120CD53C997060EBC8381B7A70F78D0A5F26FB7D8688DA253ED9AB56F357658FACA3EE57FFD9104E59C9C5360E8F2454024313FFD332EC85F151FB98AA6698AC4EA47024DF52805401B941AF27F2244CC846E1EA7E6433C8609CA218520245B1A4502A049
referer: ecash
platform: web
api_setCookie: false
email: dbarrett@expensify.com
isFromDevEnv: false

Response:

OpenReport #2:

Note: It doesn't have a reportActionID. Granted, the first OpenReport had it and it was blank, but it's the only difference.

reportID: 868417741568295
emailList: 
accountIDList: 
parentReportActionID: 0
clientLastReadTime: 2024-03-10 23:34:02.144
appversion: 1.4.62-6
apiRequestType: write
pusherSocketID: 698458.778938
shouldRetry: true
canCancel: true
authToken: F0D774A253499AC56DE79A0F2455EE2CD57762EC9B74121B02D82340CFC275BCDE2332022ACCB4C26E6343463B62AE53D60F9A5B798AB8B18063E99885BB9F729E2ABDA84A847AEDBD160E007D36E8337149981E6EE5394E3189A1BA62C8C160605D5C91386A164C5B749846CAC14FD12DC36E27559D8DD65BE14E1C848805AEABE60B2F8709B9FCCBBB28382129A9B2C6068C77752392B8F4F402A2F5D80193D2C2277657926FF85029C4DB00293DACA086F1697D16B397065DA4910D496AB236BD3EE25718C588D552119A5AA4F8FC31D563F4EF1096E73B141A26DF17B9A887B766FEF514D0A9E0CEEBF80E5446FF1C5EA845119582399F391C8120CD53C997060EBC8381B7A70F78D0A5F26FB7D8688DA253ED9AB56F357658FACA3EE57FFD9104E59C9C5360E8F2454024313FFD332EC85F151FB98AA6698AC4EA47024DF52805401B941AF27F2244CC846E1EA7E6433C8609CA218520245B1A4502A049
referer: ecash
platform: web
api_setCookie: false
email: dbarrett@expensify.com
isFromDevEnv: false

Response:

GetNewerMessages (all are identical payloads)

reportID: 868417741568295
reportActionID: 1296882577368295752
appversion: 1.4.62-6
apiRequestType: read
authToken: F0D774A253499AC56DE79A0F2455EE2CD57762EC9B74121B02D82340CFC275BCDE2332022ACCB4C26E6343463B62AE53D60F9A5B798AB8B18063E99885BB9F729E2ABDA84A847AEDBD160E007D36E8337149981E6EE5394E3189A1BA62C8C160605D5C91386A164C5B749846CAC14FD12DC36E27559D8DD65BE14E1C848805AEABE60B2F8709B9FCCBBB28382129A9B2C6068C77752392B8F4F402A2F5D80193D2C2277657926FF85029C4DB00293DACA086F1697D16B397065DA4910D496AB236BD3EE25718C588D552119A5AA4F8FC31D563F4EF1096E73B141A26DF17B9A887B766FEF514D0A9E0CEEBF80E5446FF1C5EA845119582399F391C8120CD53C997060EBC8381B7A70F78D0A5F26FB7D8688DA253ED9AB56F357658FACA3EE57FFD9104E59C9C5360E8F2454024313FFD332EC85F151FB98AA6698AC4EA47024DF52805401B941AF27F2244CC846E1EA7E6433C8609CA218520245B1A4502A049
referer: ecash
platform: web
api_setCookie: false
email: dbarrett@expensify.com
isFromDevEnv: false

Response: (All identical, except for different "requestID": "8747cf3479c42763-SEA".

{
    "onyxData": [
        {
            "onyxMethod": "merge",
            "key": "reportActions_868417741568295",
            "value": {
                "1296882577368295752": {
                    "person": [
                        {
                            "type": "TEXT",
                            "style": "strong",
                            "text": "David Barrett (EXFY CEO)"
                        }
                    ],
                    "actorAccountID": 1,
                    "message": [
                        {
                            "type": "COMMENT",
                            "html": "This room offers a glimpse into the future of Expensify, showcasing the exciting new features that we\u2019ll be releasing soon. Our main goal here is to focus solely on discussions around Expensify&#x27;s product roadmap. <br \/><br \/>These discussions may contain forward-looking statements based on our current beliefs and expectations, and are subject to various risks and uncertainties described in our SEC filings. Please note that any irrelevant content or off-topic discussion will be promptly removed by our moderators. <br \/><br \/>Please scroll up to the top of the room to join the conversation. We encourage you to dive into the specifics of our roadmap, and warmly invite any questions or feedback on the content we discuss. Thanks for joining us!",
                            "text": "This room offers a glimpse into the future of Expensify, showcasing the exciting new features that we\u2019ll be releasing soon. Our main goal here is to focus solely on discussions around Expensify's product roadmap. \n\nThese discussions may contain forward-looking statements based on our current beliefs and expectations, and are subject to various risks and uncertainties described in our SEC filings. Please note that any irrelevant content or off-topic discussion will be promptly removed by our moderators. \n\nPlease scroll up to the top of the room to join the conversation. We encourage you to dive into the specifics of our roadmap, and warmly invite any questions or feedback on the content we discuss. Thanks for joining us!",
                            "isEdited": false,
                            "whisperedTo": [
                                16626091
                            ],
                            "isDeletedParentAction": false,
                            "deleted": "",
                            "reactions": []
                        }
                    ],
                    "originalMessage": {
                        "html": "This room offers a glimpse into the future of Expensify, showcasing the exciting new features that we\u2019ll be releasing soon. Our main goal here is to focus solely on discussions around Expensify&#x27;s product roadmap. <br \/><br \/>These discussions may contain forward-looking statements based on our current beliefs and expectations, and are subject to various risks and uncertainties described in our SEC filings. Please note that any irrelevant content or off-topic discussion will be promptly removed by our moderators. <br \/><br \/>Please scroll up to the top of the room to join the conversation. We encourage you to dive into the specifics of our roadmap, and warmly invite any questions or feedback on the content we discuss. Thanks for joining us!",
                        "lastModified": "2024-04-14 01:12:03.118",
                        "type": "automated",
                        "whisperedTo": [
                            16626091
                        ]
                    },
                    "avatar": "https:\/\/d1wpcgnaa73g0y.cloudfront.net\/ef06eb8856c0e2370ac7e83b7c71c29d736761ca_128.jpeg",
                    "created": "2024-04-14 01:12:03.118",
                    "timestamp": 1713057123,
                    "reportActionTimestamp": 1713057123118,
                    "automatic": false,
                    "actionName": "ADDCOMMENT",
                    "shouldShow": true,
                    "reportActionID": "1296882577368295752",
                    "previousReportActionID": "7992506713650258309",
                    "lastModified": "2024-04-14 01:12:03.118",
                    "actorEmail": "dbarrett@expensify.com",
                    "whisperedToAccountIDs": [
                        16626091
                    ]
                }
            }
        }
    ],
    "jsonCode": 200,
    "requestID": "8747cf3479c42763-SEA"
}

Bonus track:

Diagnose and fix why switching back to that same room in the future (ie, after it's already loaded) still calls:

• Two calls to GetNewerMessages (it should be zero, and especially shouldn't call _before OpenReport)
• One call to OpenReport (this is correct)

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011c68fdadf05c3556
• Upwork Job ID: 1779672975704821760
• Last Price Increase: 2024-04-15

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011c68fdadf05c3556

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[skyweb331]

I am checking this but one strange thing is , on development env, it just calls one OpenReport and two GetNewerMessages. And on staging, it calls double times than local. is it because of some configurations on staging???

Still investigating not to make GetNewerMessages api calls...

[twisterdotcom]

Is this related or a dupe of #39674 (comment)?

[nayabatir1]

Here is what I found, fetchReport and openReportIfNecessary both uses same reportID and reportActionID to make api call. I'm not sure but I think openReportIfNecessary function can be removed.

App/src/pages/home/ReportScreen.tsx

Lines 411 to 430 in 2d19c95

	const fetchReportIfNeeded = useCallback(() => {
	// Report ID will be empty when the reports collection is empty.
	// This could happen when we are loading the collection for the first time after logging in.
	if (!ReportUtils.isValidReportIDFromPath(reportIDFromRoute)) {
	return;
	}
	// It is possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
	// is not stored locally yet. If report.reportID exists, then the report has been stored locally and nothing more needs to be done.
	// If it doesn't exist, then we fetch the report from the API.
	if (report.reportID && report.reportID === reportIDFromRoute && !reportMetadata?.isLoadingInitialReportActions) {
	return;
	}
	if (!shouldFetchReport(report)) {
	return;
	}
	fetchReport();
	}, [report, reportMetadata?.isLoadingInitialReportActions, fetchReport, reportIDFromRoute]);

App/src/pages/home/ReportScreen.tsx

Lines 390 to 392 in 2d19c95

	const fetchReport = useCallback(() => {
	Report.openReport(reportIDFromRoute, reportActionIDFromRoute);
	}, [reportIDFromRoute, reportActionIDFromRoute]);

App/src/pages/home/report/ReportActionsView.tsx

Lines 113 to 119 in 2d19c95

	const openReportIfNecessary = () => {
	if (!shouldFetchReport(report)) {
	return;
	}
	Report.openReport(reportID, reportActionID);
	};

Same report is being used in ReportScreen.tsx and ReportActionsView.tsx

App/src/pages/home/ReportScreen.tsx

Lines 676 to 687 in 2d19c95

	{shouldShowReportActionList && (
	<ReportActionsView
	reportActions={reportActions}
	report={report}
	parentReportAction={parentReportAction}
	isLoadingInitialReportActions={reportMetadata?.isLoadingInitialReportActions}
	isLoadingNewerReportActions={reportMetadata?.isLoadingNewerReportActions}
	isLoadingOlderReportActions={reportMetadata?.isLoadingOlderReportActions}
	isReadyForCommentLinking={!shouldShowSkeleton}
	transactionThreadReportID={transactionThreadReportID}
	/>
	)}

[quinthar]

Let's reduce the scope of this issue to simply: "Two OpenReports when opening a room", and the other issue can work on the redundant GetNewerMessages.

[quinthar]

Just happened again, btw. It doesn't happen every time on every report. But clicking around on a few reports and I'll see it, I think maybe longer reports. Please test on Staging.

[bernhardoj]

The double OpenReport is the same issue as this one

[twisterdotcom]

Looks like no Bug label was added here. Looks like @bernhardoj is going to handle it in the above issue, so will close this anyway.