Chat - Message sent offline change positions between each other once back online

[kbecciv]

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

---

Issue found when executing PR #12626

Action Performed:

1. Go to https://staging.new.expensify.com/
   and login
2. Open any chat
3. Go offline
4. Input several multi-word messages
5. Go back online

Expected Result:

Messages should stay at the same place

Actual Result:

Message sent offline change positions between each other once back online

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

• Web
• Android
• mWeb

Version Number: 1.2.36.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5846933_video_08.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[marcaaron]

Not convinced this is something we need to fix. As long as the app ends up in the state the user expects. cc @roryabraham because you started a similar conversation about this in Slack already (even though I'm pretty sure we have different opinions on this). Maybe we can HOLD this until there is a consensus.

[tienifr]

Proposal

Problem

In

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

Line 254 in 36a009b

getSortedReportActionsForDisplay(reportActions) {

we're re-sorting the reports by created. Unfortunately, the created in optimistic data is different from that one on BE side => when user backs online the created changes the reports order.

Solution

I think we should fix both on FE and BE sides.

App/src/libs/actions/Report.js

Lines 432 to 440 in 36a009b

	const parameters = {
	reportID,
	reportActionID: file ? attachmentAction.reportActionID : reportCommentAction.reportActionID,
	commentReportActionID: file && reportCommentAction ? reportCommentAction.reportActionID : null,
	reportComment: reportCommentText,
	clientID: lastAction.clientID,
	commentClientID: lodashGet(reportCommentAction, 'clientID', ''),
	file,
	};

I'll add created as the parameter and on BE side will use this instead of creating the new one

        file,
+        created: file? attachmentAction.created:reportCommentAction.created
    };

[zanyrenney]

Will wait for @roryabraham input here about the prioritisation of fixing this after @marcaaron comment.

[roryabraham]

I agree we need internal consensus here. The slack conversation is here. I think the only consensus we've got so far is that:

• we can punt this and treat it like a new feature instead of a bug. So mark as New Feature and put on HOLD for WAQ for now.
• if we do decide that we should fix this, we can use a batching solution that sends all optimistically created reportActions in a single request (probably with ReconnectApp)
• This needs to be an internal issue

Thanks for the proposal @tienifr, but we want to treat the database created time as the canonical one, because that more closely relates to the actual order in which messages were sent.

[zanyrenney]

This is on hold, not overdue. @roryabraham as you've removed the bug label, I assume I should not be assigned to this anymore and when this is picked back up as a new feature, the engineer working on it will self-assign. please lmk!

[zanyrenney]

https://expensify.slack.com/archives/C01SKUP7QR0/p1671467869541089

[zanyrenney]

clearing my assignment out for this posted my thoughts in slack as to why!

[melvin-bot]

This issue has not been updated in over 15 days. eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[roryabraham]

This no longer needs to be on HOLD, but we still need to further discuss whether this is a new feature worth implementing.

[neil-marcellini]

@roryabraham you fixed this recently correct? Would you please link the PR and close the issue?

[roryabraham]

#15942

Closing this out as a dupe of #13250