[Due for payment 2025-05-22] [$250] Tax backwards compatibility - After disable Tax in OD display an it's not here page in ND

[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: 9.1.18-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/cases/view/2990335
Issue reported by: Applause - Internal Team

Action Performed:

1. Login and create a Collect policy (NewDot compatible)
2. In OldDot - Enable Tax in the workspace
3. Click on the Tax page in ND
4. Toggle the Tax feature off from OldDot
5. Switch to ND

Expected Result:

The Workspace settings should display

Actual Result:

After toggling off the Tax in the OldDot, an it's not here page displays in NewDot

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6781614_1742913389913.Tax_disable.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021904956556885752483
• Upwork Job ID: 1904956556885752483
• Last Price Increase: 2025-03-26
• Automatic offers:
• FitseTLT | Contributor | 106752126

Issue Owner

Current Issue Owner: @zanyrenney

[melvin-bot]

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

[CyberAndrii]

Proposal

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

When opening a page that has a disabled feature, an "it's not here" error is displayed

What is the root cause of that problem?

When a feature is disabled, we display the "it's not here" page

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Line 150 in 869cd0f

const shouldShowNotFoundPage = (!isMoneyRequest && !isFromGlobalCreate && isPolicyNotAccessible) || !isPageAccessible || !isPolicyFeatureEnabled || shouldBeBlocked;

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Lines 173 to 183 in 869cd0f

	if (shouldShowNotFoundPage) {
	return (
	<PageNotFoundFallback
	policyID={policyID}
	isMoneyRequest={isMoneyRequest}
	isFeatureEnabled={isFeatureEnabled}
	isPolicyNotAccessible={isPolicyNotAccessible}
	fullPageNotFoundViewProps={fullPageNotFoundViewProps}
	/>
	);
	}

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

Instead, we can navigate to the WORKSPACE_OVERVIEW or WORKSPACE_MORE_FEATURES page. Add the following code before this line

    if (!isFeatureEnabled) {
        Navigation.navigate(ROUTES.WORKSPACE_OVERVIEW.getRoute(policyID));
        return null;
    }

We intentionally don't wrap the navigate call in useEffect. Otherwise, the not-found page will be displayed for a split second.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

What alternative solutions did you explore? (Optional)

If we still want to use useEffect, we can return null like this

    useEffect(() => {
        if (!isFeatureEnabled) {
            Navigation.navigate(ROUTES.WORKSPACE_OVERVIEW.getRoute(policyID));
        }
    }, [isFeatureEnabled, policyID]);

    if (!isFeatureEnabled) {
        return null;
    }

Note For C+: The proposal below says I'm copying it, but in reality, it pretty much took my main solution and wrapped it in a useEffect. Then I added an explanation of why that is not a good idea and suggested a better way (alternative solution).

[FitseTLT]

🚨 Edited by proposal-police: This proposal was edited at 2025-03-25 16:25:42 UTC.

Proposal

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

Tax backwards compatibility - After disable Tax in OD display an it's not here page in ND

What is the root cause of that problem?

WE use AccessOrNotFoundWrapper that will show not found page if the feature is not enabled

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Line 150 in 869cd0f

const shouldShowNotFoundPage = (!isMoneyRequest && !isFromGlobalCreate && isPolicyNotAccessible) || !isPageAccessible || !isPolicyFeatureEnabled || shouldBeBlocked;

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

We should add an effect that will re-navigate the user to the workspace settings page(or any other page decided by the design team) either in AccessOrNotFoundWrapper for general fix or WorkspaceTaxesPage(or only apply the effect in AccessOrNotFoundWrapper for specific features as tax) to handle this issue only

    useEffect(() => {
        if (isFeatureEnabled || !policyID) {
            return;
        }
        Navigation.navigate(ROUTES.WORKSPACE_OVERVIEW.getRoute(policyID));
    }, [isFeatureEnabled]);

or update this

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Lines 154 to 159 in b7f3f5b

	useEffect(() => {
	if (pendingField && !isOffline && !isFeatureEnabled) {
	return;
	}
	setIsPolicyFeatureEnabled(isFeatureEnabled);
	}, [pendingField, isOffline, isFeatureEnabled]);

useEffect(() => {
        if (pendingField && !isOffline && !isFeatureEnabled) {
            return;
        }
        if (!isFeatureEnabled) {
            Navigation.navigate(ROUTES.WORKSPACE_OVERVIEW.getRoute(policyID));
            return;
        }
        setIsPolicyFeatureEnabled(isFeatureEnabled);
    }, [pendingField, isOffline, isFeatureEnabled]);

If we start to navigate away on feature disable for all features we will not need the isPolicyFeatureEnabled state anymore because we won't display not found for disabled feature.

Note For C+: please check the timestamps of the edit because the above proposal is copying mine after I posted. Thx.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N / A

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

The app shows full screen not found when the tax is disabled. It's not limited to tax, but to all features.

What is the root cause of that problem?

The tax (or other features) page is wrapped with AccessOrNotFoundWrapper, and if the feature is disabled, we show the full screen not found.

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Lines 87 to 91 in ca1f4ff

	const shouldShowFullScreenFallback = !isFeatureEnabled || isPolicyNotAccessible;
	const {shouldUseNarrowLayout} = useResponsiveLayout();
	return (
	<NotFoundPage
	shouldForceFullScreen={shouldShowFullScreenFallback}

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

I think it's better to show a non-fullscreen not found when a feature is disabled. This way, the user knows that the feature is not available and can switch to another tab from the workspace LHN.

const shouldShowFullScreenFallback = isPolicyNotAccessible;

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

[melvin-bot]

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

[melvin-bot]

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

[zanyrenney]

We already have a proposal here @eVoloshchak - please could you review it? Thank you!

[eVoloshchak]

@bernhardoj's proposal

I think it's better to show a non-fullscreen not found when a feature is disabled. This way, the user knows that the feature is not available and can switch to another tab from the workspace LHN.

This is an awesome idea for web and desktop, but with narrow screen layout (mobile phones, narrow browser window), this would still show a full screen not found view

@CyberAndrii's proposal
The fix is pretty straightforward (we just need to navigate to workspace settings if a feature is disabled), but we shouldn't apply this logic to all instances of AccessOrNotFoundWrapper, as it is used in many other places throughout the app

@FitseTLT's proposal uses identical approach, but the important part here is

(or only apply the effect in AccessOrNotFoundWrapper for specific features as tax) to handle this issue only

We should apply this fix only to AccessOrNotFoundWrapper on Pages for worspace features

@FitseTLT's proposal looks good to me!
🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[neil-marcellini]

@FitseTLT's proposal uses identical approach, but the important part here is

(or only apply the effect in AccessOrNotFoundWrapper for specific features as tax) to handle this issue only

We should apply this fix only to AccessOrNotFoundWrapper on Pages for worspace features

@FitseTLT's proposal looks good to me! 🎀👀🎀 C+ reviewed!

I agree. Also, instead of navigating to the workspace overview page, how about we navigate to the "More features" page so the user can likely see that the feature was disabled?

[melvin-bot]

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

[melvin-bot]

This issue has not been updated in over 15 days. @eVoloshchak, @neil-marcellini, @FitseTLT, @zanyrenney eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[melvin-bot]

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

[melvin-bot]

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

• Fix - Tax backwards compatibility - After disable Tax in OD display an it's not here page in ND #59565

If no regressions arise, payment will be issued on 2025-05-22. 🎊

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

• @eVoloshchak requires payment through NewDot Manual Requests
• @FitseTLT requires payment automatic offer (Contributor)

[melvin-bot]

@eVoloshchak @zanyrenney @eVoloshchak 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]

[zanyrenney]

I have sent @eVoloshchak a DM asking him to complete the checklist since we cannot move ahead with the payment that is due today until it is complete. Thank you!

[eVoloshchak]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: N/A, this is a case that wasn't considered initially

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: regression wasn't critical

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Test:

1. Login and create a Collect policy (NewDot compatible)
2. In OldDot - Enable Tax in the workspace
3. Click on the Tax page in ND
4. Toggle the Tax feature off from OldDot
5. Switch to ND
6. Verify that you are navigated to more features page

Do we agree 👍 or 👎

[zanyrenney]

payment summary

@eVoloshchak requires payment through NewDot Manual Requests - please request 250USD on ND.
@FitseTLT requires payment automatic offer (Contributor) - paid 250 USD on upwork.