Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

mWeb - Group chat - The member removed in offline is included while creating split expense #48270

Closed lanitochka17 closed 1 week ago

lanitochka17 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: 9.0.26 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/4901873 Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/home
  2. Tap on a group chat created with a user
  3. Tap header -- members
  4. Go offline
  5. Remove the member
  6. Go back to group chat page
  7. Tap plus icon and select split expense
  8. Enter an amount and tap next
  9. Note the removed member is displayed in the expense participants list
  10. Navigate back and tap header from group chat page
  11. Tap leave
  12. Note heads up last member leaving warning modal displayed

Expected Result:

If member removed in offline is included while creating split expense, then while leaving group also the member must be included and last member leaving warning modal must not be shown

Actual Result:

The member removed in offline is included while creating split expense, but while leaving group, the member is not included and last member leaving warning modal is shown

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/4ffd3e57-02bd-4361-b2d2-ba71083e33cc

View all open jobs on GitHub

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @abekkala (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 commented 2 weeks ago

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

nkdengineer commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2023-10-04T15:23:00Z.

Proposal

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

The member removed in offline is included while creating split expense, but while leaving group, the member is not included and last member leaving warning modal is shown

What is the root cause of that problem?

When we set the participant for money request flow we don't filter out the member that is pending delete

https://github.com/Expensify/App/blob/e77f20458ad13d94c7b0188acc2ed1a91bd8ccbd/src/libs/actions/IOU.ts#L7500-L7505

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

We should filter out the pending delete member as we do in ReportDetailPage here

const chatReportOtherParticipants = Object.keys(chatReport?.participants ?? {})
            .flatMap((accountID) => {
                const pendingMember = chatReport?.pendingChatMembers?.findLast((member) => member.accountID === accountID.toString());
                return !pendingMember || pendingMember.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? accountID : [];
            })
            .map(Number)
            .filter((accountID) => accountID !== currentUserAccountID);

https://github.com/Expensify/App/blob/e77f20458ad13d94c7b0188acc2ed1a91bd8ccbd/src/libs/actions/IOU.ts#L7500-L7505

Or we can use getParticipantsAccountIDsForDisplay function with shouldExcludeDeleted params as true

const chatReportOtherParticipants = ReportUtils.getParticipantsAccountIDsForDisplay(chatReport, false, true).filter((accountID) => accountID !== currentUserAccountID);
participants = chatReportOtherParticipants.map((accountID) => ({accountID, selected: true}));

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 week ago

@abekkala Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 1 week ago

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

abekkala commented 1 week ago

Attendee List is not based on added members.

nkdengineer commented 1 week ago

Attendee List is not based on added members.

@abekkala Can you clarify this? Is this not a bug?