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.48k stars 2.83k forks source link

[$250] The "Not Found" page does not display when we press on the deleted message link. #50120

Open m-natarajan opened 2 weeks ago

m-natarajan commented 2 weeks 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.43-4 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: @jayeshmangwani Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727868246323429

Action Performed:

  1. Go to any chat.
  2. Long-press on any message that you have sent and select "Copy Link."
  3. Send that link in the chat.
  4. Delete the message for which you selected "Copy Link."
  5. Press on the link in the chat.
  6. The user scrolls in the chat, but the "Not Found" page does not appear.
  7. Now press any pressable on the page, e.g., Emoji.
  8. Now, we can see the "Not Found" page after pressing the button.

Expected Result:

The "Not Found" page should be visible immediately if the message is deleted.

Actual Result:

The "Not Found" page does not display when we press on the deleted message link.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/7fa3489a-b512-41c5-a4ea-f62834cf1ba9

https://github.com/user-attachments/assets/c4104637-fe09-4179-95a9-89dd182f9c16

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021841649844209879395
  • Upwork Job ID: 1841649844209879395
  • Last Price Increase: 2024-10-03
  • Automatic offers:
    • rayane-djouah | Reviewer | 104343030
    • Nodebrute | Contributor | 104343031
Issue OwnerCurrent Issue Owner: @rayane-djouah
melvin-bot[bot] commented 2 weeks ago

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

greg-schroeder commented 2 weeks ago

Typically I'd put this in #vip-vsb but that project is closed/paused. It sounds like this kind of chat bugs falls under #newdot-quality now per DBs comment here.

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

Nodebrute commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-10-03 03:17:43 UTC.

Proposal

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

The "Not Found" page does not display when we press on the deleted message link.

What is the root cause of that problem?

We use this check to show not found page. It was working fine until in this PR we added && !!prevIsLinkedActionDeleted) https://github.com/Expensify/App/blob/e02df29fc8ff05da2a242775fe972cf2bc5b817a/src/pages/home/ReportScreen.tsx#L363-L371

But this was also added to fix

But the user will notice a brief blink of the not found page because we clear the reportActionID not immediately and to prevent that, we can update the not found condition to only show if both the previous prevIsLinkedActionDeleted and isLinkedActionDeleted are true.

In this PR we added an effect to remove reportActionID from params

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

We can move the below code above this https://github.com/Expensify/App/blob/e02df29fc8ff05da2a242775fe972cf2bc5b817a/src/pages/home/ReportScreen.tsx#L664

And then we can use isLinkedActionBecomesDeleted here instead of prevIsLinkedActionDeleted. We will not show not found page when it's previously deleted. https://github.com/Expensify/App/blob/e02df29fc8ff05da2a242775fe972cf2bc5b817a/src/pages/home/ReportScreen.tsx#L364

pseudocode

        (!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted && !isLinkedActionBecomesDeleted) 

https://github.com/user-attachments/assets/f44e18f2-acfa-4eb1-95d8-a78df188fe2f

And the blinking issue remains fixed

https://github.com/user-attachments/assets/e4445cea-89ed-4c56-a371-4132c75a6a40

What alternative solutions did you explore? (Optional)

huult commented 2 weeks ago

Proposal

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

he "Not Found" page does not display when we press on the deleted message link.

What is the root cause of that problem?

prevIsLinkedActionDeleted returns undefined because the previous value is undefined. As a result, the "Not found" page cannot be displayed due to this condition https://github.com/Expensify/App/blob/f83b69e45451946c7505b773fb6e085fb2fe66c0/src/pages/home/ReportScreen.tsx#L364

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

To resolve this issue, we simply trigger an update to the value of prevIsLinkedActionDeleted.

// .src/pages/home/ReportScreen.tsx#L666
useEffect(() => {
        // If the linked action is previously available but now deleted,
        // remove the reportActionID from the params to not link to the deleted action.
        const isLinkedActionBecomesDeleted = prevIsLinkedActionDeleted !== undefined && !prevIsLinkedActionDeleted && isLinkedActionDeleted;
        if (!isLinkedActionBecomesDeleted) {
+           if (!isLinkedActionInaccessibleWhisper && isLinkedActionDeleted) {
+                fetchReportIfNeeded();
+            }
            return;
        }
        Navigation.setParams({reportActionID: ''});
-    }, [prevIsLinkedActionDeleted, isLinkedActionDeleted]);
+   }, [prevIsLinkedActionDeleted, isLinkedActionDeleted, isLinkedActionInaccessibleWhisper, fetchReportIfNeeded]);
POC https://github.com/user-attachments/assets/b89886fe-73a8-4c54-8cb7-26e29b0da459
Nodebrute commented 1 week ago

@rayane-djouah friendly bump for review.

rayane-djouah commented 1 week ago

Thanks for the reminder. Will review in my morning

rayane-djouah commented 1 week ago

@Nodebrute's proposal looks good to me

:ribbon::eyes::ribbon: C+ reviewed

melvin-bot[bot] commented 1 week ago

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

Nodebrute commented 1 week ago

@aldo-expensify bump for assignment.

melvin-bot[bot] commented 1 week ago

πŸ“£ @rayane-djouah πŸŽ‰ 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 1 week ago

πŸ“£ @Nodebrute πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

greg-schroeder commented 3 days ago

PR is up, looks like waiting on @aldo-expensify to review next!

aldo-expensify commented 2 days ago

Merged