[$500] Expense - LHN does not show expense preview when there is a thread with deleted root message #37775

lanitochka17 · 2024-03-05T17:50:46Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 1.4.47-2
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:

Go to staging.new.expensify.com
Go to workspace chat that has no pending request
Create a manual expense
Send a message to the main chat
Create another manual expense
Note that LHN shows the expense preview
Send a message to the main chat
Send a reply to the message in Step 6
Delete the parent message in Step 6
Create a manual expense

Expected Result:

LHN will show the expense preview whenever a request is made, just like Step 6

Actual Result:

LHN does not show the expense preview when the latest message is a thread with deleted parent message

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6403083_1709659019145.20240305_141324.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012901a33cf428c32c
Upwork Job ID: 1765486641961807872
Last Price Increase: 2024-04-24

melvin-bot · 2024-03-05T17:51:01Z

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

lanitochka17 · 2024-03-05T17:51:24Z

@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

lanitochka17 · 2024-03-05T17:51:49Z

We think that this bug might be related to #wave5
CC @dylanexpensify

nkdengineer · 2024-03-06T09:27:23Z

Proposal

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

Expense - LHN does not show expense preview when there is a thread with deleted root message

What is the root cause of that problem?

Before step 8, the LHN show the correct last message text. In this case, the last message text is from report.lastMessageText

But after step 10, the LHN show the incorrect last message text. It is because the lastReportAction in here is a deleted message. So the last message text is from the below:

App/src/libs/OptionsListUtils.ts

Lines 578 to 579 in 042bdcc

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) { 
        
               lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction);

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

When we get the lastReportAction, we should use both the current report's actions and the child money request actions in each report preview in the current report's actions
So, the logic to get the lastReportAction will become to:

const allReportPreviews = allSortedReportActions[report?.reportID ?? '']?.filter((reportAction) => ReportActionUtils.isReportPreviewAction(reportAction));
    let allMoneyRequestActions = [];
    forEach(allReportPreviews, (reportPreview) => {
        const allActions = allSortedReportActions[reportPreview?.childReportID];
        forEach(allActions, (action) => {
            if (ReportActionUtils.isMoneyRequestAction(action)) {
                allMoneyRequestActions.push(action);
            }
        });
    });

    const lastReportAction =
        ReportActionUtils.getSortedReportActions([...Object.values(allSortedReportActions[report?.reportID ?? '']), ...allMoneyRequestActions], true)?.find((reportAction) =>
            ReportActionUtils.shouldReportActionBeVisibleAsLastAction(reportAction),
        ) ?? null;

Then, if lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU, so we can return last message text by using the getReportPreviewMessage - that we use to get the report preview copy messge:

    if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) {
        const iouReportID = lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? lastReportAction.originalMessage.IOUReportID : '';
        const iouReport = ReportUtils.getReport(iouReportID);
        const reportPreviewAction = ReportActionsUtils.getReportPreviewAction(iouReport?.chatReportID ?? '', iouReport?.reportID ?? '');
        const displayMessage = ReportUtils.getReportPreviewMessage(iouReport, reportPreviewAction);
        return displayMessage;
    }

What alternative solutions did you explore? (Optional)

NA

melvin-bot · 2024-03-06T21:16:33Z

Job added to Upwork: https://www.upwork.com/jobs/~012901a33cf428c32c

melvin-bot · 2024-03-06T21:16:39Z

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

wildan-m · 2024-03-07T02:37:54Z

Proposal

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

Expense - LHN does not show expense preview when there is a thread with deleted root message

What is the root cause of that problem?

If the previous message is a deleted parent chat that contains a thread, the LHN will check and display the deleted message instead of report?.lastMessageText.

App/src/libs/OptionsListUtils.ts

