[HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060

lanitochka17 · 2024-01-05T20:20:35Z

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.22-0
Reproducible in staging?: Y
Reproducible in production?: Y
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Click FAB > Request money > Manual
Split bill with two users
Click FAB > Request money > Scan
Create a split scan request with the same two users in Step 2
Note that the split scan creates two same groups, one with the history and another new group. The new group becomes skeleton placeholder after a while

Expected Result:

The split scan request will be created in the existing group chat which is created in Step 2

Actual Result:

The split scan request is created in both existing and new group chat. The request disappears and leaves skeleton placeholder after a while in the new group chat

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6333455_1704483528937.bandicam_2024-01-05_18-27-06-548.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0147a95509543e3f60
Upwork Job ID: 1743366914858815488
Last Price Increase: 2024-01-12
Automatic offers:

bernhardoj | Contributor | 28104489

Issue Owner

Current Issue Owner: @stitesExpensify

melvin-bot · 2024-01-05T20:20:39Z

Job added to Upwork: https://www.upwork.com/jobs/~0147a95509543e3f60

melvin-bot · 2024-01-05T20:20:39Z

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

melvin-bot · 2024-01-05T20:20:41Z

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

melvin-bot · 2024-01-05T20:20:44Z

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

unidev727 · 2024-01-05T20:40:45Z

Proposal

from: @unicorndev-727

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

Workspace - In Workspace, trying to add a number contact & selecting it, contact disappears

What is the root cause of that problem?

The root cause is that we are using randomly created reportId here.

App/src/pages/iou/request/step/IOURequestStepConfirmation.js

Line 220 in 7ae1ec7

    
           const existingSplitChatReportID = CONST.REGEX.NUMBER.test(reportID) ? reportID : '';

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

We don't need to send reportId so that we can find reportId from getChatByParticipants here

App/src/pages/iou/request/step/IOURequestStepConfirmation.js

Lines 222 to 230 in 7ae1ec7

    
               selectedParticipants, 
        
               currentUserPersonalDetails.login, 
        
               currentUserPersonalDetails.accountID, 
        
               trimmedComment, 
        
               transaction.category, 
        
               transaction.tag, 
        
               receiptFile, 
        
               existingSplitChatReportID, 
        
           );

to

IOU.startSplitBill(
                    selectedParticipants,
                    currentUserPersonalDetails.login,
                    currentUserPersonalDetails.accountID,
                    trimmedComment,
                    transaction.category,
                    transaction.tag,
                    receiptFile
                );

App/src/libs/actions/IOU.js

Lines 1700 to 1702 in 7ae1ec7

    
           const existingSplitChatReport = 
        
               existingSplitChatReportID || participants[0].reportID 
        
                   ? allReports[`${ONYXKEYS.COLLECTION.REPORT}${existingSplitChatReportID || participants[0].reportID}`]

screen-capture.2.webm

What alternative solutions did you explore?

We can find existing reportId using getChatByParticipants in ReportUtils for further correctness in IOURequestStepConfirmation page.
const existingSplitChatReportID = ReportUtils.getChatByParticipants(_.map(selectedParticipants, (participant) => Number(participant.accountID))) ?? '';

bernhardoj · 2024-01-05T21:24:43Z

Proposal

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

Every time we create a new scan split bill, it will create the request on a new group chat.

What is the root cause of that problem?

If we create a scan split bill, it will call startSplitBill. In this function, we check if there is an existing group chat for the split bill, if it doesn't exist, we create a new one.

App/src/libs/actions/IOU.js

Lines 1700 to 1704 in 1752b97

    
           const existingSplitChatReport = 
        
               existingSplitChatReportID || participants[0].reportID 
        
                   ? allReports[`${ONYXKEYS.COLLECTION.REPORT}${existingSplitChatReportID || participants[0].reportID}`] 
        
                   : ReportUtils.getChatByParticipants(participantAccountIDs); 
        
           const splitChatReport = existingSplitChatReport || ReportUtils.buildOptimisticChatReport(participantAccountIDs);

The way we get the existing group chat is by checking either existingSplitChatReportID and participants[0]?.reportID (for policy expense chat). If it exists, we will get it from the report Onyx object. If not, we will get the report with getChatByParticipants.

However, after the big money request refactor, existingSplitChatReportID always exists because every time we open the money request page, we generate a new random report ID. So, we always trying to get the non-existing report because existingSplitChatReportID is not empty.

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

Because existingSplitChatReportID always exists, we can update the condition to,

const existingSplitChatReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${existingSplitChatReportID}`] || allReports[`${ONYXKEYS.COLLECTION.REPORT}${participants[0]?.reportID}`] || ReportUtils.getChatByParticipants(participantAccountIDs);

What alternative solutions did you explore? (Optional)

The existingSplitChatReportID logic also exists on normal splitBill, but if we look at the splitBill calls, we pass report.reportID to the existingSplitChatReportID params, so we can do the same.

App/src/pages/iou/request/step/IOURequestStepConfirmation.js

Lines 237 to 248 in 0c351a4

    
           IOU.splitBill( 
        
               selectedParticipants, 
        
               currentUserPersonalDetails.login, 
        
               currentUserPersonalDetails.accountID, 
        
               transaction.amount, 
        
               trimmedComment, 
        
               transaction.currency, 
        
               transaction.merchant, 
        
               transaction.category, 
        
               transaction.tag, 
        
               report.reportID, 
        
           );

isabelastisser · 2024-01-08T17:59:05Z

@Santhosh-Sellavel please review the proposals above. Thanks!

isabelastisser · 2024-01-09T18:03:53Z

Bump @Santhosh-Sellavel

Santhosh-Sellavel · 2024-01-09T18:53:35Z

Thanks for the bump, I will update on this tomorrow.

Santhosh-Sellavel · 2024-01-10T18:28:08Z

Both proposals are pretty much similar, will discuss them internally and update here.

melvin-bot · 2024-01-12T16:00:06Z

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

isabelastisser · 2024-01-12T18:18:11Z

@Santhosh-Sellavel any updates? Thanks!

Santhosh-Sellavel · 2024-01-12T18:46:13Z

Adding to get a CME here
C+ Reviewed
🎀 👀 🎀

melvin-bot · 2024-01-12T18:46:20Z

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

Santhosh-Sellavel · 2024-01-12T19:09:04Z

After looking again seems I missed @bernhardoj's alternate proposal, straightforward one. So let's go with them here, thanks!

C+ Reviewed
🎀 👀 🎀
cc: @stitesExpensify

melvin-bot · 2024-01-12T19:09:09Z

Current assignee @stitesExpensify is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot · 2024-01-16T17:24:40Z

@isabelastisser, @stitesExpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

stitesExpensify · 2024-01-16T17:46:13Z

Agreed. Assigning @bernhardoj for their alternate proposal

bernhardoj · 2024-01-17T07:46:03Z

PR is ready

cc: @Santhosh-Sellavel

melvin-bot · 2024-01-19T09:38:59Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-01-19T09:39:02Z

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

Fix a new group is always created on creating a split scan request #34634

If no regressions arise, payment will be issued on 2024-01-26. 🎊

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

@bernhardoj requires payment automatic offer (Contributor)
@Santhosh-Sellavel requires payment through NewDot Manual Requests

melvin-bot · 2024-01-19T09:39:04Z

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:

[@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
[@Santhosh-Sellavel] 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:
[@Santhosh-Sellavel] 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:
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
[@Santhosh-Sellavel] 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.
[@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

isabelastisser · 2024-01-29T12:47:43Z

Hey @Santhosh-Sellavel, can you please complete the checklist above? Thanks!

isabelastisser · 2024-01-29T12:52:13Z

Payment summary:

@bernhardoj requires payment automatic offer (Contributor) $500 paid in Upwork.
@Santhosh-Sellavel requires payment through NewDot Manual Requests $500 PENDING PAYMENT IN NEWDOT.

isabelastisser · 2024-01-29T12:56:30Z

Waiting for @Santhosh-Sellavel to complete the BZ list.

isabelastisser · 2024-01-30T14:46:08Z

Bump @Santhosh-Sellavel

isabelastisser · 2024-01-31T19:19:03Z

waiting for @Santhosh-Sellavel.

isabelastisser · 2024-02-01T21:39:38Z

Bump @Santhosh-Sellavel, please let me know if I need to reassign this issue.

Santhosh-Sellavel · 2024-02-02T19:25:49Z

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:

[@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR: Refactor the money request creation flow #28618
[@Santhosh-Sellavel] 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: https://github.com/Expensify/App/pull/28618/files#r1476574472
[@Santhosh-Sellavel] 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: This should be found while testing
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug. Yes
[@Santhosh-Sellavel] 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. [HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060 (comment)
[@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Santhosh-Sellavel · 2024-02-02T19:26:17Z

Bug:
Split scan with existing group members is created in both existing and new group chat

Regression Steps

Create a new group chat with any user (if you don't have one)
Create a split scan request from FAB with the participants from the group chat above
Verify the request is added to the group chat above and not create a duplicate group chat
Create a split scan request from the group chat composer add (+) button
Verify the request is added to the group chat above and not create a duplicate group chat

isabelastisser · 2024-02-05T17:09:11Z

Issue for regression test is created.

PENDING

@Santhosh-Sellavel requires payment through NewDot Manual Requests $500 PENDING PAYMENT IN NEWDOT.

Santhosh-Sellavel · 2024-02-10T20:37:25Z

Requested on ND

JmillsExpensify · 2024-02-11T13:05:06Z

$500 approved for @Santhosh-Sellavel based on summary above.

lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 5, 2024

melvin-bot bot changed the title ~~Split - Split scan with existing group members is created in both existing and new group chat~~ [$500] Split - Split scan with existing group members is created in both existing and new group chat Jan 5, 2024

melvin-bot bot assigned isabelastisser Jan 5, 2024

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

melvin-bot bot assigned Santhosh-Sellavel Jan 5, 2024

melvin-bot bot added the Overdue label Jan 8, 2024

melvin-bot bot removed the Overdue label Jan 8, 2024

melvin-bot bot assigned stitesExpensify Jan 12, 2024

melvin-bot bot added the Overdue label Jan 14, 2024

melvin-bot bot removed the Overdue label Jan 16, 2024

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 17, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 19, 2024

melvin-bot bot changed the title ~~[$500] Split - Split scan with existing group members is created in both existing and new group chat~~ [HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat Jan 19, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jan 19, 2024

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

isabelastisser removed the Overdue label Jan 29, 2024

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

isabelastisser removed the Overdue label Jan 31, 2024

melvin-bot bot added the Overdue label Jan 31, 2024

Santhosh-Sellavel mentioned this issue Feb 2, 2024

Refactor the money request creation flow #28618

Merged

58 tasks

isabelastisser closed this as completed Feb 5, 2024

melvin-bot bot removed the Overdue label Feb 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060

[HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060

lanitochka17 commented Jan 5, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jan 5, 2024

melvin-bot bot commented Jan 5, 2024

melvin-bot bot commented Jan 5, 2024

melvin-bot bot commented Jan 5, 2024

unidev727 commented Jan 5, 2024 •

edited

Loading

bernhardoj commented Jan 5, 2024 •

edited

Loading

isabelastisser commented Jan 8, 2024

isabelastisser commented Jan 9, 2024

Santhosh-Sellavel commented Jan 9, 2024

Santhosh-Sellavel commented Jan 10, 2024 •

edited

Loading

melvin-bot bot commented Jan 12, 2024

isabelastisser commented Jan 12, 2024

Santhosh-Sellavel commented Jan 12, 2024

melvin-bot bot commented Jan 12, 2024

Santhosh-Sellavel commented Jan 12, 2024

melvin-bot bot commented Jan 12, 2024

melvin-bot bot commented Jan 16, 2024

stitesExpensify commented Jan 16, 2024

bernhardoj commented Jan 17, 2024

melvin-bot bot commented Jan 19, 2024

melvin-bot bot commented Jan 19, 2024

melvin-bot bot commented Jan 19, 2024

isabelastisser commented Jan 29, 2024

isabelastisser commented Jan 29, 2024 •

edited

Loading

isabelastisser commented Jan 29, 2024

isabelastisser commented Jan 30, 2024

isabelastisser commented Jan 31, 2024

isabelastisser commented Feb 1, 2024

Santhosh-Sellavel commented Feb 2, 2024 •

edited by isabelastisser

Loading

Santhosh-Sellavel commented Feb 2, 2024

isabelastisser commented Feb 5, 2024

Santhosh-Sellavel commented Feb 10, 2024

JmillsExpensify commented Feb 11, 2024

[HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060

[HOLD for payment 2024-01-26] [$500] Split - Split scan with existing group members is created in both existing and new group chat #34060

Comments

lanitochka17 commented Jan 5, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jan 5, 2024

melvin-bot bot commented Jan 5, 2024

melvin-bot bot commented Jan 5, 2024

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Jan 5, 2024

unidev727 commented Jan 5, 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?

bernhardoj commented Jan 5, 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)

isabelastisser commented Jan 8, 2024

isabelastisser commented Jan 9, 2024

Santhosh-Sellavel commented Jan 9, 2024

Santhosh-Sellavel commented Jan 10, 2024 • edited Loading

melvin-bot bot commented Jan 12, 2024

isabelastisser commented Jan 12, 2024

Santhosh-Sellavel commented Jan 12, 2024

melvin-bot bot commented Jan 12, 2024

Santhosh-Sellavel commented Jan 12, 2024

melvin-bot bot commented Jan 12, 2024

melvin-bot bot commented Jan 16, 2024

stitesExpensify commented Jan 16, 2024

bernhardoj commented Jan 17, 2024

melvin-bot bot commented Jan 19, 2024

melvin-bot bot commented Jan 19, 2024

melvin-bot bot commented Jan 19, 2024

isabelastisser commented Jan 29, 2024

isabelastisser commented Jan 29, 2024 • edited Loading

isabelastisser commented Jan 29, 2024

isabelastisser commented Jan 30, 2024

isabelastisser commented Jan 31, 2024

isabelastisser commented Feb 1, 2024

Santhosh-Sellavel commented Feb 2, 2024 • edited by isabelastisser Loading

Santhosh-Sellavel commented Feb 2, 2024

Regression Steps

isabelastisser commented Feb 5, 2024

Santhosh-Sellavel commented Feb 10, 2024

JmillsExpensify commented Feb 11, 2024

lanitochka17 commented Jan 5, 2024 •

edited by melvin-bot bot

Loading

unidev727 commented Jan 5, 2024 •

edited

Loading

bernhardoj commented Jan 5, 2024 •

edited

Loading

Santhosh-Sellavel commented Jan 10, 2024 •

edited

Loading

isabelastisser commented Jan 29, 2024 •

edited

Loading

Santhosh-Sellavel commented Feb 2, 2024 •

edited by isabelastisser

Loading