[$1000] Group Details - The order of the group members changes

[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 #15509

Action Performed:

1. Go to URL https://staging.new.expensify.com/
2. Sign in with any account
3. Click on the FAB button -> New group -> Create a group chat with a combination of members with phone numbers and email/name logins
4. Note the order of group members shown in the chat list page.
5. Open the group chat
6. Open the member's list page of the group
7. Click on a member with a phone number login and choose the message option in the user detail page
8. Now the order at step 2 should be maintained

Expected Result:

The order of the group members NOT changes

Actual Result:

The order of the group members changes

Workaround:

Unknown

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.2.78.0

Reproducible in staging?: Yes

Reproducible in production?: No

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

Bug5962480_15509_Web.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019bb05bb5e04ee628
• Upwork Job ID: 1633448413985218560
• Last Price Increase: 2023-03-08

[MelvinBot]

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

[MelvinBot]

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

[luacmartins]

This might be a dupe of #14886

[MelvinBot]

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

[MelvinBot]

@miljakljajic Whoops! This issue is 2 days overdue. Let's get this updated quick!

[miljakljajic]

Dupe - going to close out.

[luacmartins]

Ok, seems like this one is not a dupe. Gonna reopen it and make it external

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~019bb05bb5e04ee628

[MelvinBot]

Current assignee @miljakljajic is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

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

[MelvinBot]

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

[abdulrahuman5196]

Proposal

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

To reproduce the bug we don't need to do all the steps mentioned in the bug.

1. Just create a new group chat with phone numbers and names
2. Move to another chat and come back to group chat.
3. The order at step 1 will be changed

What is the root cause of that problem?

If you notice closely on the video when you create the chat itself the order of the group member was in the order of numbers followed by names. Let me show the code flow and example.

1. User selects list of users in selector page ([+122, a@test.com, +133, b@test.com]) - https://github.com/Expensify/App/blob/main/src/pages/NewChatPage.js#L225
2. It reaches the below code where the participants list is sorted with .sort() (Not sure if it was intentional or not). End order - [+122, +133, a@test.com, b@test.com]
   

   App/src/libs/ReportUtils.js

   Lines 1444 to 1446 in a7c196f

   	function getChatByParticipants(newParticipantList) {
   	newParticipantList.sort();
   	return _.find(allReports, (report) => {

3. Now we pass the above order to openReport API as participants list and same is passed to optimistic report as well -

   App/src/libs/actions/Report.js

   Line 364 in a7c196f

   emailList: participantList ? participantList.join(',') : '',

4. It seems if we pass participantsList as emailList param to the OpenReport API for a new group chat creation, it returns the same order in the API response. And the same order is observed for the first time in pusher event.
5. After that when we re-open the group chat we call openReport API to re-fetch new information. Here we don't pass the participantsList and OpenReport API returns the participants order correctly [a@test.com, b@test.com, +122, +133]

Thus, user can see one order until step 4 and another order in step 5 and the order at step 5 is consistently maintained always.

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

1. We should create a sort function which provides result similar to backend sort. I think the backend sort keeps names first and numbers at later part. We can use the new sort function to populate optimistic data on report creation and to OpenReport API for create group chat as well. So the order will be maintained during new group chat creation and openReport API. This way even in offline chat creation it will work the same way.

Additional fixes:

1. Similar to [HOLD for payment 2023-03-13] [$1000] Group member orders keeps on changing randomly when it has more then 1 user with phone number as primaryLogin #14886 bug we should remove inplace .sort() and use _.sortBy() so that it won't affect objects outside of the function, at

   App/src/libs/ReportUtils.js

   Lines 1444 to 1446 in a7c196f

   	function getChatByParticipants(newParticipantList) {
   	newParticipantList.sort();
   	return _.find(allReports, (report) => {

Optional:

1. Backend should provide sorted order for new chat creation in OpenReport API as well.
2. We can also add lint function to avoid .sort() to avoid issues in future

What alternative solutions did you explore? (Optional)

We can make the backend return participants in the same order of mobile _.sortBy() function. But I am not sure if we want to show numbers first followed by names.

We can also just create a sort similar to backend and use it to display the members in order.

[MelvinBot]

@robertjchen, @miljakljajic Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MelvinBot]

@robertjchen, @miljakljajic Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

@robertjchen, @miljakljajic 10 days overdue. I'm getting more depressed than Marvin.

[robertjchen]

I've started looking into the backend changes. Once those are complete, we'll re-visit the frontend behavior and see if there's anything to be done there: #15644 (comment)

[MelvinBot]

@robertjchen, @miljakljajic Eep! 4 days overdue now. Issues have feelings too...

[robertjchen]

Still deciphering the logic in the backend- hang tight!

[robertjchen]

adjusting priority now that we're handling internally

[michaelhaxhiu]

Heyo - we should keep all e/app GHs as daily unless they are on hold. I just flipped the label back. I am operating on this SO post as part of the weekly resync.

[robertjchen]

Ah, I think I've figured it out- draft PR linked. Testing things out locally, will hopefully have this closed out by EOW.

[robertjchen]

Changes under review

[robertjchen]

Changes were merged, on the way out!

[robertjchen]

Changes went to prod last week.