Error when create new chat in offline mode

[kavimuru]

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

---

Action Performed:

1. Go offline
2. Click FAB -> New chat
3. Search email -> Select user -> Send some message
4. Click FAB -> New chat
5. Select same previous user (Chat doesn’t show previous message)
6. Send some message again
7. Go online

Expected Result:

There should be no error and It should open same chat if selected for second time

Actual Result:

It opens new chat every-time, and show error when go online

Workaround:

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

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.44-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Image.from.iOS.MP4

MOTT8983.2.MP4

Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690057959260379

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[ShogunFire]

Proposal

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

When we create a chat multiple times with the same user offline, it creates multiple times the chat instead of opening the existing one

What is the root cause of that problem?

In Report, when we check if an existing chat exist we call getChatByParticipantsByLoginList here:

App/src/libs/actions/Report.js

Line 532 in cd851d0

const chat = ReportUtils.getChatByParticipantsByLoginList(formattedUserLogins);

In this method we check if the report has a field particpantsAccountIDs that is not empty, we also check if the report has the same participants than the participants of the chat we want to create here:

App/src/libs/ReportUtils.js

Lines 2216 to 2221 in cd851d0

	if (!report || _.isEmpty(report.participantAccountIDs) || isThread(report)) {
	return false;
	}
	// Only return the room if it has all the participants and is not a policy room
	return !isUserCreatedPolicyRoom(report) && _.isEqual(participantsLoginList, _.sortBy(report.participants));

Those two conditions are not met because when we create a chat offline the method here:

App/src/libs/actions/Report.js

Lines 534 to 535 in cd851d0

	const participantAccountIDs = PersonalDetailsUtils.getAccountIDsByLogins(userLogins);
	newChat = ReportUtils.buildOptimisticChatReport(participantAccountIDs);

We retrieve the list of account ids of the participants by searching the logins in personalDetails. For a new chat this will return an empty list. So the participantsList that we pass to buildOptimisticChatReport will be empty here:

App/src/libs/ReportUtils.js

Line 1728 in cd851d0

function buildOptimisticChatReport(

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

In the buildOptimisticChatReport we need to set the field "participants" that is not set so far, we can pass the logins of the users to the method.

Then in getChatByParticipantsByLoginList here:

App/src/libs/ReportUtils.js

Line 2216 in cd851d0

if (!report || _.isEmpty(report.participantAccountIDs) || isThread(report)) {

instead of checking if the particpantsAccountIDs are empty we can check if the participants field is empty

Result:

2023-07-23.20-47-09.mp4

What alternative solutions did you explore? (Optional)

[dukenv0307]

Proposal

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

Error when create new chat in offline mode

What is the root cause of that problem?

When we create a report. Optimistic data is created without participants field.

App/src/libs/ReportUtils.js

Line 1728 in cd851d0

function buildOptimisticChatReport(

It's only returned by BE, so when we search again in offline mode, we cannot find the report by using getChatByParticipantsByLoginList because participants field doesn't exist in optimisticReportData

App/src/libs/actions/Report.js

Line 532 in cd851d0

const chat = ReportUtils.getChatByParticipantsByLoginList(formattedUserLogins);

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

I know we remove all use of participants recently but acctually, we are still using getChatByParticipantsByLoginList to avoid duplicates report. So I think we should keep participants when we create an optimisticData for new report to avoid this issue until getChatByParticipantsByLoginList is deprecated and we have another solution for this.

1. We should add an extra param for buildOptimisticChatReport like participantLoginList with a default value is empty array

App/src/libs/ReportUtils.js

Line 1728 in cd851d0

function buildOptimisticChatReport(

2. Return optimisticData with participants field as participantLoginList param above

App/src/libs/ReportUtils.js

Line 1742 in cd851d0

return {

3. Pass both login list and accountID list to buildOptimisticChatReport here

newChat = ReportUtils.buildOptimisticChatReport(participantAccountIDs, formattedUserLogins);

App/src/libs/actions/Report.js

Lines 534 to 535 in cd851d0

	const participantAccountIDs = PersonalDetailsUtils.getAccountIDsByLogins(userLogins);
	newChat = ReportUtils.buildOptimisticChatReport(participantAccountIDs);

What alternative solutions did you explore? (Optional)

[puneetlath]

I believe this will get fixed by this PR: #22147

[ShogunFire]

@puneetlath If I understand correctly the api will return the first chat created for that user and we delete the next ones right ? What happens to the messages sent offline in the "not first chats"?

[garrettmknight]

@DinalJivani @kavimuru couldn't reproduce on 1.3.45 - specifically, I couldn't get a second chat to open for the same user. When I selected the same user for a second time in the new chat menu it reopened the original chat.

[ShogunFire]

I still reproduce on staging 1.3.45.3

2023-07-25.09-50-28.mp4

Are you offline ?

Are you creating two chats with a user you have never chat before ?

[puneetlath]

@ShogunFire do you still experience the error on this branch? #22147

[ShogunFire]

Offline you will still see the two chats with that PR

Is that a problem ? And that's why I asked that #23439 (comment)

[puneetlath]

Ah got it. We are soon going to be getting rid of participants in favor of participantAccountIDs. Will the problem still exist then?

[ShogunFire]

Yes, we are already not using participants in that case, my proposal was actually to use it lol...

[DinalJivani]

@garrettmknight @puneetlath
I am still able to reproduce this bug in iOS/Native v1.3.57-6.

RPReplay_Final1693340762.mp4