App: [HOLD for payment 2024-02-14] [$500] Long pressing "Go to Expensify Classic" option in Settings Crashes the App

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: 1.4.32-2 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: @ishpaul777 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1706307323470669

Action Performed:

Go to settings
Long press Go to expensify classic

Expected Result:

App should not crash

Actual Result:

App crashed

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

https://github.com/Expensify/App/assets/38435837/6c15913f-349b-4db2-af94-f46ab99b7d42

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~017b2c107d082607ce
Upwork Job ID: 1751947232616599552
Last Price Increase: 2024-01-29
Automatic offers:

ishpaul777 | Contributor | 28134405

About this issue

Original URL
State: closed
Created 5 months ago
Comments: 22 (11 by maintainers)

Most upvoted comments

not overdue, we can close this now

ishpaul777 on Feb 19, 2024

The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/33863
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/33863/files#r1492831004
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: not required
Determine if we should create a regression test for this bug. not required
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. - not required

ishpaul777 on Feb 16, 2024

$500 approved for @mananjadhav based on summary above.

JmillsExpensify on Feb 14, 2024

I tried finding the root PR, the best one where we refactored the links in InitialSettingsPage is this PR. @ishpaul777 if you could identify the offending PR?

I don’t think we need a regression test for this one.

mananjadhav on Feb 12, 2024

Proposal

Problem Statement

Users are experiencing app crashes when performing a long press on the “Go to Expensify Classic” option in Settings.

Root Cause

We are not using Link.buildOldDotURL properly in the getDefaultMenuItems in the InitialSettingsPage. The function returns a promise instead of a string, leading to unexpected behavior and crash.

https://github.com/Expensify/App/blob/c5ca9a810b529cdc8b8fceb179e9cfc01061355a/src/pages/settings/InitialSettingsPage.js#L274

Proposed Solution

To address the problem, we should handle the link properly similar to what we have done for MenuItemList:

https://github.com/Expensify/App/blob/c5ca9a810b529cdc8b8fceb179e9cfc01061355a/src/pages/settings/InitialSettingsPage.js#L274

to:

 link: () => Link.buildOldDotURL(CONST.OLDDOT_URLS.INBOX),

https://github.com/Expensify/App/blob/c5ca9a810b529cdc8b8fceb179e9cfc01061355a/src/components/MenuItemList.tsx#L34-L40

// src/pages/settings/InitialSettingsPage.js

const onSecondaryInteraction = (link, event) => {
    if (typeof link === 'function') {
        console.debug('InitialSettingsPage: secondaryInteraction: link is a function, executing it');
        link().then((url) => ReportActionContextMenu.showContextMenu(CONST.CONTEXT_MENU_TYPES.LINK, event, url, popoverAnchor.current));
    } else if (link) {
        ReportActionContextMenu.showContextMenu(CONST.CONTEXT_MENU_TYPES.LINK, event, link, popoverAnchor.current);
    }
};

<MenuItem
    // rest of props
    onSecondaryInteraction={item.link !== undefined ? (e) => onSecondaryInteraction(item.link, e) : undefined}
/>

ishpaul777 on Jan 29, 2024