[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [$1000] Chat - Accessing an attachment via link that is not found shows the error page twice

[lanitochka17]

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

---

Issue found when executing PR #25516

Action Performed:

1. Open app New Expensify
2. Log in
3. Open any chat/report
4. Send this link https://staging.new.expensify.com/r/7736083827006365/attachment?source=/chat-attachments/8443634480793859480/w_3de27a40d82c4442386d0447d8dab8af403f786f3.png
5. Tap on the link
6. Tap on the back button

Expected Result:

The user should be taken to the chat and stay there

Actual Result:

The error page opens again, so that the user has to close it again

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.56.18

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6174639_Xefe3282_1_.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016d69185d9da05b9b
• Upwork Job ID: 1694891980872962048
• Last Price Increase: 2023-09-15
• Automatic offers:
• tienifr | Contributor | 27121583

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @joelbettner (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[jeet-dhandha]

Proposal

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

• Hmm...its not here - page is coming up twice for an attachment which we do not have access to.

What is the root cause of that problem?

• The page is coming up first time when we are trying to access the attachment.
• Second time its coming up when we press back button on attachment modal. As its trying to navigate to a reportID which we do not have access to and thus showing Hmm...its not here page.

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

• We can update the dismissModal function of Navigation.js to check if any report is available for given targetReportID.

• And also we will have to remove isLoadingReportActions key from the found report as it will set key for unknown report in onyx.

• Finally, After changes if we press on back it should just close the Attachment Modal.

What alternative solutions did you explore? (Optional)

• N/A

[marcaaron]

@lanitochka17 What platforms is this related to? I can't reproduce on web. Please provide clear reproduction steps.

I don't think this needs to be a blocker. Getting linked to an attachment that doesn't exist seems like a minor flow.

[jeet-dhandha]

@marcaaron Try these steps:

1. Open the “Android” app.
2. Send the above link in one of your chat.
3. Press on the link and after that verify the not found page in attachment modal.
4. Press back on top and verify the not found page displaying second time.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @MitchExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[tienifr]

Proposal

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

Chat - Accessing an attachment via link that is not found shows the error page twice

What is the root cause of that problem?

When users hide the modal, we will dismiss the modal and open the report by getting the reportID of attachment url

App/src/pages/home/report/ReportAttachments.js

Line 33 in 34d4e54

onModalHide={() => Navigation.dismissModal(reportID)}

That why we see the Hm... twice (not found attachment and not found report)

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

The reason we pass reportID in Navigation.dismissModal is to open the corresponding report when the modal is hide.

But it is just useful if we open attachment from deeplink, because if we open it from report, we just need to dismiss the modal

When users open attachment from deeplink

• On web/desktop: we prefetched the report so after the attachment modal is closed, users can go to the corresponding report without passing reportID to Navigation.dismissModal (after this PR is merged)
• On mweb/native: We need to use reportID to navigate to correct report

Here is my suggestion change

            onModalHide={() => Navigation.dismissModal(props.isSmallScreenWidth && !Navigation.getTopmostReportId() ? reportID: undefined)}

What alternative solutions did you explore? (Optional)

We can simply remove reportID in

App/src/pages/home/report/ReportAttachments.js

Line 33 in 34d4e54

onModalHide={() => Navigation.dismissModal(reportID)}

[sobitneupane]

I will review the proposals today.

[sobitneupane]

@joelbettner @MitchExpensify

We see two error pages here. One is for attachment and one is for the report. We first show the error page for attachment and after user dismisses the modal, user navigates to the report (after split of seconds on original report).

What is the expected output here? IMO, user should be first navigated to the report and to the attachment (In this case, two error pages will be shown) . Or, if user doesn't have access to the report, attachment modal won't be shown at all (In this case, only one error page will be shown).

[tienifr]

I think when users open attachment modal from deeplink, then close it, we should display same chat in which attachment is located. Otherwise, we shouldn't navigate to {reportID} chat, just close it.

[melvin-bot]

@joelbettner, @sobitneupane, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @tienifr got assigned: 2023-10-10 14:48:59 Z
• when the PR got merged: 2023-10-13 20:00:14 UTC
• days elapsed: 3

On to the next one 🚀

[tienifr]

@joelbettner Is it eligible for 50% bonus? As I and @sobitneupane completed work very early and the total time is just 3 days and 6 hours ~ 3 days

[melvin-bot]

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

[melvin-bot]

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

• fix: 25779 Accessing an attachment via link that is not found shows the error page twice #29206

If no regressions arise, payment will be issued on 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @sobitneupane does not require payment (Eligible for Manual Requests)
• @tienifr requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

• fix: 25779 Accessing an attachment via link that is not found shows the error page twice #29206

If no regressions arise, payment will be issued on 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @sobitneupane does not require payment (Eligible for Manual Requests)
• @tienifr requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MitchExpensify]

I think the bonus is fair if all that delayed us was internal review - Do you agree @joelbettner ?

[joelbettner]

@MitchExpensify I agree.

[MitchExpensify]

Payment summary:

• $1500 @sobitneupane does not require payment (Eligible for Manual Requests)
• $1500 @tienifr requires payment offer (Contributor)

[sobitneupane]

Requested payment on newDot.

#25779 (comment)

[MitchExpensify]

Paid @tienifr!

[JmillsExpensify]

$1,500 payment approved for @sobitneupane based on BZ summary.