Lines 577 to 579 in 2979e3c

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) { 
        
               lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction); 
        
           } else if (ReportActionUtils.isPendingRemove(lastReportAction) && ReportActionUtils.isThreadParentMessage(lastReportAction, report?.reportID ?? '')) {

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

If we want the previously deleted message to behave the same as other messages after creating a new money request, we can avoid entering the isDeletedParentAction check by comparing lastReportAction?.created and report?.lastVisibleActionCreated. Change:

App/src/libs/OptionsListUtils.ts

Line 577 in 2979e3c

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) {

To:

    } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report) && lastReportAction?.created === report?.lastVisibleActionCreated) {

Or enter the condition only if report?.lastVisibleActionCreated is the most recent or equal to lastReportAction?.created.

Please note that whether the previous message is a deleted chat or other chat types, the text preview for Request Money will be removed after page refresh, because we are not modifying the sorting algorithm. In my opinion, the ideal solution is to actually move the ReportActionPreview as described in my previous suggestion (see alternative solution)

What alternative solutions did you explore? (Optional)

I believe the problem is linked to the reportpreview position. It may be best to postpone addressing this as it could be resolved in #37245.

If deemed separate, I suggest the same solution for this issue.

Proposal link.

jjcoffee · 2024-03-07T15:20:06Z

@wildan-m Thanks for the heads up! If you have time and don't mind, it'd be great if you could post a proposal specific to this issue as it does sound different to me.

wildan-m · 2024-03-07T15:25:37Z

@jjcoffee, please check the position of the report preview as it is currently stationary due to an active report preview. The last message in the LHN is determined by this sorting algorithm.

App/src/libs/ReportActionsUtils.ts

Lines 220 to 225 in a0e444e

    
           const sortedActions = reportActions?.filter(Boolean).sort((first, second) => { 
        
               // First sort by timestamp 
        
               if (first.created !== second.created) { 
        
                   return (first.created < second.created ? -1 : 1) * invertedMultiplier; 
        
               }

Please test this branch in offline mode, as it requires backend adjustments.

jjcoffee · 2024-03-11T13:20:25Z

Hmm on retesting this, now I'm getting that step 6 also doesn't show the expense preview in LHN, i.e. after an initial request, send a message, then make another request, and LHN still just shows the message. Weirdly, I seem to get the expense message in the LHN "after a while" or after signing out/in.

jjcoffee · 2024-03-11T13:32:37Z

@wildan-m The other issue you've proposed for is quite different, so a proposal directed specifically to this issue would be more appropriate here.

@nkdengineer Thanks for the proposal, and sorry I think I must have overlooked this previously! 🙇 For your RCA I'd like to hear some more detail around why this is specifically happening after a deleted thread parent message, rather than any time a further money request is made.

nkdengineer · 2024-03-11T14:49:51Z

@jjcoffee

why this is specifically happening after a deleted thread parent message

It is because after step 10, the lastReportAction in here is a deleted message. So the last message text is from the below:

App/src/libs/OptionsListUtils.ts

Lines 578 to 579 in 042bdcc

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) { 
        
               lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction);

And as you can see, getDeletedParentActionMessageForChatReport just return Deleted message or Deleted task

jjcoffee · 2024-03-11T15:00:37Z

@nkdengineer Sorry, what I'm getting at is that presumably the lastReportAction is correctly the (new) expense action in step 6, so why does it become incorrect after the deleted message? I think that's missing from your RCA.

nkdengineer · 2024-03-11T15:53:01Z

The last message to display after step 6 and after step 10 are from getLastMessageTextForReport
In case after step 6, the logic to get the last message is:

App/src/libs/OptionsListUtils.ts

Line 594 in 3d4504e

return lastMessageTextFromReport || (report?.lastMessageText ?? '');

because the lastReportAction in this case is text message

In case after step 10, the logic to get last message is:

App/src/libs/OptionsListUtils.ts

Lines 576 to 577 in 3d4504e

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) { 
        
               lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction);

because the lastReportAction is deleted parent message

andrey010 · 2024-03-11T20:32:04Z

Proposal

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

LHN does not show the expense preview when the latest message is a thread with deleted parent message

What is the root cause of that problem?

OptionsListUtils.getLastMessageTextForReport return as lastMesasgeText [Deleted message], it happens because lastReportAction

App/src/libs/OptionsListUtils.ts

Line 531 in 4239b37

    
           const lastReportAction = allSortedReportActions[report?.reportID ?? '']?.find((reportAction) => ReportActionUtils.shouldReportActionBeVisibleAsLastAction(reportAction)) ?? null;

isDeletedParentAction but not report preview actions and function execute next line with condition

App/src/libs/OptionsListUtils.ts

Line 576 in 4239b37

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) {

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

I think should add additional condition and set lastReportAction = previewAction

    const isDeletedParentAction = ReportActionUtils.isDeletedParentAction(lastReportAction)
    const reportPreview = allSortedReportActions[report?.reportID ?? '']?.find(reportAction => ReportActionUtils.isReportPreviewAction(reportAction))
    
    if (isDeletedParentAction && reportPreview) lastReportAction = reportPreview

in this way getLastMessageTextForReport function will execute next condition

App/src/libs/OptionsListUtils.ts

Line 556 in 4239b37

} else if (ReportActionUtils.isReportPreviewAction(lastReportAction)) {

What alternative solutions did you explore? (Optional)

Also reportPreviewAction has not updated created, lastModified values after request additional money, so I think if also if update create value, sortedActions sort callback will return reportPreview as last action after deleting parent message
or maybe should update lastModified and add additional condition into sortedActions sort callback

jjcoffee · 2024-03-13T12:09:16Z

The missing piece here is that the lastReportAction won't update with new money requests, so following the steps here, the lastReportAction will always be the message (ADDCOMMENT report action), even after subsequent money requests. The report's lastMessageText (from the BE) is correct though, and we display it directly if we have no overrides:

App/src/libs/OptionsListUtils.ts

Line 594 in 3d4504e

return lastMessageTextFromReport || (report?.lastMessageText ?? '');

This is how initially the new money request preview is correctly shown in the LHN. However, once we delete that message, we override the report's lastMessageText because of this condition:

App/src/libs/OptionsListUtils.ts

Line 576 in 4239b37

    
           } else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) {

I think I'm therefore most interested in a solution that just continues to use the report's lastMessageText in this situation, rather than the current proposals which seem more like workarounds when we already have the correct value to display.

andrey010 · 2024-03-13T12:32:21Z

@jjcoffee what if update created in the report preview action after a new money request ? in this way after sorting, report preview action will be as lastReportAction

jjcoffee · 2024-03-13T13:43:03Z

@andrey010 I think that would cause a regression as it would also move the money request report preview in the chat report itself.

bfitzexpensify · 2024-04-07T13:01:54Z

Waiting for confirmation of the expected behaviour via the question in #37245 (comment) - @Beamanator, don't suppose you know the answer?

melvin-bot · 2024-04-10T16:00:27Z

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

melvin-bot · 2024-04-10T18:01:53Z

@Beamanator, @abekkala, @jjcoffee Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Beamanator · 2024-04-11T12:26:55Z

Oofda sorry for my delay here!

I have some new repro steps that demonstrate the more general issue (i.e. not only involving deleting a parent thread message):

Request money from a workspace

Add a message

Verify the LHN shows the message

Request money again

Verify the LHN shows the money request (xxx owes $x.xx)

Refresh

The LHN now shows the last message from (2), even though nothing has changed

I like @jjcoffee 's simplified steps here which definitely show a more general problem. @bfitzexpensify @miljakljajic I feel like we need a slack discussion to resolve the "desired behavior" for this issue & for this other issue in 1 place. Do either of you mind starting that convo in #expensify-open-source?

mvtglobally · 2024-04-12T05:26:49Z

Issue not reproducible during KI retests. (First week)

melvin-bot · 2024-04-16T18:03:36Z

@Beamanator, @abekkala, @jjcoffee Huh... This is 4 days overdue. Who can take care of this?

jjcoffee · 2024-04-17T07:44:51Z

Issue not reproducible during KI retests. (First week)

On v1.4.62-13, after step 9

Delete the parent message in Step 6

I'm still seeing an incorrect message in the LHN, but now it's No activity yet. The rest seems to be fixed.

desktop-chrome-lhn-retest-2024-04-17_09.41.43.mp4

melvin-bot · 2024-04-17T16:00:34Z

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

mvtglobally · 2024-04-18T15:53:50Z

Issue not reproducible during KI retests. (Second week)

jjcoffee · 2024-04-22T15:15:34Z

@abekkala Should we handle this related issue here?

abekkala · 2024-04-22T18:38:06Z

ah, if there is still an issue occurring with the same flow, then yes

jjcoffee · 2024-04-23T07:42:42Z

Bumped on Slack for proposals!

melvin-bot · 2024-04-24T16:03:59Z

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

jjcoffee · 2024-04-25T15:07:26Z

Still waiting on proposals!

Beamanator · 2024-04-29T09:02:39Z

Still awaiting proposals

wildan-m · 2024-04-30T03:14:30Z

I can still reproduce it with the latest staging.

Kapture.2024-04-30.at.10.11.26.mp4

wildan-m · 2024-04-30T08:36:04Z

@Beamanator @jjcoffee, based on this, will this issue be closed or will it continue here?

abekkala · 2024-04-30T19:18:07Z

@lanitochka17, we're going to close this one out as it has really changed a bit over time. Can you create a new issue that shows the updated details and the updated flow? Noting what's wrong and what is expected?

Thanks!

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

melvin-bot bot assigned abekkala Mar 5, 2024

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

melvin-bot bot changed the title ~~Expense - LHN does not show expense preview when there is a thread with deleted root message~~ [$500] Expense - LHN does not show expense preview when there is a thread with deleted root message Mar 6, 2024

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

melvin-bot bot assigned jjcoffee Mar 6, 2024

abekkala added Help Wanted Apply this label when an issue is open to proposals by contributors and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Mar 6, 2024

wildan-m mentioned this issue Mar 8, 2024

[$500] Money Request grouped message jumping after adding one more Money Request #37245

Closed

6 tasks

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

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

melvin-bot bot added the Overdue label Apr 5, 2024

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

abekkala unassigned bfitzexpensify Apr 8, 2024

melvin-bot bot added the Overdue label Apr 9, 2024

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

melvin-bot bot added the Overdue label Apr 15, 2024

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

melvin-bot bot added the Overdue label Apr 19, 2024

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

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

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

abekkala closed this as completed Apr 30, 2024

[$500] Expense - LHN does not show expense preview when there is a thread with deleted root message #37775

[$500] Expense - LHN does not show expense preview when there is a thread with deleted root message #37775

Comments

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

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 5, 2024

lanitochka17 commented Mar 5, 2024

lanitochka17 commented Mar 5, 2024

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

melvin-bot bot commented Mar 6, 2024

melvin-bot bot commented Mar 6, 2024

wildan-m commented Mar 7, 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)

