[HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline #43042

lanitochka17 · 2024-06-04T16:02:10Z

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-79-2
Reproducible in staging?: Y
Reproducible in production?: N
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 staging.new.expensify.com
Go offline
Go to FAB > Start chat
Create a group chat with a few users
Tap on the chat header

Expected Result:

Members will show the correct number of group members

Actual Result:

Members show 0 when there are some group members

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6501741_1717515811787.zero_mem.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @ShridharGoel

melvin-bot · 2024-06-04T16:02:24Z

Triggered auto assignment to @aldo-expensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions · 2024-06-04T16:02:25Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

lanitochka17 · 2024-06-04T16:03:04Z

@aldo-expensify 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

ShridharGoel · 2024-06-04T16:16:43Z

Proposal

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

Group chat - Member amount is 0 when creating group chat offline

What is the root cause of that problem?

There is an extra && isSystemChat in the below condition which causes the condition to be always false when it's a group chat.

App/src/pages/ReportDetailsPage.tsx

Lines 94 to 101 in 1865aa8

    
           const participants = useMemo(() => { 
        
               if (isGroupChat || isSystemChat) { 
        
                   // Filter out the current user from the particpants of the systemChat 
        
                   return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID && isSystemChat); 
        
               } 
        
               return ReportUtils.getVisibleChatMemberAccountIDs(report.reportID ?? ''); 
        
           }, [report, session, isGroupChat, isSystemChat]);

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

Change it to this:

const participants = useMemo(() => {
    if (isSystemChat) {
        // Filter out the current user from the participants of the systemChat
        return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID);
    }

    return ReportUtils.getVisibleChatMemberAccountIDs(report.reportID ?? '');
}, [report, session, isSystemChat]);

We don't need the check for isGroupChat, getVisibleChatMemberAccountIDs should be used.

ShridharGoel · 2024-06-04T16:17:57Z

@aldo-expensify Since this is a blocker, I can open a PR quickly. Can you have a look at the above proposal?

It is caused by #41290.

Nodebrute · 2024-06-04T16:32:47Z

Proposal

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

Member amount is 0 when creating group chat offline

What is the root cause of that problem?

Regression caused by this PR #41290. If it's not a systemChat we are filtering out all the members. The purpose of this change was to filter current user from members list if it's a system chat

App/src/pages/ReportDetailsPage.tsx

Line 97 in 1865aa8

    
           return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID && isSystemChat);

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

This was added to filter out current user from members list. But with this is also removing current user from group's members list too

App/src/pages/ReportDetailsPage.tsx

Line 97 in 1865aa8

    
           return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID && isSystemChat);

So removing just isSystemChat will not work here. We can do something like this as we don't want to filter current user from member's list

   if (isGroupChat) {
           
            return ReportUtils.getParticipantAccountIDs(report.reportID ?? '');
        }
        if(isSystemChat){
             // Filter out the current user from the particpants of the systemChat
            return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID)
        }

What alternative solutions did you explore? (Optional)

We don't show members page if it's a system report

App/src/pages/ReportDetailsPage.tsx

Line 154 in 1865aa8

!isSystemChat

We can remover filtering logic here

 const participants = useMemo(() => {
        if (isGroupChat) {
           
            return ReportUtils.getParticipantAccountIDs(report.reportID ?? '');
        }

        return ReportUtils.getVisibleChatMemberAccountIDs(report.reportID ?? '');
    }, [report, session, isGroupChat, isSystemChat]);

ShridharGoel · 2024-06-04T16:41:25Z

Proposal

Updated to add full function code.

Nodebrute · 2024-06-04T16:42:30Z

Proposal Updated

Alternative solution added

ShridharGoel · 2024-06-04T16:44:11Z

Proposal

Updated to add few more details.

aldo-expensify · 2024-06-04T18:13:43Z

@francoisl I agree with @ShridharGoel on that this seems to have been cause by this change:

https://github.com/Expensify/App/pull/41290/files#diff-409b0e2fb439856e9897b808014031ce244a427d116ed30f37a2e0117599adc4R95-R97

What do you think of the proposed solution:

