[$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly #46298

lanitochka17 · 2024-07-26T13:21:04Z

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: 9.0.13
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Go to https://staging.new.expensify.com/home
Tap on a room chat -- header -- invite
Enter an email you have not previously had contact with eg: jaihanumanblog+1111@gmail.com
and invite to room
Tap back button
Note members count

Expected Result:

Inviting an user to room via secondary contact must display member count correctly

Actual Result:

Inviting an user to room via secondary contact must display member count correctly

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6552594_1721908859939.uu122.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b5145e9ee0c2aee8
Upwork Job ID: 1818427399700311066
Last Price Increase: 2024-07-30

Issue Owner

Current Issue Owner: @ZhenjaHorbach

melvin-bot · 2024-07-26T13:21:15Z

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

lanitochka17 · 2024-07-26T13:21:22Z

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

lanitochka17 · 2024-07-26T13:21:36Z

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

FitseTLT · 2024-07-26T13:57:56Z

Proposal

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

room-Inviting a user who you have no contact before displays member count incorrectly

What is the root cause of that problem?

The issue is not caused by using the secondary contact to add a member to a room (as indicated in OP) but instead when a user adds a member who's personal detail doesn't exist in onyx a new optimistic personal detail is created then after a while the real personal detail representing the member comes from BE and the optimistic personal detail will be deleted but the accountID of the optimistic personal detail still exists in report.participants list so the members count will be wrong as we display the participants length here

App/src/pages/ReportDetailsPage.tsx

Lines 305 to 306 in b0f810d

 subtitle: activeChatMembers.length, 

 isAnonymousAction: false,

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

We should filter particpants whose personal details doesn't exist just the same as what we did in RoomMemebersPage here

App/src/pages/RoomMembersPage.tsx

Lines 174 to 180 in b0f810d

 participants.forEach((accountID) => { 

 const details = personalDetails[accountID]; 

 if (!details) { 

 Log.hmmm(`[RoomMembersPage] no personal details found for room member with accountID: ${accountID}`); 

 return; 

 }

We should change

App/src/pages/ReportDetailsPage.tsx

Lines 138 to 141 in b0f810d

 const participants = useMemo(() => { 

 const shouldExcludeHiddenParticipants = !isGroupChat && !isSystemChat; 

 return ReportUtils.getParticipantsAccountIDsForDisplay(report, shouldExcludeHiddenParticipants); 

 }, [report, isGroupChat, isSystemChat]);

to

const participants = useMemo(() => {
        const shouldExcludeHiddenParticipants = !isGroupChat && !isSystemChat;
        const participantAccountIDs = ReportUtils.getParticipantsAccountIDsForDisplay(report, shouldExcludeHiddenParticipants);
        return participantAccountIDs?.filter((accountID) => !!personalDetails?.[accountID]);
    }, [report, isGroupChat, isSystemChat, personalDetails]);

We can also check and do the same for workspace members too if needed.

What alternative solutions did you explore? (Optional)

FitseTLT · 2024-07-26T14:04:49Z

@stephanieelliott the title of the OP is a lit bit misleading as it suggests the problem is caused by using the secondary contact to invite the member but the problem is caused by the member being invited was a member with which the current user had no contact before. So to reproduce it, you should test by adding a member with which you had no contact before.
And also I suggest you update the title of the issue accordingly Thx. 👍

bernhardoj · 2024-07-27T06:24:27Z

Proposal

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

Inviting user we never chat before to a room will make an extra count to the room member count.

What is the root cause of that problem?

When we are searching for a new user, the app will generate a random optimistic account ID.

App/src/libs/OptionsListUtils.ts

Lines 1748 to 1749 in 5e65276

 // Generates an optimistic account ID for new users not yet saved in Onyx 

 const optimisticAccountID = UserUtils.generateAccountID(searchValue);

When we invite the user, the app will optimistically merge the generated account ID to the room participants.

App/src/libs/actions/Report.ts

Lines 2809 to 2836 in 5e65276

 const participantsAfterInvitation = inviteeAccountIDs.reduce( 

 (reportParticipants: Participants, accountID: number) => { 

 const participant: ReportParticipant = { 

 hidden: false, 

 role: CONST.REPORT.ROLE.MEMBER, 

 }; 

 // eslint-disable-next-line no-param-reassign 

 reportParticipants[accountID] = participant; 

 return reportParticipants; 

 }, 

 {...report.participants}, 

 ); 

 const logins = inviteeEmails.map((memberLogin) => PhoneNumber.addSMSDomainIfPhoneNumber(memberLogin)); 

 const {newAccountIDs, newLogins} = PersonalDetailsUtils.getNewAccountIDsAndLogins(logins, inviteeAccountIDs); 

 const newPersonalDetailsOnyxData = PersonalDetailsUtils.getPersonalDetailsOnyxDataForOptimisticUsers(newLogins, newAccountIDs); 

 const pendingChatMembers = ReportUtils.getPendingChatMembers(inviteeAccountIDs, report?.pendingChatMembers ?? [], CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); 

 const optimisticData: OnyxUpdate[] = [ 

 { 

 onyxMethod: Onyx.METHOD.MERGE, 

 key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 

 value: { 

 participants: participantsAfterInvitation, 

 pendingChatMembers, 

 }, 

 }, 

 ];

However, we never clear the optimistic report participant. This logic is used for both invite to room and group, but the issue doesn't happen in a group chat because the API for invite to group chat returns a response when success that will clears the optimistic report participant.

The reason the optimistic participant disappears from the member list is because we skip over members that doesn't have personal details (which are cleared correctly when successful).

App/src/pages/RoomMembersPage.tsx

Lines 177 to 180 in 5e65276

 if (!details) { 

 Log.hmmm(`[RoomMembersPage] no personal details found for room member with accountID: ${accountID}`); 

 return; 

 }

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

Fix it in the BE so invite to room also returns a response to clear the optimistic report participant.

OR

Fix it in the FE by clearing the optimistic report participant in success data.

const participantsSuccessData = inviteeAccountIDs.reduce(
    (reportParticipants, accountID: number) => ({...reportParticipants, [accountID]: null}),
    {},
);
const successData: OnyxUpdate[] = [
    {
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
        value: {
            participants: participantsSuccessData,
            pendingChatMembers: successPendingChatMembers,
        },
    },
];

melvin-bot · 2024-07-29T18:50:12Z

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

stephanieelliott · 2024-07-30T23:22:40Z

Thanks @FitseTLT! I see what you mean -- updated the title and action steps to make that more clear.

stephanieelliott · 2024-07-30T23:24:05Z

Given that it affects any user you've not previously had contact with (not secondary logins) and persists on the workspace, I think this is worth fixing. Adding labels to get the proposals reviewed.

melvin-bot · 2024-07-30T23:24:13Z

Job added to Upwork: https://www.upwork.com/jobs/~01b5145e9ee0c2aee8

melvin-bot · 2024-07-30T23:24:18Z

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

ZhenjaHorbach · 2024-07-31T08:11:57Z

I will check proposals today or tomorrow

ZhenjaHorbach · 2024-08-01T11:41:12Z

@FitseTLT @bernhardoj
Thank you for your proposals !

I agree with @bernhardoj that we can fix this in BE

But on the other side If we decide to fix this in FE
I like @FitseTLT proposal more
Firstly, because it works as expected (@bernhardoj proposal works a little unpredictably if we try to invite a known user (This user will disappear from the list))
And secondly, we will be sure that these changes will work with the users already invited

But first, let's find out what the internal team thinks about where it is best to fix it
🎀👀🎀 C+ reviewed

melvin-bot · 2024-08-01T11:41:25Z

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

dangrous · 2024-08-02T20:03:52Z

Okay yep, I think it makes the most sense to copy the group chat behavior on the backend. I should be able to get to that early next week!

To confirm, if we do that, we shouldn't need any front end fixes, right?

ZhenjaHorbach · 2024-08-05T09:52:13Z

Okay yep, I think it makes the most sense to copy the group chat behavior on the backend. I should be able to get to that early next week!

To confirm, if we do that, we shouldn't need any front end fixes, right?

Since inviting to a group chat works correctly
I think the changes in BE will be enough

dangrous · 2024-08-05T17:24:36Z

This did need an incredibly minor front end adjustment, so I went ahead and did that myself. Both PRs will be in review in a sec!

stephanieelliott · 2024-08-08T03:00:54Z

The second PR is held on #46825 which is merged but not yet on prod , so we should be able to resume this soon.

dangrous · 2024-08-09T15:28:45Z

still waiting on deploy to prod, will likely be Monday due to earnings, Friday...

dangrous · 2024-08-13T13:58:37Z

we should be able to test this again on staging and hopefully close out!

dangrous · 2024-08-14T15:15:34Z

So this ends up with the correct amount, but it takes a while to get there. It doesn't get higher than it should but if I'm the only person in a room and then I invite another person, it stays 1 for a while, before switching to 2. @ZhenjaHorbach (or others) do you want to give this a look and see if there's anything we can do optimistically on the front end that might make this cleaner?

Or if I need to update the backend again we can do that.

ZhenjaHorbach · 2024-08-14T15:58:16Z

So this ends up with the correct amount, but it takes a while to get there. It doesn't get higher than it should but if I'm the only person in a room and then I invite another person, it stays 1 for a while, before switching to 2. @ZhenjaHorbach (or others) do you want to give this a look and see if there's anything we can do optimistically on the front end that might make this cleaner?

Or if I need to update the backend again we can do that.

Could you provide a video, please ?
Because everything works great for me now

dangrous · 2024-08-14T18:27:24Z

Oh weird! I just tested again (in order to take the video) and it works like a charm. Maybe something was happening on the backend.

Are we good to close, or do we need any payments handled?

ZhenjaHorbach · 2024-08-14T18:54:27Z

Oh weird! I just tested again (in order to take the video) and it works like a charm. Maybe something was happening on the backend.

Are we good to close, or do we need any payments handled?

Following the new rules and if i understand correctly
I am eligible for 50% of the payment 😀

But if not, then I think we can close this issue !

dangrous · 2024-08-14T20:27:54Z

Ah yep that seems right to me! cc @stephanieelliott

stephanieelliott · 2024-08-16T01:18:38Z

Hey @ZhenjaHorbach I extended the offer to you in Upwork, can you accept when you get a sec? https://www.upwork.com/nx/wm/offer/103546715

ZhenjaHorbach · 2024-08-16T04:12:49Z

Hey @ZhenjaHorbach I extended the offer to you in Upwork, can you accept when you get a sec? https://www.upwork.com/nx/wm/offer/103546715

Done, thanks !

stephanieelliott · 2024-08-17T01:44:14Z

Summarizing payment on this issue:

Contributor: N/A internal
Contributor+: @ZhenjaHorbach $250 via Upwork -- PAID

Upwork job is here: https://www.upwork.com/jobs/~01b5145e9ee0c2aee8

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 26, 2024

melvin-bot bot assigned stephanieelliott Jul 26, 2024

melvin-bot bot added the Overdue label Jul 29, 2024

stephanieelliott changed the title ~~mWeb - room-Inviting an user to room via secondary contact displays member count incorrectly~~ mWeb - room- Inviting a previously unknown user to room displays member count incorrectly Jul 30, 2024

melvin-bot bot removed the Overdue label Jul 30, 2024

stephanieelliott added the External Added to denote the issue can be worked on by a contributor label Jul 30, 2024

melvin-bot bot changed the title ~~mWeb - room- Inviting a previously unknown user to room displays member count incorrectly~~ [$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly Jul 30, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 30, 2024

melvin-bot bot assigned ZhenjaHorbach Jul 30, 2024

melvin-bot bot assigned dangrous Aug 1, 2024

dangrous added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Aug 2, 2024

dangrous mentioned this issue Aug 5, 2024

Send optimistic IDs to InviteToRoom so they can be removed #46825

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 5, 2024

stephanieelliott closed this as completed Aug 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly #46298

[$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly #46298

lanitochka17 commented Jul 26, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jul 26, 2024

lanitochka17 commented Jul 26, 2024

lanitochka17 commented Jul 26, 2024

FitseTLT commented Jul 26, 2024

FitseTLT commented Jul 26, 2024

bernhardoj commented Jul 27, 2024 •

edited

Loading

melvin-bot bot commented Jul 29, 2024

stephanieelliott commented Jul 30, 2024

stephanieelliott commented Jul 30, 2024

melvin-bot bot commented Jul 30, 2024

melvin-bot bot commented Jul 30, 2024

ZhenjaHorbach commented Jul 31, 2024

ZhenjaHorbach commented Aug 1, 2024 •

edited

Loading

melvin-bot bot commented Aug 1, 2024

dangrous commented Aug 2, 2024

ZhenjaHorbach commented Aug 5, 2024

dangrous commented Aug 5, 2024

stephanieelliott commented Aug 8, 2024

dangrous commented Aug 9, 2024

dangrous commented Aug 13, 2024

dangrous commented Aug 14, 2024

ZhenjaHorbach commented Aug 14, 2024 •

edited

Loading

dangrous commented Aug 14, 2024

ZhenjaHorbach commented Aug 14, 2024 •

edited

Loading

dangrous commented Aug 14, 2024

stephanieelliott commented Aug 16, 2024

ZhenjaHorbach commented Aug 16, 2024

stephanieelliott commented Aug 17, 2024

[$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly #46298

[$250] mWeb - room- Inviting a previously unknown user to room displays member count incorrectly #46298

Comments

lanitochka17 commented Jul 26, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jul 26, 2024

lanitochka17 commented Jul 26, 2024

lanitochka17 commented Jul 26, 2024

FitseTLT commented Jul 26, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

FitseTLT commented Jul 26, 2024

bernhardoj commented Jul 27, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

melvin-bot bot commented Jul 29, 2024

stephanieelliott commented Jul 30, 2024

stephanieelliott commented Jul 30, 2024

melvin-bot bot commented Jul 30, 2024

melvin-bot bot commented Jul 30, 2024

ZhenjaHorbach commented Jul 31, 2024

ZhenjaHorbach commented Aug 1, 2024 • edited Loading

melvin-bot bot commented Aug 1, 2024

dangrous commented Aug 2, 2024

ZhenjaHorbach commented Aug 5, 2024

dangrous commented Aug 5, 2024

stephanieelliott commented Aug 8, 2024

dangrous commented Aug 9, 2024

dangrous commented Aug 13, 2024

dangrous commented Aug 14, 2024

ZhenjaHorbach commented Aug 14, 2024 • edited Loading

dangrous commented Aug 14, 2024

ZhenjaHorbach commented Aug 14, 2024 • edited Loading

dangrous commented Aug 14, 2024

stephanieelliott commented Aug 16, 2024

ZhenjaHorbach commented Aug 16, 2024

stephanieelliott commented Aug 17, 2024

lanitochka17 commented Jul 26, 2024 •

edited by melvin-bot bot

Loading

bernhardoj commented Jul 27, 2024 •

edited

Loading

ZhenjaHorbach commented Aug 1, 2024 •

edited

Loading

ZhenjaHorbach commented Aug 14, 2024 •

edited

Loading

ZhenjaHorbach commented Aug 14, 2024 •

edited

Loading