Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2024-11-07] [$250] Expense - Console error (app crashes on native) when copying system message to clipboard #51127

Open
5 of 8 tasks
IuliiaHerets opened this issue Oct 19, 2024 · 20 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 19, 2024

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

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Submit expense > Manual.
  3. Submit an expense to anyone.
  4. Go to transaction thread.
  5. Click on any field, edit and save it.
  6. Right click on the system message > Copy to clipboard.

Expected Result:

The content will be copied successfully.

Actual Result:

The content is not copied. Console error shows up.
On native app, app crashes.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

1910_1.txt

Bug6639462_1729326163995.20241019_161930.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021848817438340679533
  • Upwork Job ID: 1848817438340679533
  • Last Price Increase: 2024-10-22
Issue OwnerCurrent Issue Owner: @puneetlath
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 19, 2024
Copy link

melvin-bot bot commented Oct 19, 2024

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

@IuliiaHerets
Copy link
Author

@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@NJ-2020
Copy link
Contributor

NJ-2020 commented Oct 20, 2024

Edited by proposal-police: This proposal was edited at 2024-10-21 01:56:47 UTC.

Proposal

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

Expense - Console error (app crashes on native) when copying system message to clipboard

What is the root cause of that problem?

When we click the copy clipboard button, we invoke the onPress function

onPress: (closePopover, {reportAction, transaction, selection, reportID}) => {

The error comes from this if condition, we check whether the reportAction is from OD
} else if (ReportActionsUtils.isOldDotReportAction(reportAction)) {
const oldDotActionMessage = ReportActionsUtils.getMessageOfOldDotReportAction(reportAction);
Clipboard.setString(oldDotActionMessage);

The error come because the reportAction value is null while we're not using ? before the dot .
].some((oldDotActionName) => oldDotActionName === action.actionName);

The reportAction value null because we pass invalid reportID
const showPopover = useCallback(
(event: GestureResponderEvent | MouseEvent) => {
// Block menu on the message being Edited or if the report action item has errors
if (draftMessage !== undefined || !isEmptyObject(action.errors) || !shouldDisplayContextMenu) {
return;
}
ReportActionComposeFocusManager.blurAll();
setIsContextMenuActive(true);
const selection = SelectionScraper.getCurrentSelection();
ReportActionContextMenu.showContextMenu(
CONST.CONTEXT_MENU_TYPES.REPORT_ACTION,
event,
selection,
popoverAnchorRef.current,
reportID,

We pass the current report id instead of using the reportID property from the reportAction i.e action.reportID because if we use the current report id the report action does not exist inside the current report id

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

First we need to fix the reportAction value is null, we need to pass action.reportID instead of current reportID
And add the reportID property to actionOnyxData here inside the reportAction variable

const action: ReportAction = useMemo(
() =>
({
reportActionID: reportAction.reportActionID,
message: reportAction.message,
pendingAction: reportAction.pendingAction,

So it will be:

// ReportActionsListItemRenderer.tsx
const action: ReportAction = useMemo(
    () =>
        ({
            reportID: reportAction.reportID,
            reportActionID: reportAction.reportActionID,
            message: reportAction.message,
            pendingAction: reportAction.pendingAction,
            ...
        }),
        [reportAction.reportID, ...]

// ReportActionItem.tsx
ReportActionContextMenu.showContextMenu(
    CONST.CONTEXT_MENU_TYPES.REPORT_ACTION,
    event,
    selection,
    popoverAnchorRef.current,
    action.reportID,

And we can also add ? before the dot . as we do in other function to make sure there's no error because the reportAction value can be null

].some((oldDotActionName) => oldDotActionName === action.actionName);

Result

Screen.Recording.2024-10-19.at.22.11.59.mov

What alternative solutions did you explore? (Optional)

@bernhardoj
Copy link
Contributor

Proposal

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

Can't copy to clipboard a money request update system message.

What is the root cause of that problem?

This only happens in a one-expense report. The issue is that the reportAction here is undefined.

const reportAction: OnyxEntry<ReportAction> = useMemo(() => {
if (isEmptyObject(reportActions) || reportActionID === '0' || reportActionID === '-1') {
return;
}
return reportActions[reportActionID];
}, [reportActions, reportActionID]);

When we update a money request, the update system message action is added to the transaction thread. However, we get the list of actions in the expense report.

const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
canEvict: false,
});

Previously, we get the report actions of originalReportID which contains the transaction thread reportID if it's a transaction thread report action. But it was wrongly changed in this PR.

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

We need to use the originalReportID prop instead of reportID.

const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
canEvict: false,
});
const transactionID = ReportActionsUtils.getLinkedTransactionID(reportActionID, reportID);

We have an originalReportID here but we need to use the originalReportID props.

const originalReportID = useMemo(() => ReportUtils.getOriginalReportID(reportID, reportAction), [reportID, reportAction]);

@NJ-2020
Copy link
Contributor

NJ-2020 commented Oct 21, 2024

Proposal Updated

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
Copy link

melvin-bot bot commented Oct 22, 2024

@puneetlath Whoops! This issue is 2 days overdue. Let's get this updated quick!

@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Oct 22, 2024
@melvin-bot melvin-bot bot changed the title Expense - Console error (app crashes on native) when copying system message to clipboard [$250] Expense - Console error (app crashes on native) when copying system message to clipboard Oct 22, 2024
Copy link

melvin-bot bot commented Oct 22, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 22, 2024
Copy link

melvin-bot bot commented Oct 22, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 22, 2024
@puneetlath
Copy link
Contributor

@thesahindia we have some existing proposals here.

@thesahindia
Copy link
Member

I will review it in the morning.

@thesahindia
Copy link
Member

Is this still reproducible? I couldn't repro it.

@NJ-2020
Copy link
Contributor

NJ-2020 commented Oct 23, 2024

@thesahindia The issue is still reproducible with the latest main

Screen.Recording.2024-10-22.at.21.24.44.mov

Steps to reproduce:

  1. Create new account
  2. Enable staging server
  3. New workspace
  4. Submit Expense (it will only reproducible for the first expense)
  5. Change any fields
  6. Right click to report action comment and copy to clipboard

@NJ-2020
Copy link
Contributor

NJ-2020 commented Oct 23, 2024

Proposal Updated

  • Fix misspelling from reportAction.reportActionID to reportAction.reportID inside ReportActionsListItemRenderer.tsx

Actually I've the correct one mentioned in my proposal inside the useMemo dependencies array

@thesahindia
Copy link
Member

Thanks! I was able to repro it.

@thesahindia
Copy link
Member

@bernhardoj's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 23, 2024

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 23, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review and removed Daily KSv2 labels Oct 24, 2024
@melvin-bot melvin-bot bot added the Weekly KSv2 label Oct 24, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @thesahindia

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 31, 2024
@melvin-bot melvin-bot bot changed the title [$250] Expense - Console error (app crashes on native) when copying system message to clipboard [HOLD for payment 2024-11-07] [$250] Expense - Console error (app crashes on native) when copying system message to clipboard Oct 31, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 31, 2024
Copy link

melvin-bot bot commented Oct 31, 2024

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

Copy link

melvin-bot bot commented Oct 31, 2024

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

If no regressions arise, payment will be issued on 2024-11-07. 🎊

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

  • @bernhardoj requires payment through NewDot Manual Requests
  • @thesahindia requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Oct 31, 2024

@thesahindia @puneetlath The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants