[$250] Deeplink - Duplicate concierge chat is displayed on LHN when deeplinking to concierge

[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.2-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 Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/44063

Action Performed:

1. Sign out from ND
2. Paste this link to the browser tab https://staging.new.expensify.com/concierge
3. Press Enter key
4. Enter a new user email address in the email field
5. Click Continue button
6. Click the Join button
7. While staying in the onboarding process, observe the LHN

Expected Result:

Only one concierge chat is displayed on the LHN

Actual Result:

Duplicate concierge chat is briefly displayed on the LHN

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/Expensify/App/assets/78819774/f12f02f1-b0a5-4f87-a2b3-7d3304ea975c

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01761bb8fe9958d8df
• Upwork Job ID: 1808128563862616294
• Last Price Increase: 2024-10-02

Issue Owner

Current Issue Owner: @getusha

[melvin-bot[bot]]

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

@sonialiap 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 #vip-vsp

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01761bb8fe9958d8df

[melvin-bot[bot]]

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

[jnowakow]

Hello, I'm from SoftwareMansion, an expert agency, and I'm going to work on this one

[jnowakow]

It's strange, I'm able to reproduce this issue on staging but when I try to do so on local build it works correctly:

https://github.com/Expensify/App/assets/56261019/5b08bb42-48b4-48f7-b956-3845af6df18d

[getusha]

Same, not able to reproduce this on local build.

[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? 💸

[melvin-bot[bot]]

@sonialiap, @getusha Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

@sonialiap @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@sonialiap, @getusha 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[sonialiap]

@jnowakow @getusha I'm not sure how local build is created (not an engineer 😅), is the local build based on staging or is it possible for staging to be different from a local build? Maybe the builds have to be renewed?

[melvin-bot[bot]]

@sonialiap, @getusha 10 days overdue. Is anyone even seeing these? Hello?

[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? 💸

[melvin-bot[bot]]

@sonialiap, @getusha 12 days overdue now... This issue's end is nigh!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot[bot]]

This issue has not been updated in over 14 days. @sonialiap, @getusha eroding to Weekly issue.

[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? 💸

[melvin-bot[bot]]

@sonialiap @getusha this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[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? 💸

[getusha]

Seems like the issue is not reproducible anymore, i will confirm it tomorrow https://github.com/Expensify/App/issues/44465#issuecomment-2238040878

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[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? 💸

[mvtglobally]

Issue not reproducible during KI retests. (Third week)

[sonialiap]

Closing since it's not reproducible for 3 weeks

[IuliiaHerets]

The issue is still reproducible with the latest build if you first go to the deep link in the browser. Build - v9.0.31-22

https://github.com/user-attachments/assets/a5d9ff11-701a-410e-9a06-2b793e218630

[sonialiap]

looking for proposals

[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? 💸

[wildan-m]

Edited by proposal-police: This proposal was edited at 2023-10-02T08:53:00Z.

Proposal

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

Duplicate concierge chat displayed on LHN when deeplinking.

What is the root cause of that problem?

In navigateToConciergeChat, we are trying to create the concierge chat when it's not found:

https://github.com/Expensify/App/blob/336e4782aec98bb29a06c28e0d630c276d4eddf6/src/libs/actions/Report.ts#L2155-L2159

Deeplinking can cause issues when there is a conflict between the existing concierge chat from the server and a newly created one. This can lead to the focus being moved to the existing chat or, optimistically, the newly created concierge report showing a "not found" page.

There is a chance that chat not loaded even if isLoadingReportData is false, applying onyx updates, especially report take longer than setting isLoadingReportData to false.

There is a gap between isLoadingReportData become false and allReports being actually populated.

The situation can be described this way:

1. isLoadingReportData being set to false after getOnyxDataForOpenOrReconnect
2. Then applyingOnyxUpdate process will asynchronously update data from the 1st request
3. allReports data might be null or partially added, especially if we have huge number of reports
4. Then since isLoadingReportData already false onServerDataReady() will be executed, but 3rd step might not completed yet

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

If concierge chat doesn't exist, we should wait for the report to load instead of recreating it.

We can either add a new NVP_CONCIERGE_CHAT_REPORT_ID or utilize NVP_ONBOARDING's chatReportID (uncertain if NVP_ONBOARDING consistently represents concierge report id).

In this check code:

https://github.com/Expensify/App/blob/336e4782aec98bb29a06c28e0d630c276d4eddf6/src/libs/actions/Report.ts#L1390-L1394

We can fill NVP_CONCIERGE_CHAT_REPORT_ID value with report.reportID

    if (report?.reportID) {
        if (ReportUtils.isConciergeChatReport(report)) {
            conciergeChatReportID = report.reportID;
            Onyx.set(ONYXKEYS.NVP_CONCIERGE_CHAT_REPORT_ID, report.reportID);
        }
    }

Reset here:

https://github.com/Expensify/App/blob/336e4782aec98bb29a06c28e0d630c276d4eddf6/src/libs/actions/Report.ts#L168-L172

        // When signed out, val is undefined
        if (!value?.accountID) {
            conciergeChatReportID = undefined;
            Onyx.set(ONYXKEYS.NVP_CONCIERGE_CHAT_REPORT_ID, undefined);
            return;
        }

In src/libs/actions/Welcome/index.ts Add new if condition to checkServerDataReady

function checkServerDataReady() {
    if (isLoadingReportData || isEmpty(conciergeChatReportID)) {
        return;
    }

    resolveIsReadyPromise?.();
}

And listen to the key

Onyx.connect({
    key: ONYXKEYS.NVP_CONCIERGE_CHAT_REPORT_ID,
    initWithStoredValues: false,
    callback: (value) => {
        conciergeChatReportID = value ?? '';
        checkServerDataReady();
    },
});

We navigate once the conciergeChatReportID is ready, without having to recreate the chat.

Change this code to

function navigate(reportID: string, shouldDismissModal: boolean, actionType?: string) {
    if (shouldDismissModal) {
        Navigation.dismissModal(reportID);
    } else {
        Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID), actionType);
    }
}

function navigateToConciergeChat(shouldDismissModal = false, checkIfCurrentPageActive = () => true, actionType?: string) {
    // If conciergeChatReportID contains a concierge report ID, we navigate to the concierge chat using the stored report ID.
    // Otherwise, we would find the concierge chat and navigate to it.
    if (!conciergeChatReportID) {
        // In order to avoid creating concierge repeatedly,
        // we need to ensure that the server data has been successfully pulled
        Welcome.onServerDataReady().then(() => {
            // If we don't have a chat with Concierge then create it
            if (!checkIfCurrentPageActive()) {
                return;
            }

            if(!conciergeChatReportID){
                return;
            }

            navigate(conciergeChatReportID, shouldDismissModal, actionType);
        });
    } else {
        navigate(conciergeChatReportID, shouldDismissModal, actionType);
    }

Branch for this solution

What alternative solutions did you explore? (Optional)

Alternative 1 -- add data delay

Add small delay after data ready here:

https://github.com/Expensify/App/blob/336e4782aec98bb29a06c28e0d630c276d4eddf6/src/libs/actions/Report.ts#L2154-L2160

This ensure that the concierge chat loaded, there is a chance that chat not loaded even if isLoadingReportData is false, applying onyx updates, especially report take longer than setting isLoadingReportData to false.

Alternative 2 -- use main solution with concierge chat report ID provided from BE

It would be more efficient if the backend provided the concierge chat report ID when calling the WRITE_COMMANDS.OPEN_APP endpoint, eliminating the need to loop through the reports.

Alternative 3 -- Wait for ReportConnection.getAllReports() not undefined

We can wait for ReportConnection.getAllReports() not undefined like what we did with PriorityMode

https://github.com/Expensify/App/blob/b027baaf086c4a8f57bec48dd1cbbfb3c317726b/src/libs/actions/PriorityMode.ts#L79-L85

In checkServerDataReady add this check ReportConnection.getAllReports() === undefined

src/libs/actions/Welcome/index.ts

import * as ReportConnection from '@libs/ReportConnection';

function checkServerDataReady() {
    if (ReportConnection.getAllReports() === undefined || isLoadingReportData) {
        return;
    }

    resolveIsReadyPromise?.();
}

Or combine with

function checkServerDataReady() {
    if (ReportConnection.getAllReports() === undefined || isLoadingReportData  || isEmpty(conciergeChatReportID)) {
        return;
    }

    resolveIsReadyPromise?.();
}

[wildan-m]

Proposal Updated

• Add optimization suggestion as alternative 2

[getusha]

I am not able to reproduce this

https://github.com/user-attachments/assets/5127c82a-38d2-4b80-a118-283f46b5c2ba

[wildan-m]

@getusha it's reproducible in desktop now,

no need for new user, you can use existing user, just access

https://staging.new.expensify.com/concierge (not direct concierge chat with id)

and then open link in desktop

Like this rep steps:

https://github.com/Expensify/App/issues/44465#issuecomment-2341834174

[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? 💸

[melvin-bot[bot]]

@sonialiap, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@sonialiap, @getusha Still overdue 6 days?! Let's take care of this!

[sonialiap]

I can also still reproduce

When I open the link and log in, I am shown two Concierge messages in the LHN. If I click out of the one that's displayed first then it vanishes (I'm not in #focus mode) and the only way to get back to it is to click the browser back button.

[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? 💸

[melvin-bot[bot]]

@sonialiap, @getusha Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot[bot]]

@sonialiap, @getusha 10 days overdue. I'm getting more depressed than Marvin.

[ugogiordano]

Edited by proposal-police: This proposal was edited at 2024-09-28 10:38:21 UTC.

Proposal

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

Duplicate concierge chat displayed on LHN when deeplinking to /concierge path.

What is the root cause of that problem?

When users log out and deep link to the concierge page, logging back in should open the concierge chat. However, if the reports are not fully loaded from the server, the navigateToConciergeChat function creates a new report instead of navigating to the existing concierge chat.

https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/pages/ConciergePage.tsx#L37-L50

The root cause is that the conciergeChatReportID is not populated when navigateToConciergeChat is called. This occurs because the function checks the conciergeChatReportID to determine whether to navigate to an existing report or create a new one. If the reports have not been loaded yet, the getChatByParticipants function defaults to using an empty reports parameter from ReportConnection.getAllReports(), which leads to it returning an empty object. Consequently, the navigateToAndOpenReport function incorrectly attempts to create a new concierge chat.

Here's the relevant source code for clarity: https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/actions/Report.ts#L2155-L2176

The key issue lies within the navigateToAndOpenReport function, specifically in this block of code: https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/actions/Report.ts#L1005-L1012

The getChatByParticipants function uses ReportConnection.getAllReports() as its default parameter: https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/ReportUtils.ts#L6137 When the data isn’t ready, ReportConnection.getAllReports() returns an empty report collection. As a result, this leads to the creation of a new concierge chat instead of navigating to the existing one.

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

We should implement a check to ensure that the reports are fully loaded before calling navigateToConciergeChat. If the reports are not yet available, we can delay the navigation or provide feedback to the user. This would involve modifying the function to wait for the reports to load before proceeding with navigation logic.

We can define a variable to check if the reports are still loading, using something like:

const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA, {initialValue: true});

Before navigating to the concierge chat, we should ensure that isLoadingReportData is defined and false. If it's still loading or undefined, we should not proceed with the navigation.

The revised source code for ConciergePage would look like this:

function ConciergePage({session}: ConciergePageProps) {
    const styles = useThemeStyles();
    const isUnmounted = useRef(false);
    const {shouldUseNarrowLayout} = useResponsiveLayout();
    const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA, {initialValue: true});

    useFocusEffect(
        useCallback(() => {
            if (session && 'authToken' in session) {
                App.confirmReadyToOpenApp();
                Navigation.isNavigationReady().then(() => {
                    if (isUnmounted.current || isLoadingReportData === undefined || !!isLoadingReportData) {
                        return;
                    }
                    Report.navigateToConciergeChat(true, () => !isUnmounted.current);
                });
            } else {
                Navigation.navigate();
            }
        }, [session, isLoadingReportData]),
    );

    useEffect(() => {
        isUnmounted.current = false;
        return () => {
            isUnmounted.current = true;
        };
    }, []);

    return (
        <ScreenWrapper testID={ConciergePage.displayName}>
            <View style={[styles.borderBottom, styles.appContentHeader, !shouldUseNarrowLayout && styles.headerBarDesktopHeight]}>
                <ReportHeaderSkeletonView onBackButtonPress={Navigation.goBack} />
            </View>
            <ReportActionsSkeletonView />
        </ScreenWrapper>
    );
}

By wrapping the callback in useCallback, we can avoid unnecessary re-renders and ensure the effect only runs when necessary.

What alternative solutions did you explore? (Optional)

I considered a workaround where we immediately navigate to the chat but add logic to prevent creating a new report if the ID isn’t available. However, this could lead to inconsistencies, and ensuring the reports are fully loaded first is a more robust solution.

Another valid option could involve checking the length of the reports collection and only navigating when it's not empty:

    const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {initialValue: {}});

    useFocusEffect(
        useCallback(() => {
            if (session && 'authToken' in session) {
                App.confirmReadyToOpenApp();
                // Pop the concierge loading page before opening the concierge report.
                Navigation.isNavigationReady().then(() => {
                    if (isUnmounted.current || Object.keys(reports ?? {}).length === 0) {
                        return;
                    }
                    Report.navigateToConciergeChat(true, () => !isUnmounted.current);
                });
            } else {
                Navigation.navigate();
            }
        }, [session, reports]),
    );

This ensures we only attempt to access the chat when there are available reports.

[melvin-bot[bot]]

📣 @ugogiordano! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[ugogiordano]

Contributor details Your Expensify account email: ugo.giordano1987@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0113b790fc6c7a1ad9

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[sonialiap]

@getusha what do you think of the above proposal?

[getusha]

Reviewing, i'll update today

[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? 💸

[getusha]

@ugogiordano seems like we already fixed/attempted to fix this as i can see

https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/actions/Report.ts#L2162-L2164

navigateToAndOpenReport is called on onServerDataReady. I am still trying to understand why isLoadingReportData remains true after onServerDataReady.

[ugogiordano]

@getusha I understand your point, but I believe we should first check that all reports have been loaded.

The conciergeChatReportID is assigned within the handleReportChanged function: https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/actions/Report.ts#L1400-L1404

which is only called after all reports are loaded: https://github.com/Expensify/App/blob/ee772a9e599c1adfee46047ba27e961574617e62/src/libs/ReportConnection.ts#L13-L35

Therefore, with my proposal we can also remove the Welcome.onServerDataReady() call, ensuring that we avoid performing unnecessary operations since we can be certain that all reports are ready at that stage.