HIGH: [UX Reliability] [$500] When opening a new DM to an existing user, draft message is lost

[quinthar]

Problem:

When you open a DM for the first time to an existing user, we create an offline/optimistic DM -- and then swap the real one in. In the process, however, whatever draft you were optimistically writing to the user gets blown away. To reproduce:

1. Alice and Bob are existing users
2. Alice and Bob do ~not~ have an existing DM, ~however~ but have not messaged yet since Alice opened the client
3. Alice opens a DM to Bob -- an optimistic DM is created as a placeholder
4. Alice begins typing a message in the optimistic DM, but doesn't send it
5. The server responds with the true DM to the real user
6. The draft message is lost

Solution:

The draft message should be retained through the switcharoo to the final correct DM.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0188c8c90dac40b06a
• Upwork Job ID: 1772521262207307776
• Last Price Increase: 2024-04-23

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0188c8c90dac40b06a

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[nkdengineer]

Proposal

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

When opening a new DM to an existing user, draft message is lost

What is the root cause of that problem?

When we switch to the current report, we don't update the draft comment from the optimistic DM to this current DM so the draft message is lost.

https://github.com/Expensify/App/blob/d8361eee3486ccb5472060f2af7655e96461471e/src/libs/actions/Report.ts#L1100

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

1. We can emit an event and pass the preexistingReportID and a callback as data

// Only re-route them if they are still looking at the optimistically created report
let callback = () => {};
if (Navigation.getActiveRoute().includes(`/r/${report.reportID}`)) {
    callback = () => {
        Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.preexistingReportID ?? ''), CONST.NAVIGATION.TYPE.FORCED_UP);
    }
}
DeviceEventEmitter.emit(`switchToCurrentReport_${report.reportID}`, {
    preexistingReportID: report.preexistingReportID,
    callback
});

https://github.com/Expensify/App/blob/d8361eee3486ccb5472060f2af7655e96461471e/src/libs/actions/Report.ts#L1100

2. In Report.ts, create a function like saveReportDraftCommentWithCallack that will save the draft comment and then do the callback or we can add an extra param callback with default value is null in saveReportDraftComment

function saveReportDraftCommentWithCallack(reportID: string, comment: string | null, callback: () => void) {
    Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, prepareDraftComment(comment)).then(callback);
}

3. And then we will listen this in ComposerWithSuggestion and we will save the draft comment to the current report and then do the callback after saving complete. That will make sure the draft is saved completely before we navigating to the current report.

const switchToCurrentReport = DeviceEventEmitter.addListener(`switchToCurrentReport_${reportID}`, ({preexistingReportID, callback}) => {
    if (!commentRef.current) {
        callback();
        return;
    }
    Report.saveReportDraftCommentWithCallack(preexistingReportID, commentRef.current, callback);
});

What alternative solutions did you explore? (Optional)

NA

[Affaq-Ahmed]

To address this issue, you can implement a mechanism to save the draft message locally on the client side when the optimistic DM is created. Then, when the real DM is received from the server, you can check if there's a saved draft message for that user and load it into the real DM if it exists. This way, the draft message won't be lost during the swap process.

[melvin-bot[bot]]

📣 @Affaq-Ahmed! 📣 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>

[OleksandrBashchenko]

Hi, how can I contribute to this task? May I report my solutions at here? Where is your source code that I can build on my local?

[melvin-bot[bot]]

📣 @OleksandrBashchenko! 📣 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>

[gijoe0295]

I can't reproduce it today. Did I miss something?

https://github.com/Expensify/App/assets/153004152/e7ae3245-7bbc-4ae3-985c-e5bed953df16

[Santhosh-Sellavel]

Unable to reproduce this

https://github.com/Expensify/App/assets/85645967/6da75c32-1934-455d-b624-71a980ca4922

[quinthar]

Whoops, the initial reproduction steps were wrong; I've fixed them. Here's an example -- note the draft comment I started writing disappeared.

https://github.com/Expensify/App/assets/575186/97265a31-0507-4a64-88f4-d4fe71554c3c

[quinthar]

@Santhosh-Sellavel can you try with the new steps?

[Santhosh-Sellavel]

Yeah was able to reproduce easily while offline or when the internet speed is low.

https://github.com/Expensify/App/assets/85645967/6a0403f4-3fdb-4f75-9e47-53586ade94ea

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

[quinthar]

Great, what's the next step? Can we get a proposal for this? I get hit by it all the time.

[miljakljajic]

@Santhosh-Sellavel what do you think of the initial proposal that @nkdengineer has shared?

@Affaq-Ahmed @OleksandrBashchenko if you have proposals to share for this issue, or questions, please let us know.

[Santhosh-Sellavel]

@nkdengineer's proposal didn't work for me. So we are waiting for proposals here

[nkdengineer]

@Santhosh-Sellavel I tested and it works well. Actually when we search for new user SearchForReport API return the login of user so we still see this user in start chat. You can try to logout and login again after start chat with new user and continue the test steps.

https://github.com/Expensify/App/assets/161821005/e7e997a5-c61f-43da-be46-a0f2abb92f11

[Santhosh-Sellavel]

@nkdengineer I tested it didn't work for me, can you check & update your proposal if required. Or share diff/branch? Not sure what's missing?

https://github.com/Expensify/App/assets/85645967/0817841e-473d-4948-b88b-7388a9a8f9c4

[nkdengineer]

@Santhosh-Sellavel The test branch here https://github.com/nkdengineer/App/tree/fix/38983. If it still doesn't work, please share the video here so I can see what is the problem thanks.

[Santhosh-Sellavel]

@nkdengineer Tried again still didn't work https://github.com/Expensify/App/assets/85645967/6c88974e-8e52-4361-bd7a-d2192a5887ef

[nkdengineer]

@Santhosh-Sellavel the problem is here because, we save the draft with debounced 1s so if the OpenReport complete quickly the draft of the optimistic report isn't save and then we can't see this after switching to the current report. You can test with offline mode of slow 3G network, you will not see this problem.

I suggest we can decrease the debounce time from 1000 to 500 or smaller. So the time is enough to save the draft before we switch to the current report. What do you think?

https://github.com/Expensify/App/blob/a1f9cecb6c81a6d2728905c67c39d33888c72b82/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx#L335-L340

[Santhosh-Sellavel]

We can't go on an assumption that it would work on 500 ms, and come with a solid approach. We should save the draft before this report change happens, with some hook if possible

[quinthar]

Yes, race conditions are bugs, not solutions. Let's make a real solution.

[nkdengineer]

@Santhosh-Sellavel Thanks for your feedback, updated my proposal https://github.com/Expensify/App/issues/38983#issuecomment-2019570054 with new solution to make sure the draft is saved before switching.

[Santhosh-Sellavel]

Will review shortly

[Santhosh-Sellavel]

@nkdengineer Still not working can you please check and update thanks!

[nkdengineer]

@Santhosh-Sellavel I updated the test branch https://github.com/nkdengineer/App/tree/fix/38983, please help to check 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? 💸

[Santhosh-Sellavel]

@nkdengineer Still not working

https://github.com/Expensify/App/assets/85645967/3f0a097d-336f-442d-a370-c6d5cc15bd82