#43042 (comment)

aldo-expensify · 2024-06-04T18:15:22Z

cc @NikkiWines ^

aldo-expensify · 2024-06-04T18:18:14Z

@ShridharGoel

I don't think we should change to use getVisibleChatMemberAccountIDs for group chats since this exception was introduced by Marc here: https://github.com/Expensify/App/pull/39757/files#diff-409b0e2fb439856e9897b808014031ce244a427d116ed30f37a2e0117599adc4R84-R88, and I guess he had a reason for it.

Nodebrute · 2024-06-04T18:19:08Z

@aldo-expensify You can check my proposal here #43042 (comment)

aldo-expensify · 2024-06-04T18:25:23Z

Yep, that sounds like the easiest best fix. About the alternative solution, it does make sense to me, but I'm not 100% that there won't be regressions since the participants variable is used in multiple places below and I'm not familiar with the code.

Nodebrute · 2024-06-04T18:26:43Z

@aldo-expensify You can assign this to me. I'll raise a PR with my main solution.

melvin-bot · 2024-06-04T18:31:12Z

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

roryabraham · 2024-06-06T17:39:18Z

no longer a deploy blocker, has been fixed.

melvin-bot · 2024-06-11T23:25:47Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

[CP staging] Fix system chat participant count #43054

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

@ShridharGoel requires payment (Needs manual offer from BZ)

melvin-bot · 2024-06-11T23:25:48Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@aldo-expensify] The PR that introduced the bug has been identified. Link to the PR:
[@aldo-expensify] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@aldo-expensify] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@ShridharGoel] Determine if we should create a regression test for this bug.
[@ShridharGoel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2024-06-13T15:24:06Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

[CP staging] Fix system chat participant count #43054

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

@ShridharGoel requires payment (Needs manual offer from BZ)

melvin-bot · 2024-06-13T15:24:07Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@aldo-expensify] The PR that introduced the bug has been identified. Link to the PR:
[@aldo-expensify] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@aldo-expensify] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@ShridharGoel] Determine if we should create a regression test for this bug.
[@ShridharGoel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

sonialiap · 2024-06-20T10:52:54Z

Payment summary:

@ShridharGoel contributor $250 - paid ✔️

melvin-bot · 2024-06-20T18:01:56Z

Payment Summary

Upwork Job

ROLE: @ShridharGoel paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@sonialiap)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

sonialiap · 2024-06-24T10:17:04Z

@ShridharGoel please accept the offer and complete the checklist steps in which you are tagged

sonialiap · 2024-07-03T09:41:39Z

@ShridharGoel please accept the offer. I've also sent you a message in Upwork in case you see that first

ShridharGoel · 2024-07-03T14:34:43Z

Accepted, thanks.

sonialiap · 2024-07-03T20:09:12Z

@ShridharGoel please complete the checklist

ShridharGoel · 2024-07-03T20:16:16Z

@sonialiap I think that would be handled by @aldo-expensify.

sonialiap · 2024-07-04T08:17:47Z

@ShridharGoel melvin assigned you to the regression part of the checklist

melvin-bot · 2024-07-29T18:52:21Z

This issue has not been updated in over 15 days. @sonialiap, @ShridharGoel, @aldo-expensify 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!

ShridharGoel · 2024-07-30T10:37:47Z

Determine if we should create a regression test for this bug.

Looks like no additional test is needed, if we already have a test to check the member count when offline.

lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jun 4, 2024

github-actions bot added the Engineering label Jun 4, 2024

github-actions bot added the Hourly KSv2 label Jun 4, 2024

melvin-bot bot assigned aldo-expensify Jun 4, 2024

github-actions bot mentioned this issue Jun 4, 2024

Deploy Checklist: New Expensify 2024-06-04 #43037

Closed

melvin-bot bot mentioned this issue Jun 4, 2024

[CRITICAL] [Groups] Frontend (Part 2): Update the Group Chat creation flow UI and Details page #34927

Closed

9 tasks

aldo-expensify mentioned this issue Jun 4, 2024

[CP staging] Fix system chat participant count #43054

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Jun 4, 2024

