[HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread #38689

lanitochka17 · 2024-03-20T17:05:52Z

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.55-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: N/S
Issue reported by: Applause - Internal Team

Action Performed:

Go to staging.new.expensify.com
Go to 1:1 DM
Send a message, reply in thread and leave thread
Note that user is redirected to 1:1 DM in Step 2
Click the Search icon in the LGN > type 'you' > start a self DM
Send a message, reply in thread and leave thread

Expected Result:

After leaving thread, app will return to self-DM

Actual Result:

After leaving thread, app returns to a previous conversation before self-DM

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6420704_1710951422604.20240321_001108.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~016b6d3e4e9cf9c054
Upwork Job ID: 1770633111502032896
Last Price Increase: 2024-03-21
Automatic offers:

abzokhattab | Contributor | 0

melvin-bot · 2024-03-20T17:06:04Z

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

lanitochka17 · 2024-03-20T17:06:05Z

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

lanitochka17 · 2024-03-20T17:06:21Z

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

abzokhattab · 2024-03-20T17:21:24Z

Proposal

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

App returns to previous conversation instead of self-DM after leaving self-DM thread

What is the root cause of that problem?

we dont consider the self dm when calculating the lastAccessedReportID here which was added in this PR:

App/src/libs/actions/Report.ts

Lines 2363 to 2379 in 26ab4aa

    
           // We want to filter out the current report, hidden reports and empty chats 
        
           const filteredReportsByLastRead = sortedReportsByLastRead.filter( 
        
               (sortedReport) => 
        
                   sortedReport?.reportID !== reportID && 
        
                   sortedReport?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN && 
        
                   ReportUtils.shouldReportBeInOptionList({ 
        
                       report: sortedReport, 
        
                       currentReportId: '', 
        
                       isInGSDMode: false, 
        
                       betas: [], 
        
                       policies: {}, 
        
                       excludeEmptyChats: true, 
        
                       doesReportHaveViolations: false, 
        
                   }), 
        
           ); 
        
           const lastAccessedReportID = filteredReportsByLastRead.at(-1)?.reportID;

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

we need to add includeSelfDM: true, to the shouldReportBeInOptionList function input above

POC

Screen.Recording.2024-03-20.at.7.23.21.PM.mov

nkdengineer · 2024-03-20T18:27:43Z

Proposal

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

Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread

What is the root cause of that problem?

In here, we allow user to leave in case of selfDM 's thread.
And when leaving room, we do not include the selfDM in here, that leads to the bug.

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

We should not allow user to leave the selftDM 's thread because the self-DM is the spot for private notes. We can do it by update this to:

    const outerMostReportID = last(getAllAncestorReportActionIDs(report).reportIDs)
    const isSelfDMThread = ReportUtils.isSelfDM(getReport(outerMostReportID))
    const canJoinOrLeave = !isSelfDMThread && !isSelfDM && (isChatThread || isUserCreatedPolicyRoom || canLeaveRoom);

What alternative solutions did you explore? (Optional)

Additonally, because the self-DM is the spot for private notes, so we should not display the selfDM 's threads in LHN. We can do it by adding the below logic to this one:

    const outerMostReportID = last(getAllAncestorReportActionIDs(report).reportIDs);
    const isSelfDMThread = isSelfDM(getReport(outerMostReportID));
    if (isSelfDMThread) {
        return false;
    }

allgandalf · 2024-03-20T18:28:12Z

Proposal

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

App returns to previous conversation instead of self-DM after leaving self-DM thread

What is the root cause of that problem?

We have not set includeSelfDM to true in shouldReportBeInOptionList:

App/src/libs/actions/Report.ts

Lines 2348 to 2357 in 399077f

    
                   ReportUtils.shouldReportBeInOptionList({ 
        
                       report: sortedReport, 
        
                       currentReportId: '', 
        
                       isInGSDMode: false, 
        
                       betas: [], 
        
                       policies: {}, 
        
                       excludeEmptyChats: true, 
        
                       doesReportHaveViolations: false, 
        
                   }), 
        
           );

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

When saw other pages which use shouldReportBeInOptionList we pass the value of includeSelfDM as true everywhere.

App/src/libs/SidebarUtils.ts