jjcoffee commented Mar 7, 2024

wildan-m commented Mar 7, 2024

jjcoffee commented Mar 11, 2024 • edited Loading

jjcoffee commented Mar 11, 2024

nkdengineer commented Mar 11, 2024

jjcoffee commented Mar 11, 2024 • edited Loading

nkdengineer commented Mar 11, 2024 • edited Loading

andrey010 commented Mar 11, 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)

jjcoffee commented Mar 13, 2024

andrey010 commented Mar 13, 2024

jjcoffee commented Mar 13, 2024

bfitzexpensify commented Apr 7, 2024

melvin-bot bot commented Apr 10, 2024

melvin-bot bot commented Apr 10, 2024

Beamanator commented Apr 11, 2024

mvtglobally commented Apr 12, 2024

melvin-bot bot commented Apr 16, 2024

jjcoffee commented Apr 17, 2024

melvin-bot bot commented Apr 17, 2024

mvtglobally commented Apr 18, 2024

jjcoffee commented Apr 22, 2024

abekkala commented Apr 22, 2024

jjcoffee commented Apr 23, 2024

melvin-bot bot commented Apr 24, 2024

jjcoffee commented Apr 25, 2024

Beamanator commented Apr 29, 2024

wildan-m commented Apr 30, 2024

wildan-m commented Apr 30, 2024

abekkala commented Apr 30, 2024

lanitochka17 commented Mar 5, 2024 •

edited by melvin-bot bot

Loading

nkdengineer commented Mar 6, 2024 •

edited

Loading

wildan-m commented Mar 7, 2024 •

edited

Loading

jjcoffee commented Mar 11, 2024 •

edited

Loading

jjcoffee commented Mar 11, 2024 •

edited

Loading

nkdengineer commented Mar 11, 2024 •

edited

Loading