aldo-expensify added the Bug Something is broken. Auto assigns a BugZero manager. label Jun 4, 2024

melvin-bot bot assigned sonialiap Jun 4, 2024

melvin-bot bot added the Daily KSv2 label Jun 4, 2024

roryabraham removed the DeployBlockerCash This issue or pull request should block deployment label Jun 6, 2024

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jun 11, 2024

melvin-bot bot changed the title ~~[HOLD for payment 2024-06-13] Group chat - Member amount is 0 when creating group chat offline~~ [HOLD for payment 2024-06-18] [HOLD for payment 2024-06-13] Group chat - Member amount is 0 when creating group chat offline Jun 11, 2024

melvin-bot bot added Daily KSv2 Weekly KSv2 and removed Weekly KSv2 Daily KSv2 labels Jun 12, 2024

sonialiap changed the title ~~[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [HOLD for payment 2024-06-13] Group chat - Member amount is 0 when creating group chat offline~~ [HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline Jun 19, 2024

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

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

melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Jul 29, 2024

sonialiap closed this as completed Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline #43042

[HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline #43042

lanitochka17 commented Jun 4, 2024 •

edited by sonialiap

Loading

melvin-bot bot commented Jun 4, 2024

github-actions bot commented Jun 4, 2024

lanitochka17 commented Jun 4, 2024

ShridharGoel commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading

Nodebrute commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024

Nodebrute commented Jun 4, 2024

ShridharGoel commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

Nodebrute commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

Nodebrute commented Jun 4, 2024 •

edited

Loading

melvin-bot bot commented Jun 4, 2024

roryabraham commented Jun 6, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 13, 2024

melvin-bot bot commented Jun 13, 2024

sonialiap commented Jun 20, 2024 •

edited

Loading

melvin-bot bot commented Jun 20, 2024

sonialiap commented Jun 24, 2024 •

edited

Loading

sonialiap commented Jul 3, 2024

ShridharGoel commented Jul 3, 2024

sonialiap commented Jul 3, 2024

ShridharGoel commented Jul 3, 2024

sonialiap commented Jul 4, 2024

melvin-bot bot commented Jul 29, 2024

ShridharGoel commented Jul 30, 2024

[HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline #43042

[HOLD for payment 2024-06-20] Group chat - Member amount is 0 when creating group chat offline #43042

Comments

lanitochka17 commented Jun 4, 2024 • edited by sonialiap Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jun 4, 2024

github-actions bot commented Jun 4, 2024

lanitochka17 commented Jun 4, 2024

ShridharGoel commented Jun 4, 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?

ShridharGoel commented Jun 4, 2024 • edited Loading

Nodebrute commented Jun 4, 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?

What alternative solutions did you explore? (Optional)

ShridharGoel commented Jun 4, 2024

Proposal

Nodebrute commented Jun 4, 2024

ShridharGoel commented Jun 4, 2024

Proposal

aldo-expensify commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

Nodebrute commented Jun 4, 2024

aldo-expensify commented Jun 4, 2024

Nodebrute commented Jun 4, 2024 • edited Loading

melvin-bot bot commented Jun 4, 2024

roryabraham commented Jun 6, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 13, 2024

melvin-bot bot commented Jun 13, 2024

sonialiap commented Jun 20, 2024 • edited Loading

melvin-bot bot commented Jun 20, 2024

Payment Summary

BugZero Checklist (@sonialiap)

sonialiap commented Jun 24, 2024 • edited Loading

sonialiap commented Jul 3, 2024

ShridharGoel commented Jul 3, 2024

sonialiap commented Jul 3, 2024

ShridharGoel commented Jul 3, 2024

sonialiap commented Jul 4, 2024

melvin-bot bot commented Jul 29, 2024

ShridharGoel commented Jul 30, 2024

lanitochka17 commented Jun 4, 2024 •

edited by sonialiap

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading

Nodebrute commented Jun 4, 2024 •

edited

Loading

Nodebrute commented Jun 4, 2024 •

edited

Loading

sonialiap commented Jun 20, 2024 •

edited

Loading

sonialiap commented Jun 24, 2024 •

edited

Loading