[$250] Chat - Chat reloads when changing from #focus to Most recent mode

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.20-0 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): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Trigger #focus modal pop up by pasting Onyx.set('focusModeNotification', true); in the console
3. Click Settings on the pop up
4. Select Most recent

Expected Result:

Chat report will not reload

Actual Result:

Chat report reloads when changing from #focus to Most recent mode This issue also occurs on mobile environment when the pop up appears in chat and user changes to Most recent mode

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/69818983-f5c8-4506-907f-7cfc0e2a901d

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e8e6bd741ebc762d
• Upwork Job ID: 1825661286720810919
• Last Price Increase: 2024-08-26

Issue Owner

Current Issue Owner: @jayeshmangwani

[melvin-bot[bot]]

Triggered auto assignment to @strepanier03 (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]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@strepanier03 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

[daledah]

Edited by proposal-police: This proposal was edited at {2023-10-02T06:30:00Z}.

Proposal

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

Chat report reloads when changing from #focus to Most recent mode This issue also occurs on mobile environment when the pop up appears in chat and user changes to Most recent mode

What is the root cause of that problem?

Changing focus to recent mode will trigger OpenApp API, that set isLoadingApp=true optimistically.

When isLoadingApp=true, we have the logic to show the skeleton

https://github.com/Expensify/App/blob/fcf4b4f03721ae3acefa33873cd8b2e2b13ba671/src/pages/home/ReportScreen.tsx#L397-L399

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

We should detect the the app is already loaded before. Then use it to check whether we should show the skeleton or not

    const firstTimeAppLoadedRef = useRef(false);
    useEffect(()=>{
        if(firstTimeAppLoadedRef.current || !finishedLoadingApp){
            return;
        }
        firstTimeAppLoadedRef.current = true
    },[finishedLoadingApp])

...

    const isLoading = (isLoadingApp && !firstTimeAppLoadedRef.current) || !reportIDFromRoute || (!isSidebarLoaded && !isInNarrowPaneModal) || PersonalDetailsUtils.isPersonalDetailsEmpty();

What alternative solutions did you explore? (Optional)

We can detect the report is already loaded by using isLoading (the first time this value is false)

Then use it to check shouldShowSkeleton

    const shouldShowSkeleton =
        (isLinkingToMessage && !isLinkedMessagePageReady) || (!isLinkingToMessage && !isInitialPageReady) || isLoadingReportOnyx || !isCurrentReportLoadedFromOnyx || (isLoading && isFirstTimeLoaded.current);

[melvin-bot[bot]]

@strepanier03 Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[strepanier03]

Repro just fine on staging v9.0.22-5.

[jayeshmangwani]

This issue also occurs on mobile environment when the pop up appears in chat and user changes to Most recent mode

@daledah You mentioned that this is only available in a mobile environment. Could you please help me understand why that is?

[jayeshmangwani]

Thank you for the proposal @daledah , but your solution doesn't work for me in two scenarios:

1. Start a new chat with a user, send a few messages, then follow the steps from the description.
2. Go to any IOU report and follow the steps from the description.

I am attaching the video below. Please check it for reference.

https://github.com/user-attachments/assets/2b6873c0-816b-4f93-b5f8-63cfba65053f

[huult]

Proposal

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

Chat reloads when changing from #focus to Most recent mode

What is the root cause of that problem?

When changing from #focus mode to Most Recent mode, the functionupdateChatPriorityMode will execute https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/libs/actions/User.ts#L733-L742 nextPriorityMode will be set to default, and priorityMode will be set to GSD. Then, openApp will execute, which causes the issue to occur. https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/libs/actions/App.ts#L68-L78

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

As commented "When someone switches their priority mode we need to fetch all their chats because only #focus mode works with a subset of a user's chats. This is only possible via the OpenApp command." we need to call openApp to fetch subset of a user's chat.

I think this ticket is not a bug. To improve the user experience without showing a skeleton, some thing like that:

// src/libs/actions/App.ts#L74
- openApp();
+ openApp(false);

// src/libs/actions/App.ts#L247
- function openApp() {
+ function openApp(isLoading?: boolean) { 
    getPolicyParamsForOpenOrReconnect().then((policyParams: PolicyParamsForOpenOrReconnect) => {
        const params: OpenAppParams = {enablePriorityModeFilter: true, ...policyParams};
-        API.write(WRITE_COMMANDS.OPEN_APP, params, getOnyxDataForOpenOrReconnect(true));
+.      API.write(WRITE_COMMANDS.OPEN_APP, params, getOnyxDataForOpenOrReconnect(true, isLoading));
    });
}

// src/libs/actions/App.ts#L204
- function getOnyxDataForOpenOrReconnect(isOpenApp = false): OnyxData {
+ function getOnyxDataForOpenOrReconnect(isOpenApp = false, isLoading = true): OnyxData {
    const defaultData = {
        optimisticData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: ONYXKEYS.IS_LOADING_REPORT_DATA,
                value: true,
            },
        ],
        finallyData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: ONYXKEYS.IS_LOADING_REPORT_DATA,
                value: false,
            },
        ],
    };
    if (!isOpenApp) {
        return defaultData;
    }
    return {
        optimisticData: [
            ...defaultData.optimisticData,
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: ONYXKEYS.IS_LOADING_APP,
-                value: true,
+                value: isLoading,
            },
        ],
        finallyData: [
            ...defaultData.finallyData,
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: ONYXKEYS.IS_LOADING_APP,
                value: false,
            },
        ],
    };
}

What alternative solutions did you explore? (Optional)

If we know the current report data is the latest, there's no need to call openApp. We just need to update the condition like this

// src/libs/actions/App.ts#L72
- if (nextPriorityMode === CONST.PRIORITY_MODE.DEFAULT && priorityMode === CONST.PRIORITY_MODE.GSD) {
+ if (nextPriorityMode === CONST.PRIORITY_MODE.DEFAULT && priorityMode === CONST.PRIORITY_MODE.GSD && !isNewestReportData) {

Note:isNewestReportData does not currently exist. This is just an example in the code

[jayeshmangwani]

I asked on #expensify-open-source whether we really need to fix this issue, considering that a chat reload should be expected when calling the openApp command.

[daledah]

@jayeshmangwani finishedLoadingApp seems to be incorrect when users open the new chat, because we don't call other OpenApp in that case. I think we can use my alternative solution. isLoading=false means the report is already loaded, so we shouldn't show the skeleton again.

[melvin-bot[bot]]

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

[jayeshmangwani]

@daledah Can you please clarify what 'isFirstTimeLoaded' refers to in your alternate proposal?

[jayeshmangwani]

Tested the changes from the @huult 's Proposal, and they do work. So, if we don't want to show the skeleton when changing the priority mode, we can go with the @huult 's Proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[thienlnam]

Hmm, I'm not entirely convinced we need to fix this - but if it's just a case of correctly determining when to update the loading then it's something we can do here

[jayeshmangwani]

I was also feeling the same, opened a Slack discussion https://github.com/Expensify/App/issues/47412#issuecomment-2308487342, but it seems not getting any eyes on it

[thienlnam]

yeah, after another look at the proposal - I don't think we should add this by adding a loading parameter into openApp that is only used for this. Just going to close this