[$250] Thread - On deleting parent message, sending thread message directed to parent conversation page

[IuliiaHerets]

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: v9.0.64-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Issue reported by: Applause Internal Team

Action Performed:

1. Launch app
2. Open a chat
3. Send a message
4. Create a reply thread for the message sent
5. Send few reply messages
6. Note user is in same reply thread page
7. Tap sub header link and navigate to parent conversation
8. Delete the parent message
9. Tap reply
10. Add a reply message in thread page

Expected Result:

On deleting parent message, sending thread message must not be directed to parent conversation page.

Actual Result:

On deleting parent message, sending thread message directed to parent conversation page.

Workaround:

Unknown

Platforms:

• [x] Android: Standalone
• [x] Android: HybridApp
• [x] Android: mWeb Chrome
• [ ] iOS: Standalone
• [ ] iOS: HybridApp
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/629b0c4d-88ae-4e9c-b854-bbf0d85a73e8

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021861170835174996793
• Upwork Job ID: 1861170835174996793
• Last Price Increase: 2024-11-25

Issue Owner

Current Issue Owner: @suneox

[melvin-bot[bot]]

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

[jacobkim9881]

Edited by proposal-police: This proposal was edited at 2024-11-25 05:22:26 UTC.

Proposal

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

When enter a message in deleted thread, it navigates to parent report.

What is the root cause of that problem?

From BE replacing plan isn't finished which changes originalMessage to message at AddComment API.

This is a part of addComment response:

childVisibleActionCount: 18
originalMessage : 
{
    "deleted": "2024-11-24 09:47:00.125",
    "edits": [
        "j"
    ],
    "html": "",
    "isDeletedParentAction": true,
    "isNewDot": true,
    "lastModified": "2024-11-24 09:47:00.125"
}

https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/pages/home/ReportScreen.tsx#L131-L132

Here isDeletedParentAction checks if parent action is deleted.

https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/pages/home/ReportScreen.tsx#L564

And here this condition is checks if it shows ghost transaction screen.

https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/pages/home/ReportScreen.tsx#L578

In this issue if isDeletedParentAction is false then it goes to parent report in a short for the code above. In the issue isDeletedParentAction should be true not to go parent report.

https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/libs/ReportActionsUtils.ts#L142-L144

This function decide isDeletedParentAction and it uses getReportActionMessage.

https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/libs/ReportActionsUtils.ts#L138-L140

Here it checks only in message object. However in this issue isDeletedParentAction object is in originalMessage. If this function checks originalMessage too then the issue will be solved but we decided not to use originalMessage.

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

We should change originalMessage into message object of addComment's response from BE side.

What alternative solutions did you explore? (Optional)

As an alternative, we can edit isDeletedParentAction as a temporary. In this function only message report is allowed but we should allow original message object too.

function isDeletedParentAction(reportAction: OnyxInputOrEntry<ReportAction>): boolean {
  const tempIsDeletedParentAction = reportAction?.originalMessage?.isDeletedParentAction; 
  return (getReportActionMessage(reportAction)?.isDeletedParentAction ?? tempIsDeletedParentAction ?? : false) && (reportAction?.childVisibleActionCount ?? 0) > 0;
  }

Plus, whenever entering any message in the deleted thread, this condition at getReportName is checked :

https://github.com/Expensify/App/blob/03639a9a0fc6f7adc77ffbdff10aed9b48e40658/src/libs/ReportUtils.ts#L3986-L3988

of here :

https://github.com/Expensify/App/blob/03639a9a0fc6f7adc77ffbdff10aed9b48e40658/src/pages/home/HeaderView.tsx#L97

So we should fix condition as a temporary to load header title without infinite loading :

if (ReportActionsUtils.isDeletedParentAction(parentReportAction)) {
  return Localize.translateLocal('parentReportAction.deletedMessage'); 
}

Then when enter a message in deleted thread, then it will not navigate to parent report. The fix code above can be refactored.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[suneox]

@jacobkim9881 Thank you for your proposal, but your alternative solution will cause the header to show an infinite loading state. Could you please take another look at this issue?

https://github.com/user-attachments/assets/70643d70-d8f6-43d7-b560-9161c00a54d0

@puneetlath It seems that the BE has updated the response to exclude the isDeletedParentAction field on the AddComment action.

AddComment	OpenReport

However, when we OpenReport, the isDeletedParentAction field is included in the response. I’d like to confirm if the absence of the isDeletedParentAction field in the AddComment response is expected behavior. If it's expected behavior on BE side, we can proceed with the required updates on the client side.

[ChanakaWithanage]

Edited by proposal-police: This proposal was edited at 2024-11-29 03:50:47 UTC.

