[HOLD for Payment 2024-09-06] [$125] Action menu - Download option is missing from action menu

[izarutskaya]

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: v9.0.18-7
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4836901
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Log out of New Expensify
2. Navigate to a public room (https://staging.new.expensify.com/r/2091104345528462
   ) as an anonymous user
3. Right click on any attachment to access the action menu

Expected Result:

There should be an option to download the attachment.

Actual Result:

Option to download attachment is missing from action menu.

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

Bug6566483_1723193410070.Recording_2024-08-09_100836_3.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018493448a5422f7d5
• Upwork Job ID: 1821912903486929052
• Last Price Increase: 2024-08-09
• Automatic offers:
• brunovjk | Reviewer | 103540677
• Krishna2323 | Contributor | 103540678

Issue Owner

Current Issue Owner: @Krishna2323

[melvin-bot]

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

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-08-09 13:29:28 UTC.

Proposal

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

Action menu - Download option is missing from action menu

What is the root cause of that problem?

Download is included in restrictedReadOnlyActions.

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 596 to 602 in 9812479

	const restrictedReadOnlyActions: TranslationPaths[] = [
	'common.download',
	'reportActionContextMenu.replyInThread',
	'reportActionContextMenu.editAction',
	'reportActionContextMenu.joinThread',
	'reportActionContextMenu.deleteAction',
	];

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

Remove 'common.download' from restrictedReadOnlyActions.

What alternative solutions did you explore? (Optional)

If we want to hide the download option for anonymous users then we should also do the same in ReportAttachments and all other places which allows users to download attachments.

In ReportAttachments we can check if the download is in the restricted actions list and if so then we can hide the download button.

const disabledActions = useMemo(() => (!ReportUtils.canWriteInReport(report) ? RestrictedReadOnlyContextMenuActions : []), [report]);

allowDownload={!(disabledActions as ContextMenuActionWithIcon[])?.find((item) => item.textTranslateKey === 'common.download')}

// OR
allowDownload={ReportUtils.canWriteInReport(report)}

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

Line 37 in 9812479

allowDownload

The same needs to be done in BaseAnchorForAttachmentsOnly and other files.

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx

Line 66 in 88e7e8d

shouldShowDownloadIcon={!!sourceID && !isOffline}

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx

Lines 47 to 53 in 88e7e8d

	onPress={() => {
	if (isDownloading || isOffline || !sourceID) {
	return;
	}
	Download.setDownload(sourceID, true);
	fileDownload(sourceURLWithAuth, displayName, '', Browser.isMobileSafari()).then(() => Download.setDownload(sourceID, false));
	}}

[Krishna2323]

Proposal Updated

• Minor update in alternative section

[melvin-bot]

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

[melvin-bot]

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

[twisterdotcom]

Man that room takes a minute to load

[twisterdotcom]

Making this just $125 because this doesn't actually prevent the behaviour though (ie, you can still download from the big attachment modal) and public rooms are not well used yet,s o not a huge company priority.

[melvin-bot]

Upwork job price has been updated to $125

[brunovjk]

I tested the proposed solution, and removing common.download from the restricted actions list does fix the issue. However, I want to confirm the expected behavior. Should we remove download from all or implement a dynamic approach, as suggested in the alternative solution? Anyway I’m leaning towards @Krishna2323’s proposal but wanted to check in.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[Krishna2323]

@yuwenmemon, friendly bump #47149 (comment)

[yuwenmemon]

Ah, apologies, I was OOO - thanks for the ping! Assigned 🙂

[melvin-bot]

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

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

[Krishna2323]

I want to confirm the expected behavior. Should we remove download from all or implement a dynamic approach, as suggested in the alternative solution?

@yuwenmemon, can you please confirm the behaviour?

[brunovjk]

Hey @yuwenmemon :D have you had a chance to look at the alternative solution here? I believe the main solution will work fine, but I wanted to hear your thoughts. Thanks.

[twisterdotcom]

I do not think we should prevent downloads for anonymous users personally. If I want to download a picture of this cat, why not?

I agree though that we have some disparity between the menu options for an authenticated user and a restricted user.

Shouldn't we just show the download option to all users?

[yuwenmemon]

Agreed with @twisterdotcom

[Krishna2323]

Will raise the PR today.

[brunovjk]

Note

The production deploy automation failed: This should be on [HOLD for Payment 2024-09-06] according to #48035 prod deploy checklist, confirmed in https://expensify.slack.com/archives/C01GTK53T8Q/p1725051480370659?thread_ts=1725050431.962859&cid=C01GTK53T8Q.

cc: @twisterdotcom

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: The code that introduced the bug was present since the creation of the src/pages/home/report/ContextMenu/ContextMenuActions.tsx file. No specific PR was found that later modified this line. Link to file in creation commit

• [@brunovjk] 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: Since the bug was not introduced by a specific PR but has been present since the file's creation, there is no PR to comment on.

• [@brunovjk] 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: No discussion is necessary on Slack as this issue is isolated and was caused by a historical oversight rather than a review process flaw. The regression test should suffice to prevent recurrence.

• [@brunovjk] Determine if we should create a regression test for this bug. Yes

• [@brunovjk] 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.

Regression Test Proposal

• Log out of the app.
• Navigate to a public room as an anonymous user (https://staging.new.expensify.com/r/2091104345528462).
• Right-click on any attachment to access the action menu.
• Verify that the download option is visible in the action menu.
• Perform the same steps while offline and confirm the download option is still visible.

Do we agree 👍 or 👎

[twisterdotcom]

Payment Summary:

• @brunovjk paid $250 via Upwork here (C+)
• @Krishna2323 paid $250 via Upwork here (Contributor)