[HOLD for payment 2024-08-20][$500] WS Chat – Blank page opens when go back from IOU thread in a Collect WS chat

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.65-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Log in as Collect WS Employee
3. In a WS chat create a Submit expense
4. Open just created Submit expense
5. Navigate back to WS chat via header link

Expected Result:

Collect WS chat with history opens

Actual Result:

Blank page opens when go back from IOU thread in a Collect WS chat

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6460580_1713980697854.Blank_page.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01dee105cffad79099
• Upwork Job ID: 1783269604755496960
• Last Price Increase: 2024-06-11
• Automatic offers:
• jjcoffee | Contributor | 0
• tsa321 | Contributor | 103410989

Issue Owner

Current Issue Owner: @thesahindia

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01dee105cffad79099

[melvin-bot]

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

[beodw]

Cannot reproduce...

Screen.Recording.2024-04-25.at.01.25.51.mov

[charles-liang]

Proposal

Please re-state the problem that we are trying to solve in this issue.

WS Chat – Blank page opens when go back from IOU thread in a Collect WS chat

What is the root cause of that problem?

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

Lines 164 to 182 in 0f39e43

	const reportActions = useMemo(() => {
	if (!reportActionID) {
	return combinedReportActions;
	}
	if (isLoading || indexOfLinkedAction === -1) {
	return [];
	}
	if (isFirstLinkedActionRender.current) {
	return combinedReportActions.slice(indexOfLinkedAction);
	}
	const paginationSize = getInitialPaginationSize;
	return combinedReportActions.slice(Math.max(indexOfLinkedAction - paginationSize, 0));
	// currentReportActionID is needed to trigger batching once the report action has been positioned
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [reportActionID, combinedReportActions, indexOfLinkedAction, isLoading, currentReportActionID]);

The issue is that when the network is slow, loading is set to true, and then when the page switches, loading remains true, resulting in an empty array being returned. If the page is not refreshed, loading will remain blank, waiting for the response to return.

What changes do you think we should make in order to solve the problem?

remove condition isLoading in Line 169 if not effect another PR. As your comment suggests, even if loading, it shouldn't be blank, should use data what we had. Therefore, I suggest removing it here.

What alternative solutions did you explore? (Optional)

N/A

[charles-liang]

Reproduce method is turn on slow network, like chrome network or emulator setting

[beodw]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Collect WS Chat – Blank page opens when go back from IOU thread in a Collect WS chat

What is the root cause of that problem?

The reportsActions array is temporarily empty in the ReportActionsView while loading new report data on slow connections. This is because it takes a while for the response to be received from the backend and within the reportActionView component we set reportActions to an empty array while loading.

const reportActions = useMemo(() => {
        if (!reportActionID) {
            return combinedReportActions;
        }
        if (isLoading || indexOfLinkedAction === -1) {
            return [];
        }

        if (isFirstLinkedActionRender.current) {
            return combinedReportActions.slice(indexOfLinkedAction);
        }
        const paginationSize = getInitialPaginationSize;
        return combinedReportActions.slice(Math.max(indexOfLinkedAction - paginationSize, 0));

        // currentReportActionID is needed to trigger batching once the report action has been positioned
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [reportActionID, combinedReportActions, indexOfLinkedAction, isLoading, currentReportActionID]);

What changes do you think we should make in order to solve the problem?

In the reportsActionsView component we can return a skeleton loader instead of null when the reportActions array is temporarily empty.

In ReportsActionsView.tsx:

if (!reportActions.length) {
        return null;
    }

Changes to:

if (!reportActions.length) {
        return <ReportActionsSkeletonView />;
    }

What alternative solutions did you explore? (Optional)

None

Screen.Recording.2024-04-25.at.18.26.02.mov

[beodw]

@jjcoffee @sakluger Hi guys, kindly following up on whether this issue has been resolved. If not, please kindly let me know when you will have the capacity to review it. Cheers!

[jjcoffee]

@beodw Thanks for the proposal!

I think the expected result here is probably not to show a loader, as we actually already have all the data we need. It looks like there's something specifically wrong with loading via the route that highlights the expense's reportAction, i.e. /r/{reportID}/{reportActionID}. If you omit the latter part, it loads immediately as expected (regardless of connection speed).

40937-desktop-chrome-2024-04-29_11.18.22.mp4

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sakluger]

@beodw @charles-liang do you have any updated proposals based on @jjcoffee's comment?

[charles-liang]

@sakluger

I don't know if they didn't read my proposal, or if I wasn't clear. I suggested using a cache from the previous, to store the data. Then, when we return, reload it, and then asynchronously refresh the network data. Isn't this the same as @jjcoffee comment?

[jjcoffee]

@charles-liang Apologies for not giving feedback on your proposal. We don't need to implement caching as such. You can see from the issue's video that tapping on the report directly loads it just fine (as I also point out here).

[charles-liang]

@jjcoffee I'm also apology that I miss understand your comment.

[charles-liang]

Proposal

Updated

[jjcoffee]

@marcaaron Yes, looks like #43791 is merged so we're ready to move forward here! Was there anything else you needed to clarify with @tsa321's proposal?

[marcaaron]

It's been a while but I've been waiting on an answer this:

shouldShowSkeleton is specific to ReportScreen and isn't reliant on whether or not we are comment linking. isLoading is used in ReportActionsView specifically for comment linking/pagination. As for why we don't just use shouldShowSkeleton in place of isLoading, I don't think we have a clear answer on that. I did test doing that a while ago and it sort of works, but then the linked report action jumps around on the screen. Maybe @tsa321 can chime in here, or @perunt if he's around.

@tsa321 can you chime in / are you still interested in working on this issue?

[tsa321]

@marcaaron please wait. I will take another look.

[tsa321]

@marcaaron, currently the isLoading:

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

Line 125 in 55a29dd

const isLoading = (!!reportActionID && isLoadingInitialReportActions) || !isReadyForCommentLinking;

The condition !!reportActionID && isLoadingInitialReportActions is already covered by the shouldShowSkeleton of ReportScreen:

App/src/pages/home/ReportScreen.tsx

Line 406 in 55a29dd

(!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions) ||

and:

App/src/pages/home/ReportScreen.tsx

Line 791 in 55a29dd

isReadyForCommentLinking={!shouldShowSkeleton}

The !isReadyForCommentLinking condition also depends on shouldShowSkeleton. Therefore, I believe it is safe to remove the isLoading from ReportActionsView.

[tsa321]

I have created a test branch to show the code changes. I need to remove the (!!reportActionIDFromRoute && !!reportMetadata?.isLoadingInitialReportActions) check in the shouldShowSkeleton of the report screen to display the report actions list immediately, because it is available in the cache.

[melvin-bot]

📣 @tsa321 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[marcaaron]

Ok, let's give it a shot and test.

[jjcoffee]

PR is under review, just one little issue that I think is also there on main, but otherwise testing really well!

[jjcoffee]

I discovered an issue on native (iOS & Android) whilst testing the PR that was also there on main. We decided to fix it as a follow-up and @tsa321 has a solution, so @sakluger would you be able to create an issue and assign us both? (I'm not sure what the process is otherwise!).

Repro steps (video here):

1. From the main workspace chat, submit an expense request using the + button on the left side of the composer.
2. Complete the expense request submission process.
3. Click on the expense preview in the main workspace chat to open the report with all expense previews.
4. Click on the 3rd expense preview from the last (bottom) expense message/last expense preview. This will open the transaction thread report.
5. Immediately go back by clicking on the header link.
6. Try to scroll down

[jjcoffee]

Deployed to production 13 Aug, @sakluger can you update the title to hold for payment for 20 Aug? 🙏

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: HIGH: (Comment linking: Step 4) Main "glue" PR for Comment Linking #30269
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/30269/files#r1642624582
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A - comment linking was a complex PR so it makes sense some issues were missed
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. If there aren't already, add some expenses to any workspace chat, e.g. 5.
2. Open each expense to view the transaction details report.
3. Go back to the workspace chat and submit a new expense using the + button on the left side of the composer.
4. Click on the expense preview in the chat to open the report with all expense previews.
5. Click on the 3rd expense from the bottom. This will open the transaction thread report.
6. Immediately go back by clicking on the header link.
7. Verify that there is no loading skeleton in the report and that the message loads immediately.

Do we agree 👍 or 👎

[jjcoffee]

@sakluger Payment is due today, as this was deployed 13 Aug.

[jjcoffee]

@marcaaron Are you able to create the follow-up issue we discussed? Details here. Or is there some other process we should follow?

[marcaaron]

yes, report bug in #expensify-bugs

[jjcoffee]

@sakluger Friendly bump for payment (the fix was deployed 13 Aug) 🙏

[sakluger]

Summarizing payment on this issue:

Contributor: @tsa321 $500, paid via Upwork
Contributor+: @jjcoffee $500, paid via Upwork

[sakluger]

@jjcoffee thanks for being on top of everything here! And sorry for the delayed payment. All done now 🙇

[jjcoffee]

@sakluger Thanks for making the payment! It looks like the job price didn't update to $500, any way you can correct it?

[sakluger]

@jjcoffee - sorry about that! I added a $250 bonus to the contract to bring the total payment to $500.