Proposal

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

When a parent message in a chat thread is deleted, sending a reply message in the thread page incorrectly navigates the user back to the parent conversation page.

What is the root cause of that problem?

The navigation logic does not properly differentiate between actions related to the deletion of messages in a thread and the addition of new messages. As a result, any action in the thread after the parent message is deleted triggers navigation back to the parent conversation page.

Code causing the navigation back: https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/pages/home/ReportScreen.tsx#L558-L566

Specific Navigation line: https://github.com/Expensify/App/blob/f4f8da74c100f2c5ddb666ff4314038010104cbf/src/pages/home/ReportScreen.tsx#L581

Based on the commit, this logic was introduced to address the behavior: "Navigate back to parent report when the last comment of a deleted thread is deleted."

However, the current implementation also navigates back when a new reply is added to a deleted thread, which is unintended.

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

We should refine the navigation logic to ensure that it only triggers when the last comment of a deleted thread is deleted, and not when a reply is added.

To fix this issue while preserving the original feature, we can add a check to ensure that the navigation logic does not trigger if the mostRecentReportAction is not a deleted action.

`const mostRecentReportAction = reportActions.at(0);
 const isDeletedAction = ReportActionsUtils.isDeletedAction(mostRecentReportAction);`
 
 `if (ReportUtils.isChatThread(report) && !isDeletedAction) {
          return;
  }
  Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(prevReport.parentReportID));`

This solution minimizes changes to the existing codebase while maintaining the behavior introduced by the original commit.

https://github.com/user-attachments/assets/ba89be42-1a96-49d5-9a8d-f54259202cde

[melvin-bot[bot]]

📣 @ChanakaWithanage! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[ChanakaWithanage]

Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~014d3e5915a582d1e6

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[suneox]

@ChanakaWithanage Welcome to Expensify! First, please take some time to read through the guidelines on how to contribute to this project. This will help you understand the process and expectations for contributing effectively.

[jacobkim9881]

@suneox Thank you for reviewing. I found the reason for infinite loading on header. It is that ReportUtils.getReportName can't find any report name to return. In this case isDeletedParentAction is not found here :

https://github.com/Expensify/App/blob/03639a9a0fc6f7adc77ffbdff10aed9b48e40658/src/libs/ReportUtils.ts#L3986-L3988

So I have updated my proposal.

[jacobkim9881]

Proposal

updated

[ChanakaWithanage]

@suneox Thank you! I have reviewed the guidelines and updated my proposal with more specifics about the solution. Please find the updated version here: Updated Proposal.

[melvin-bot[bot]]

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

[suneox]

It seems that the BE has updated the response to exclude the isDeletedParentAction field on the AddComment action.

We’re still waiting for confirmation from the internal team to determine whether this change is the expected behavior on the backend side. If the current response is expected, we can proceed with the frontend fix using @jacobkim9881 proposal.

[!NOTE]
The typecheck error can be addressed during the PR review process.

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@puneetlath @suneox this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@puneetlath, @suneox Still overdue 6 days?! Let's take care of this!

[suneox]

We’re still waiting for the internal team to make a decision.

[puneetlath]

However, when we OpenReport, the isDeletedParentAction field is included in the response. I’d like to confirm if the absence of the isDeletedParentAction field in the AddComment response is expected behavior. If it's expected behavior on BE side, we can proceed with the required updates on the client side.

Hm, I'm not sure if it's expected but we can have the back-end do whatever would make the most sense here. Would it make sense for AddComment to return this?

[melvin-bot[bot]]

@puneetlath, @suneox Huh... This is 4 days overdue. Who can take care of this?

[suneox]

Hm, I'm not sure if it's expected but we can have the back-end do whatever would make the most sense here. Would it make sense for AddComment to return this?

The AddComment action only includes the isDeletedParentAction value in the originalMessage, not in the message field. If it’s feasible for the backend to add isDeletedParentAction to the message field as well, I believe this should be handled on the backend side for consistency and to reduce complexity on the client side.

[melvin-bot[bot]]

@puneetlath @suneox this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[suneox]

Not overdue. We're still waiting for the internal team to make a decision. If we're unsure about BE, we can handle it on the client side instead.

[jacobkim9881]

@suneox I have found alternative solution. Since this issue, we can check if the parent report is deleted with ReportActionsUtils.isThreadParentMessage function:

https://github.com/Expensify/App/blob/918488aa5586be249f392b26692572aaba010f31/src/libs/actions/Report.ts#L1535

[melvin-bot[bot]]

@puneetlath, @suneox Huh... This is 4 days overdue. Who can take care of this?