search

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.56k stars 2.9k forks source link

[$250] clicking on a chat row in a thread within a thread leads to "not found" page. #47892

Closed m-natarajan closed 2 months ago

m-natarajan commented 2 months 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.24-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: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724336539348799

Action Performed:

  1. Initiate a chat
  2. Reply in the thread
  3. Reply in the thread of thread
  4. Click on the chat row in a thread within the thread

    Expected Result:

    Able to open the chat

    Actual Result:

    Leads to "not found" page

    Workaround:

    unknown

    Platforms:

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

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

Screenshots/Videos

https://github.com/user-attachments/assets/a4d75568-9067-48a5-bd44-9e6f5f69474d

https://github.com/user-attachments/assets/a030bd11-4635-4ab8-b29a-d6f2a1eccd76

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b2e4f856351af3b6
  • Upwork Job ID: 1828188239764165076
  • Last Price Increase: 2024-08-26
  • Automatic offers:
    • brunovjk | Reviewer | 103719086
Issue OwnerCurrent Issue Owner: @brunovjk
melvin-bot[bot] commented 2 months ago

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

daledah commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-08-23 03:21:15 UTC.

Proposal

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

  1. Press a thread ancestor leads to "not found" page
  2. The first thread ancestor is not pressable

What is the root cause of that problem?

We're using parentReportID from these ancestors:

https://github.com/Expensify/App/blob/caab39bececcb7533afd77d3e000c6aa0c9dabc9/src/pages/home/report/ReportActionItemParentAction.tsx#L128-L141

which leads us up to 1 level higher.

Because the report data in these ancestors already points to its parent report:

https://github.com/Expensify/App/blob/ed910fa1cae6597ea317f24bd7d4d60e4f6f4d5e/src/libs/ReportUtils.ts#L7175

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

Use reportID instead of parentReportID.

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

brunovjk commented 2 months ago

It seems to me that those parentReportID should be replaced by reportID here, but the part pointed out in the proposal is missing. I tested the suggested changes and it solved the issue just fine. However, I wonder if there are other places we should check to update. What do you think @daledah? Either way, the proposal looks good to me.

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 2 months ago

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

raza-ak commented 2 months ago

Proposal

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

Clicking on a chat row in a thread within a thread leads to "not found" page.

What is the root cause of that problem?

The issue occurs when attempting to navigate to a report that doesn't exist on the backend, resulting in an error response from the OpenReport API call.

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

The navigation logic has been updated to ensure that, under certain conditions, it navigates back to the parent chat instead of trying to access a non-existent report. Following changes in the https://github.com/Expensify/App/blob/main/src/pages/home/report/ReportActionItemParentAction.tsx#L134 will fix the issue.

Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.parentReportID ? ancestor.report.parentReportID : ancestor.reportAction.reportActionID));

image

https://github.com/user-attachments/assets/4105d15a-83bf-4fdc-b159-b3bb2f8cf3b8

raza-ak commented 2 months ago

@brunovjk Please review my proposal.

brunovjk commented 2 months ago

Hi @raza-ak, Thank you for your proposal. But I believe the first proposal effectively resolves the problem and aligns better with the existing codebase. It seems reasonable to support it and move forward with that solution. However, @arosiclair will have the final say on this matter. Let’s wait for their review.

arosiclair commented 2 months ago

@daledah's proposal looks good and tests well on my end 👍

melvin-bot[bot] commented 2 months ago

📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 months ago

📣 @daledah You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

brunovjk commented 2 months ago

[!NOTE]
The production deploy automation failed: This should be on [HOLD for Payment 2024-09-09] according to https://github.com/Expensify/App/issues/48360 prod deploy checklist, confirmed in https://github.com/Expensify/App/pull/48232#issuecomment-2325378483.

cc: @kevinksullivan

brunovjk commented 2 months ago

Regression Test Proposal

kevinksullivan commented 2 months ago

Paid @brunovjk . @daledah , can you apply for the upwork job and let us know once you've done so here?

kevinksullivan commented 2 months ago

Looping in another BZ member for final payment to @daledah as I'm going OOO , otherwise this is all set!

melvin-bot[bot] commented 2 months ago

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

Christinadobrzyn commented 2 months ago

@daledah here's an Upwork offer - https://www.upwork.com/nx/wm/offer/103950508 - can you accept this so we can pay you?

daledah commented 2 months ago

@Christinadobrzyn I did, thanks

Christinadobrzyn commented 2 months ago

Awesome! Thanks @daledah I've paid you in Upwork.

Payment summary just in case it's helpful.

Payouts due:

Regression test here - https://github.com/Expensify/App/issues/49133 Closing this as complete!