Lines 85 to 93 in 399077f

    
           return ReportUtils.shouldReportBeInOptionList({ 
        
               report, 
        
               currentReportId: currentReportId ?? '', 
        
               isInGSDMode, 
        
               betas, 
        
               policies, 
        
               excludeEmptyChats: true, 
        
               doesReportHaveViolations, 
        
               includeSelfDM: true,

App/src/libs/OptionsListUtils.ts

Lines 1451 to 1459 in 399077f

    
           return ReportUtils.shouldReportBeInOptionList({ 
        
               report, 
        
               currentReportId: Navigation.getTopmostReportId() ?? '', 
        
               betas, 
        
               policies, 
        
               doesReportHaveViolations, 
        
               isInGSDMode: false, 
        
               excludeEmptyChats: false, 
        
               includeSelfDM,

App/src/libs/OptionsListUtils.ts

Lines 1451 to 1459 in 399077f

    
           return ReportUtils.shouldReportBeInOptionList({ 
        
               report, 
        
               currentReportId: Navigation.getTopmostReportId() ?? '', 
        
               betas, 
        
               policies, 
        
               doesReportHaveViolations, 
        
               isInGSDMode: false, 
        
               excludeEmptyChats: false, 
        
               includeSelfDM,

So to avoid redundancy of code i propose that we update the shouldReportBeInOptionList function itself to set the default value of includeSelfDM to true, currently it is set to false:

App/src/libs/ReportUtils.ts

Lines 4176 to 4185 in 399077f

    
           function shouldReportBeInOptionList({ 
        
               report, 
        
               currentReportId, 
        
               isInGSDMode, 
        
               betas, 
        
               policies, 
        
               excludeEmptyChats, 
        
               doesReportHaveViolations, 
        
               includeSelfDM = false, 
        
           }: {

Then we remove the redundant part of the code in every component

What alternative solutions did you explore? (Optional)

N/A

Christinadobrzyn · 2024-03-21T02:06:39Z

I can recreate this based on the steps in the OP - I think this can be external and part of vip-vsb

melvin-bot · 2024-03-21T02:06:47Z

Job added to Upwork: https://www.upwork.com/jobs/~016b6d3e4e9cf9c054

melvin-bot · 2024-03-21T02:06:51Z

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

Christinadobrzyn · 2024-03-22T03:44:57Z

we have some proposals to review when you have a chance @Santhosh-Sellavel!

shubham1206agra · 2024-03-22T04:29:31Z

@ishpaul777 Can you help here as you were the author of self DM?

Santhosh-Sellavel · 2024-03-25T17:53:52Z

@abzokhattab's proposal looks good and straight-forward!

C+ Reviewed
🎀 👀 🎀

melvin-bot · 2024-03-25T17:54:08Z

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

nkdengineer · 2024-03-25T17:55:58Z

@luacmartins @Santhosh-Sellavel Should we hide the "Leave" option in case of selfDM? May be it is related to this comment

Santhosh-Sellavel · 2024-03-25T18:16:08Z

Should we hide the "Leave" option in case of selfDM?

Yes for self-dm we should hide leave, not for self-dm thread

nkdengineer · 2024-03-25T18:21:27Z

Yeah, I mean that selfDM 's threads. Because, self-DM is the spot for private notes, only current users can access it, and its thread as well. So I think the "leave" or "join" options in self-DM are redundant

Santhosh-Sellavel · 2024-03-25T18:45:18Z

We can invite any users to specific threads. So Leave or Join makes sense to me, I'll let the team decide on it

cc: @luacmartins

luacmartins · 2024-03-25T19:00:48Z

Yea, agreed that we should hide those buttons for selfDM threads too. @Santhosh-Sellavel do you need to revise the approved proposal based on this?

shubham1206agra · 2024-03-26T01:02:26Z

@luacmartins I thought we could invite anyone on the thread related to self-DM but not self-DM itself.

Tagging @thienlnam here.

thienlnam · 2024-03-26T01:10:53Z

Yeah that's correct - you can't invite anyone to the self-DM itself, but you are able to for child reports (tasks, chat reports, etc)

luacmartins · 2024-04-01T16:22:37Z

@abzokhattab what's the ETA for the PR?

abzokhattab · 2024-04-01T16:35:48Z

will be ready today thanks @luacmartins for the ping.

abzokhattab · 2024-04-03T07:35:00Z

The PR is ready. please have a look and let me know if you have any comments.

Christinadobrzyn · 2024-04-08T06:11:12Z

PR is under review

melvin-bot · 2024-04-08T10:59:46Z

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

melvin-bot · 2024-04-08T10:59:49Z

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

Fixing the navigation of leaving self-dm thread #39476

If no regressions arise, payment will be issued on 2024-04-15. 🎊

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

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

melvin-bot · 2024-04-08T10:59:50Z

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.
[@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Christinadobrzyn · 2024-04-11T03:21:16Z

Payouts due:

Contributor: $500 @abzokhattab (Paid in Upwork here - https://www.upwork.com/nx/wm/workroom/36815600/overview)
Contributor+: $500 @Santhosh-Sellavel (in NewDot)

Upwork job is here.

@Santhosh-Sellavel do we need a regression test for this?

Santhosh-Sellavel · 2024-04-17T16:38:37Z

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: https://github.com/Expensify/App/pull/31097/files#r1569141794
[@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/31097/files#r1569141794
[@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: N/A Should have been caught based on the checklist
[@Santhosh-Sellavel] Determine if we should create a regression test for this bug. N/A As this basic should be part of tests already we won't need a separate reg tests here
[@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. N/A
[@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Christinadobrzyn · 2024-04-22T04:39:50Z

awesome! no regression test is needed. Closing this. Let me know if I missed anything.

Payment summary here for @Santhosh-Sellavel payment through NewDot.

Santhosh-Sellavel · 2024-05-22T18:08:12Z

Requested on ND

JmillsExpensify · 2024-05-31T04:30:36Z

$500 approved for @Santhosh-Sellavel

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

melvin-bot bot assigned Christinadobrzyn Mar 20, 2024

Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Mar 21, 2024

melvin-bot bot changed the title ~~Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread~~ [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread Mar 21, 2024

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

melvin-bot bot assigned Santhosh-Sellavel Mar 21, 2024

melvin-bot bot added the Overdue label Mar 25, 2024

melvin-bot bot removed the Overdue label Mar 25, 2024

melvin-bot bot assigned luacmartins Mar 25, 2024

melvin-bot bot removed the Overdue label Apr 1, 2024

abzokhattab mentioned this issue Apr 3, 2024

Fixing the navigation of leaving self-dm thread #39476

Merged

50 tasks

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

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

melvin-bot bot changed the title ~~[$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread~~ [HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread Apr 8, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Apr 8, 2024

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Apr 14, 2024

Santhosh-Sellavel mentioned this issue Apr 17, 2024

Feat: drop user to last chat after leaving a room #31097

Merged

50 tasks

melvin-bot bot added Overdue and removed Overdue labels Apr 17, 2024

Christinadobrzyn closed this as completed Apr 22, 2024

melvin-bot bot removed the Overdue label Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread #38689

[HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread #38689

lanitochka17 commented Mar 20, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Mar 20, 2024

lanitochka17 commented Mar 20, 2024

lanitochka17 commented Mar 20, 2024

abzokhattab commented Mar 20, 2024 •

edited

Loading

nkdengineer commented Mar 20, 2024 •

edited

Loading

allgandalf commented Mar 20, 2024

Christinadobrzyn commented Mar 21, 2024

melvin-bot bot commented Mar 21, 2024

melvin-bot bot commented Mar 21, 2024

Christinadobrzyn commented Mar 22, 2024

shubham1206agra commented Mar 22, 2024

Santhosh-Sellavel commented Mar 25, 2024

melvin-bot bot commented Mar 25, 2024

nkdengineer commented Mar 25, 2024 •

edited

Loading

Santhosh-Sellavel commented Mar 25, 2024

nkdengineer commented Mar 25, 2024 •

edited

Loading

Santhosh-Sellavel commented Mar 25, 2024

luacmartins commented Mar 25, 2024 •

edited

Loading

shubham1206agra commented Mar 26, 2024

thienlnam commented Mar 26, 2024

luacmartins commented Apr 1, 2024

abzokhattab commented Apr 1, 2024

abzokhattab commented Apr 3, 2024

Christinadobrzyn commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

Christinadobrzyn commented Apr 11, 2024 •

edited

Loading

Santhosh-Sellavel commented Apr 17, 2024

Christinadobrzyn commented Apr 22, 2024

Santhosh-Sellavel commented May 22, 2024

JmillsExpensify commented May 31, 2024

[HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread #38689

[HOLD for payment 2024-04-15] [$500] Self - DM - App returns to previous conversation instead of self-DM after leaving self-DM thread #38689

Comments

lanitochka17 commented Mar 20, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 20, 2024

lanitochka17 commented Mar 20, 2024

lanitochka17 commented Mar 20, 2024

abzokhattab commented Mar 20, 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?

POC

nkdengineer commented Mar 20, 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)

allgandalf commented Mar 20, 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)

Christinadobrzyn commented Mar 21, 2024

melvin-bot bot commented Mar 21, 2024

melvin-bot bot commented Mar 21, 2024

Christinadobrzyn commented Mar 22, 2024

shubham1206agra commented Mar 22, 2024

Santhosh-Sellavel commented Mar 25, 2024

melvin-bot bot commented Mar 25, 2024

nkdengineer commented Mar 25, 2024 • edited Loading

Santhosh-Sellavel commented Mar 25, 2024

nkdengineer commented Mar 25, 2024 • edited Loading

Santhosh-Sellavel commented Mar 25, 2024

luacmartins commented Mar 25, 2024 • edited Loading

shubham1206agra commented Mar 26, 2024

thienlnam commented Mar 26, 2024

luacmartins commented Apr 1, 2024

abzokhattab commented Apr 1, 2024

abzokhattab commented Apr 3, 2024

Christinadobrzyn commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

melvin-bot bot commented Apr 8, 2024

Christinadobrzyn commented Apr 11, 2024 • edited Loading

Santhosh-Sellavel commented Apr 17, 2024

Christinadobrzyn commented Apr 22, 2024

Santhosh-Sellavel commented May 22, 2024

JmillsExpensify commented May 31, 2024

lanitochka17 commented Mar 20, 2024 •

edited by melvin-bot bot

Loading

abzokhattab commented Mar 20, 2024 •

edited

Loading

nkdengineer commented Mar 20, 2024 •

edited

Loading

nkdengineer commented Mar 25, 2024 •

edited

Loading

nkdengineer commented Mar 25, 2024 •

edited

Loading

luacmartins commented Mar 25, 2024 •

edited

Loading

Christinadobrzyn commented Apr 11, 2024 •

edited

Loading