[HOLD for payment 2024-01-24] [$500] Attachment - Attachment download option not available in offline but receipt download available

[kbecciv]

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.21-1
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:

1. Launch app
2. Tap on a chat with attachments
3. Tap on a attachment
4. Note 3 dots on top with download option displayed
5. Navigate to LHN
6. Tap profile icon
7. Tap preferences-- toggle on forced offline
8. Tap on a chat with attachments
9. Tap on a attachment
10. In offline, note 3dots on top with download option not displayed
11. Navigate to LHN
12. Tap fab --- request money --- scan
13. Upload a image
14. Select a contact
15. Tap request
16. Tap on scan request created
17. Tap on receipt
18. In offline, note 3 dots on top with download option displayed

Expected Result:

In offline, download option must be available or not available but behaviour must be consistent throughout the app.

Actual Result:

Attachment download option not available in offline but receipt download available and there is inconsistency in behaviour.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6331988_1704361873327.67.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010f6117214034f596
• Upwork Job ID: 1742850933425078272
• Last Price Increase: 2024-01-04
• Automatic offers:
• alitoshmatov | Reviewer | 28088854
• abzokhattab | Contributor | 28088855

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[abzokhattab]

Proposal

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

Download option is visible in offline mode inisde the receipt attachment

What is the root cause of that problem?

the download option inside the threeDotMenu is always pushed regardless of whether the network is offline or not:

App/src/components/AttachmentModal.js

Lines 383 to 387 in 9f5c3ec

	menuItems.push({
	icon: Expensicons.Download,
	text: props.translate('common.download'),
	onSelected: () => downloadAttachment(source),
	});

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

we should add an if condition that the network is !isOffline

     if (!isOffline) {
            menuItems.push({
                icon: Expensicons.Download,
                text: props.translate('common.download'),
                onSelected: () => downloadAttachment(source),
            });
        }

Alternatively

OR instead, we can already use the value of shouldShowDownloadButton in the if condition, this var is used to decide whether the download button should be shown in the header or not .. in this case its used in the chat attachment

.. so to preserve consistency we can use it with threedotmenu download option as well :

in order to use shouldShowDownloadButton we need to move this code block above so that the variable is defined before the threeDotsMenuItems function

then use shouldShowDownloadButton as follows

if (shouldShowDownloadButton){
     menuItems.push({
                icon: Expensicons.Download,
                text: props.translate('common.download'),
                onSelected: () => downloadAttachment(source),
            });
}

[jliexpensify]

Pinged Dylan here to add to Wave 5 - https://expensify.slack.com/archives/C05DWUDHVK7/p1704411372335989

[dukenv0307]

Proposal

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

Attachment download option not available in offline but receipt download available and there is inconsistency in behaviour.

What is the root cause of that problem?

We always add the download option for receipt image.

App/src/components/AttachmentModal.js

Line 391 in 6fad557

menuItems.push({

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

We should check if the network is offline and the receipt is a local file or not. We don't need to check this for attachment because when we click on this attachment to open preview, it's already uploaded.

if (!isOffline && !props.isAuthTokenRequired) {
    menuItems.push({
        icon: Expensicons.Download,
        text: props.translate('common.download'),
        onSelected: () => downloadAttachment(source),
    });
}

What alternative solutions did you explore? (Optional)

NA

[jliexpensify]

@alitoshmatov bump for reviews please! Essentially, if it's possible to be downloading something whilst offline, we always want to be showing the download icon. Thanks!

[alitoshmatov]

@dukenv0307 Can you explain how would you check if file is available locally?

[dukenv0307]

props.isAuthTokenRequired

@alitoshmatov I mentioned it in the code change of my proposal, props.isAuthTokenRequired can help us to know whether the file now is a local file or not. If it's false the file now is the local file.

Or we can use the same logic as we do here

App/src/libs/ReceiptUtils.ts

Line 69 in 463b7bc

const isLocalFile = typeof path === 'number' || path.startsWith('blob:') || path.startsWith('file:') || path.startsWith('/');

[alitoshmatov]

@dukenv0307 They are not working. We also have a case where if receipt is uploaded in offline mode it is downloadable but after refreshing the page the file is not downloadable and it is redirecting to invalid file url.

I think the best approach here is to not to show download button when in offline.

Screen.Recording.2024-01-08.at.17.25.28.mov

[alitoshmatov]

We can go with @abzokhattab 's proposal

We should not show download option when network is offline.
@abzokhattab also not that when three dot menu is empty we don't want to show it to prevent empty menu:

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[dukenv0307]

@alitoshmatov Actually, we're using a preference force offline for dev testing. In production, if we're offline we cannot reload the App so this case is invalid in production.

[abzokhattab]

@abzokhattab also not that when three dot menu is empty we don't want to show it to prevent empty menu:

Okay will handle this condition in the PR

[melvin-bot]

📣 @alitoshmatov 🎉 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]

📣 @abzokhattab 🎉 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 📖

[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.4.25-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Hide download receipt button when offline #34118

If no regressions arise, payment will be issued on 2024-01-24. 🎊

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

• @abzokhattab requires payment automatic offer (Contributor)
• @alitoshmatov requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@alitoshmatov] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Bump @alitoshmatov to complete the checklist please!

Payment Summary

• Contributor: @abzokhattab $500
• C+: @alitoshmatov $500

Upwork job

[alitoshmatov]

• The PR that introduced the bug has been identified. Link to the PR: Hide replace option for the request that cannot edit #28373
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/28373/files#r1464781176
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/a
• Determine if we should create a regression test for this bug. No need, it is very simple logic. It is pretty hard to break it

[jliexpensify]

Paid and job closed!