[HOLD for payment 2024-09-18] [HOLD for payment 2024-09-17][Search v2.2] Context menu is available for attachment which leads to crash and invalid link

[IuliiaHerets]

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.31-5
Reproducible in staging?: Y
Reproducible in production?: N/A
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Launch New Expensify app.
2. Go to Search.
3. Go to Chats.
4. Long press on message with attachment.
5. Tap Copy to clipboard.
6. Note that app crashes.
7. Repeat Step 1 to 4.
8. Tap Copy link.
9. Paste the link elsewhere.
10. Note that the copied link is https://staging.new.expensify.com/r/-1/undefined

Expected Result:

In Step 6, app should not crash and Copy to clipboard option should not appear if it is not working.
In Step 10, the copied link should be a valid report link.

Actual Result:

In Step 6, app crashes after tapping Copy to clipboard.
In Step 10, the copied link is https://staging.new.expensify.com/r/-1/undefined

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

1009.txt

Bug6598019_1725916389199.ScreenRecording_09-10-2024_05-07-27_1.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @CortneyOfstad

[melvin-bot]

Triggered auto assignment to @nkuoch (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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

[github-actions]

👋 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.

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-09-10 14:24:05 UTC.

Proposal

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

In Step 6, app crashes after tapping Copy to clipboard.
In Step 10, the copied link is https://staging.new.expensify.com/r/-1/undefined

What is the root cause of that problem?

That ChatListItem item doesn't have ShowContextMenuContext.Provider then when we click on an image, it displays the wrong context menu actions. Then when we click on Copy To Clipboard the app crashes and and the copied link is also wrong because reportAction and report are undefined.

App/src/components/SelectionList/ChatListItem.tsx

Line 69 in 1725041

<AttachmentContext.Provider value={attachmentContextValue}>

App/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx

Line 82 in 1725041

<ShowContextMenuContext.Consumer>

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

We should add ShowContextMenuContext.Provide with value is the necessary data that we have base on the snapshot data

const contextValue = useMemo(
    () => ({
        report: {reportID: reportActionItem.reportID},
        reportNameValuePairs: undefined,
        action: {reportActionID: reportActionItem.reportActionID, message: reportActionItem.message, actionName: reportActionItem.actionName},
        transactionThreadReport: null,
        checkIfContextMenuActive: () => {},
    }),
    [reportActionItem]
);

<ShowContextMenuContext.Provider value={contextValue}>

What alternative solutions did you explore? (Optional)

If we want do disable the context menu in search page, we can prevent calling showContextMenuForReport if action is empty

App/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx

Line 97 in 286b945

showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

App/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx

Line 57 in 286b945

showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

App/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.tsx

Line 90 in 286b945

showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

Line 43 in 286b945

showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

App/src/components/VideoPlayerPreview/VideoPlayerThumbnail.tsx

Line 52 in a63daff

showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

Result

[luacmartins]

We need to disable the context menu in ChatListItem, since those messages don't support actions.

[nkdengineer]

Updated proposal to disable context menu in search page.

[luacmartins]

cc @shubham1206agra

[luacmartins]

@nkdengineer your alternate proposal looks good. Are you available to work on the PR?

[nkdengineer]

sure.

[nkdengineer]

@luacmartins Please assign me then I can raise the PR.

[shubham1206agra]

Can we just insert a value in Context to tell if the showContextMenu is disabled or not?

[nkdengineer]

@shubham1206agra If we insert a value in Context we still need to use this in onLongPress function to return early so I think we can use the value of action to check in this case.

[luacmartins]

@shubham1206agra will work on this since it's a regression from his PR.

[luacmartins]

@CortneyOfstad this was a regression from #47690, but we should still pay @akinwale for the review

[luacmartins]

This is fixed on staging. Although the iOS app still has a haptic feedback, which is kinda weird since it doesn't do anything.

[luacmartins]

hmm the PR was still not CPed to staging 🤔 how can I not reproduce this anymore?

[luacmartins]

PR was CPed and deployed to production yesterday.

[melvin-bot]

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

• [CP Staging] Fix showcontext menu on search page #48901

If no regressions arise, payment will be issued on 2024-09-18. 🎊

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

• @akinwale requires payment (Needs manual offer from BZ)
• @shubham1206agra requires payment (Needs manual offer from BZ)

[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:

• [@akinwale / @shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale / @shubham1206agra] 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:
• [@akinwale / @shubham1206agra] 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:
• [@akinwale / @shubham1206agra] Determine if we should create a regression test for this bug.
• [@akinwale / @shubham1206agra] 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.
• [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[shubham1206agra]

Only @akinwale is eligible for payment

[CortneyOfstad]

@akinwale sent you a proposal in Upwork here, please let me know once you accept and I will get that paid ASAP. Thanks!

[akinwale]

@akinwale sent you a proposal in Upwork here, please let me know once you accept and I will get that paid ASAP. Thanks!

@CortneyOfstad Accepted. Thanks!

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @akinwale paid $(AMOUNT) via Upwork (LINK)
• ROLE: @shubham1206agra paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@CortneyOfstad)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[CortneyOfstad]

Payment Summary

@akinwale — paid $250 via Upwork
@shubham1206agra — not paid based on comment here