[$250] mWeb - Workspace - Wrong members number is displayed

m-natarajan commented 2 weeks ago

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: 1.4.74-1 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4563078 Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

Action Performed:

Create a workspace
Invite one member
Navigate to the workspace chat with the invited member.
Click on the header to go to the details page
Notice that the members number is displayed as 4.
Click on members, Notice that only 3 members are there.

Expected Result:

members number displayed is equal to the workspace members

Actual Result:

members number is displayed as 4 when there are only 3 members.

Workaround:

unknown

Platforms:

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

[ ] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/37d0c804-ead0-4433-a111-acce390aa756

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @alitoshmatov

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0110eb98a4dc986510
Upwork Job ID: 1792305587832528896
Last Price Increase: 2024-05-26

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @alexpensify (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.

m-natarajan commented 2 weeks ago

@alexpensify 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

m-natarajan commented 2 weeks ago

We think that this bug might be related to #vip-vsb

nkdengineer commented 2 weeks ago

Proposal

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

members number is displayed as 4 when there are only 3 members.

What is the root cause of that problem?

When calling API AddMembersToWorkspace to add the account which did not signed in yet, we create an optimistic accountID for this account, then add that accountID to the workspaceMemberChats.participants.
Then when the API is called successfully, BE returns another accountID that also points to the invited user and it is added to workspaceMemberChats.participants.
Now, workspaceMemberChats.participants contains two accountID that point to one user.
The number of members displayed in ReportDetailsPage is from activeChatMembers, and as we can see, the wrong members number is displayed. And the participants to display in ReportParticipantsPage are from here, where we have a logic: https://github.com/Expensify/App/blob/e8ae3c5acedf0e6788dc574c7b6f3043ca37092a/src/pages/ReportParticipantsPage.tsx#L76-L80 which will filter out the accountID that does not have personalDetails?.[accountID].

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

In activeChatMembers, we should only get the accountID that has personalDetails[accountID] like we did when getting the participants to display in here

What alternative solutions did you explore? (Optional)

We can remove the optimistic accountID from the report's participant once the API is called successfully.

To do it, in here:

            participants: {
                [accountID]: allPersonalDetails[accountID] ? {} : null,
            },

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @adelekennedy (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.

melvin-bot[bot] commented 2 weeks ago

Current assignee @adelekennedy is eligible for the Bug assigner, not assigning anyone new.

alexpensify commented 2 weeks ago

Heads up, I will be offline until Tuesday, May 28, 2024, and will not actively watch over this GitHub during that period.

@adelekennedy - I've run out of time and have been unable to complete the checks here. Since I won't get to it in a timely manner, I'm reassigning for help. Thanks!

melvin-bot[bot] commented 1 week ago

Job added to Upwork: https://www.upwork.com/jobs/~0110eb98a4dc986510

melvin-bot[bot] commented 1 week ago

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

adelekennedy commented 1 week ago

moving this to external and adding to vip-vsb, may juts be a polish but I think we should address it @alitoshmatov one proposal above to review

alitoshmatov commented 1 week ago

@nkdengineer Can you pinpoint me where exactly this is happening, I am having a hard time understanding this one:

When calling API AddMembersToWorkspace to add the account which did not signed in yet, we create an optimistic accountID for this account, then add that accountID to the workspaceMemberChats.participants.

nkdengineer commented 1 week ago

Can you pinpoint me where exactly this is happening

When searching for the account that is not signed in yet, we generate a random one in here
Then, when calling API AddMembersToWorkspace, we add that random accountID to workspaceMemberChats.participants.

nkdengineer commented 1 week ago

we create an optimistic accountID for this account

You can see it by:

Go offline then open any workspace invite member page.
Invite the user that you do not have contact yet.
In workspace member page, click on the user invited in step 2 and see the accountID in URL
Go online. In workspace member page, click on the user invited in step 2 and see the accountID in URL

melvin-bot[bot] commented 6 days ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

alitoshmatov commented 6 days ago

@nkdengineer I think we are not addressing the real root cause, this issue is only happening right after inviting new member but if we are to refresh the page or come back later the issue is no longer present, meaning that there is already a mechanism to remove temporary account id, we should focus on why it is not immediately removing this id after user data is fetched preventing two ids for the same user

nkdengineer commented 5 days ago

@alitoshmatov When you refreshing the page, the API OpenReport is called, then the correct report.participants is returned, that leads to the issue is no longer present.

alexpensify commented 4 days ago

Thanks, @adelekennedy, for the help here. I'm back online and taking over again as the BZ member.

melvin-bot[bot] commented 2 days ago

@alexpensify @alitoshmatov 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] commented 2 days ago

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

nkdengineer commented 2 days ago

@alitoshmatov Do you have any feedback about my comment above?

nkdengineer commented 1 day ago

I updated alternative solution

Expensify / App