[Simplified Collect][Taxes] It crashes if you try to add a custom name without having any taxes

[lanitochka17]

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.54.0
Reproducible in staging?: Y
Reproducible in production?: N
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:

Issue found when executing PR #38373

Action Performed:

Precondition: With a new expensifail account, create a Collect workspace with workspace chat enabled

1. Navigate to Workspace settings - Taxes - Settings - Custom tax name
2. Type in any name
3. Click on the "Save" button

Expected Result:

It shouldn't crash

Actual Result:

It crashes if you try to add a custom name without having any taxes

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

Bug6418254_1710788901865.bandicam_2024-03-18_20-03-32-375.mp4

View all open jobs on GitHub

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

[melvin-bot]

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

[lanitochka17]

@jasperhuangg 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

[mountiny]

@luacmartins I beelive this should be fixed with adding the default taxes optimistically

[mountiny]

I assume we have to fix the crash though

[luacmartins]

I don't think this is a blocker since users don't have access to these pages yet. cc @kosmydel @filip-solecki

[dragnoir]

Proposal

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

App crashes if you try to add a custom name without having any taxes

What is the root cause of that problem?

App crash because menu items try to turn items when title: policy?.taxRates?.taxes is undefined

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

Turn items only when title: policy?.taxRates?.taxes is defined

const menuItems = useMemo(() => {
    const baseItems = [];

    // Always include the first item
    baseItems.push({
        title: policy?.taxRates?.name,
        description: translate('workspace.taxes.customTaxName'),
        action: () => Navigation.navigate(ROUTES.WORKSPACE_TAXES_SETTINGS_CUSTOM_TAX_NAME.getRoute(policyID)),
        pendingAction: policy?.taxRates?.pendingFields?.name,
    });

    // Conditionally include the second and third items if policy?.taxRates?.taxes is defined
    if (policy?.taxRates?.taxes) {
        baseItems.push({
            title: policy?.taxRates?.taxes[policy?.taxRates?.defaultExternalID]?.name,
            description: translate('workspace.taxes.workspaceDefault'),
            action: () => Navigation.navigate(ROUTES.WORKSPACE_TAXES_SETTINGS_WORKSPACE_CURRENCY_DEFAULT.getRoute(policyID)),
            pendingAction: policy?.taxRates?.pendingFields?.defaultExternalID,
        });
        baseItems.push({
            title: policy?.taxRates?.taxes[policy?.taxRates?.foreignTaxDefault]?.name,
            description: translate('workspace.taxes.foreignDefault'),
            action: () => Navigation.navigate(ROUTES.WORKSPACE_TAXES_SETTINGS_FOREIGN_CURRENCY_DEFAULT.getRoute(policyID)),
            pendingAction: policy?.taxRates?.pendingFields?.foreignTaxDefault,
        });
    }

    return baseItems;
}, [policy?.taxRates, policyID, translate]);

What alternative solutions did you explore? (Optional)

[filip-solecki]

I think it will not happen after merging this PR since there always have to be at least one tax rate

[kosmydel]

I think it will not happen after merging this PR since there always have to be at least one tax rate

We should fix it anyway, just in case the user somehow gets into this state.

[filip-solecki]

Ok, I will handle it @luacmartins

[DylanDylann]

@filip-solecki I think we should hold this issue until the PR is merged

[filip-solecki]

@DylanDylann It should not happen after merging mentioned PR but as we mentioned above fixing it in this way helps to avoid crashing app in some edge cases when user somehow gets into this state
Currently it is for sure easier to test 😄

[DylanDylann]

yeah, but your change still causes some confusion to users (The custom name disappears after entering)
What do you think about displaying the not found page without default tax?

cc @luacmartins

[luacmartins]

I think the fix is fine given that the state shouldn't happen after we have default taxes. I think it's better to not crash the App then the user being confused because the tax they just created disappered.

[trjExpensify]

As #38375 has merged, should we close this now?

[luacmartins]

Yes, we can close this