[HOLD for payment 2024-08-14] [$250] Workspace - Not found page briefly appear on Accounting feature

[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.0.7-4
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Issue found when executing PR #45239

Action Performed:

1. Log in
2. Turn off the network connection
3. Click on the account icon
4. Navigate to workspaces
5. Create a new workspace
6. Click on More Features
7. Enable accounting feature
8. Click on the enabled feature section
9. Reconnect to the network

Expected Result:

No "not found" page appears

Actual Result:

Not found page briefly appear

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

Bug6544046_1721141224071.Fail_PR_45239.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010656791d40958772
• Upwork Job ID: 1813317098317018164
• Last Price Increase: 2024-07-16
• Automatic offers:
• DylanDylann | Reviewer | 103160114
• dominictb | Contributor | 103314660

Issue Owner

Current Issue Owner: @DylanDylann

[melvin-bot]

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

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

[jasperhuangg]

Definitely doesn't need to be a blocker since the functionality still works as expected. It looks like something is wrong with the data returned by the API?

[jasperhuangg]

Based on my reproduction here it seems like EnablePolicyConnections isn't setting the correct Onyx data (it looks like some optimistic data is being incorrectly reset).

I think this can actually be worked on externally.

[melvin-bot]

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

[melvin-bot]

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

[Zakpak0]

Proposal

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

The "Accounting" integration returns a "Not Found" page when enabled in offline mode.

What is the root cause of that problem?

src/libs/actions/Policy/Policy.ts

        sucessData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
                value: {
                    pendingFields: {
                        areConnectionsEnabled: null,
                    },
                },

The success data set by Onyx, reverts the Optimistic data that was set while offline.
Nothing is set.

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

        successData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
                value: {
                    areConnectionsEnabled: enabled,
                    pendingFields: {
                        areConnectionsEnabled: null,
                    },
                },

Set the success data to enable the connection. I've observed that connection is enabled on page reload.

What alternative solutions did you explore? (Optional)

N/A

[Krishna2323]

Proposal

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

Workspace - Not found page briefly appear on Accounting feature

What is the root cause of that problem?

• Optimistic policy is created in offline mode.
• When accounting is enabled offline the the property for the the feature in optimistic policy is set as true.
• When we turn on the connection the createWorkspace api call succeeds and set feature enabled state to default, which sets accounting feature enabled as false and due to this we briefly see not found page.

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Line 127 in 52b8fdd

const isFeatureEnabled = featureName ? PolicyUtils.isPolicyFeatureEnabled(policy, featureName) : true;

App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

Line 138 in 52b8fdd

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

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

We need to follow the same approach used in WorkspaceInitialPage, we should store the features state in a useState and only update that when the feature doesn't have any pending action.

App/src/pages/workspace/WorkspaceInitialPage.tsx

Lines 213 to 228 in 52b8fdd

	// We only update feature states if they aren't pending.
	// These changes are made to synchronously change feature states along with AccessOrNotFoundWrapperComponent.
	useEffect(() => {
	setFeatureStates((currentFeatureStates) => {
	const newFeatureStates = {} as PolicyFeatureStates;
	(Object.keys(policy?.pendingFields ?? {}) as PolicyFeatureName[]).forEach((key) => {
	const isFeatureEnabled = PolicyUtils.isPolicyFeatureEnabled(policy, key);
	newFeatureStates[key] =
	prevPendingFields?.[key] !== policy?.pendingFields?.[key] || isOffline || !policy?.pendingFields?.[key] ? isFeatureEnabled : currentFeatureStates[key];
	});
	return {
	...policyFeatureStates,
	...newFeatureStates,
	};
	});
	}, [policy, isOffline, policyFeatureStates, prevPendingFields]);

    // In AccessOrNotFoundWrapper
    const policyFeatureStates = useMemo(
        () => ({
            [CONST.POLICY.MORE_FEATURES.ARE_DISTANCE_RATES_ENABLED]: policy?.areDistanceRatesEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_WORKFLOWS_ENABLED]: policy?.areWorkflowsEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_CATEGORIES_ENABLED]: policy?.areCategoriesEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_TAGS_ENABLED]: policy?.areTagsEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_TAXES_ENABLED]: policy?.tax?.trackingEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_CONNECTIONS_ENABLED]: policy?.areConnectionsEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_EXPENSIFY_CARDS_ENABLED]: policy?.areExpensifyCardsEnabled,
            [CONST.POLICY.MORE_FEATURES.ARE_REPORT_FIELDS_ENABLED]: policy?.areReportFieldsEnabled,
        }),
        [policy],
    ) as PolicyFeatureStates;

    const [featureStates, setFeatureStates] = useState(policyFeatureStates);
    const prevPendingFields = usePrevious(policy?.pendingFields);

    useEffect(() => {
        setFeatureStates((currentFeatureStates) => {
            const newFeatureStates = {} as PolicyFeatureStates;
            (Object.keys(policy?.pendingFields ?? {}) as PolicyFeatureName[]).forEach((key) => {
                const isFeatureEnabled2 = PolicyUtils.isPolicyFeatureEnabled(policy, key);
                newFeatureStates[key] =
                    prevPendingFields?.[key] !== policy?.pendingFields?.[key] || isOffline || !policy?.pendingFields?.[key] ? isFeatureEnabled2 : currentFeatureStates[key];
            });
            return {
                ...policyFeatureStates,
                ...newFeatureStates,
            };
        });
    }, [policy, isOffline, policyFeatureStates, prevPendingFields]);

    const isFeatureEnabled = featureName ? featureStates?.[featureName] : true;
    const isPolicyNotAccessible = isEmptyObject(policy) || (Object.keys(policy).length === 1 && !isEmptyObject(policy.errors)) || isFeatureEnabled || !policy?.id;
    const isPageAccessible = accessVariants.reduce((acc, variant) => {
        const accessFunction = ACCESS_VARIANTS[variant];
        return acc && accessFunction(policy, login, report, allPolicies ?? null, iouType);
    }, true);
    
    const shouldShowNotFoundPage = (!isMoneyRequest && !isFromGlobalCreate && isPolicyNotAccessible) || !isPageAccessible || shouldBeBlocked;

The code will be repeated so we should create a custom hook for this.

What alternative solutions did you explore? (Optional)

[dominictb]

Proposal

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

• Not found page briefly appear

What is the root cause of that problem?

• We have a logic to display the PolicyAccountingPage:
  

  App/src/pages/workspace/withPolicyConnections.tsx

  Lines 43 to 53 in 52b8fdd

  	if (isConnectionDataFetchNeeded && shouldBlockView) {
  	return <FullScreenLoadingIndicator />;
  	}
  	return (
  	<WrappedComponent
  	// eslint-disable-next-line react/jsx-props-no-spreading
  	{...props}
  	isConnectionDataFetchNeeded={isConnectionDataFetchNeeded}
  	/>
  	);

• In offline, we create a new workspace, turn on the accounting feature, then open the accounting page. Then go online. Firstly, it displays the loading indicator:
  

  App/src/pages/workspace/withPolicyConnections.tsx

  Lines 43 to 45 in 52b8fdd

  	if (isConnectionDataFetchNeeded && shouldBlockView) {
  	return <FullScreenLoadingIndicator />;
  	}

  
  because:
  

  App/src/pages/workspace/withPolicyConnections.tsx

  Line 31 in 52b8fdd

  const isConnectionDataFetchNeeded = !isOffline && props.policy && props.policy.areConnectionsEnabled && !props.policy.connections && !hasConnectionsDataBeenFetched;

  
  is true.

• Then, when API CreateWorkspace is done (EnablePolicyConnections is still in progress) , it set policy.areConnectionsEnabled to false. At that time, isConnectionDataFetchNeeded is false because props.policy.areConnectionsEnabled is false.

• Now, it displays the PolicyAccountingPage instead of loading indicator:
  

  App/src/pages/workspace/withPolicyConnections.tsx

  Lines 47 to 53 in 52b8fdd

  	return (
  	<WrappedComponent
  	// eslint-disable-next-line react/jsx-props-no-spreading
  	{...props}
  	isConnectionDataFetchNeeded={isConnectionDataFetchNeeded}
  	/>
  	);

• The PolicyAccountingPage is wrapped by AccessOrNotFoundWrapper component.

• At that time, isPolicyFeatureEnabled is false initially:
  

  App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

  Line 129 in 52b8fdd

  const [isPolicyFeatureEnabled, setIsPolicyFeatureEnabled] = useState(isFeatureEnabled);

  
  because isFeatureEnabled is false:
  

  App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

  Line 127 in 52b8fdd

  const isFeatureEnabled = featureName ? PolicyUtils.isPolicyFeatureEnabled(policy, featureName) : true;

• So, the not found page is displayed:
  

  App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

  Lines 154 to 162 in 52b8fdd

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

  
  because shouldShowNotFoundPage is true:
  

  App/src/pages/workspace/AccessOrNotFoundWrapper.tsx

  Line 138 in 52b8fdd

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

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

• We need to make sure we always display the loading indicator:
  

  App/src/pages/workspace/withPolicyConnections.tsx

  Lines 43 to 45 in 52b8fdd

  	if (isConnectionDataFetchNeeded && shouldBlockView) {
  	return <FullScreenLoadingIndicator />;
  	}

  
  when we start calling openPolicyAccountingPage(props.policy.id). Currently, we only display the loading indicator when we starts calling API OpenPolicyAccountingPage.

• To do it, update this
  to:

    Onyx.merge(hasConnectionsDataBeenFetchedKey, false);

and this
to:

        if ((isConnectionDataFetchNeeded || hasConnectionsDataBeenFetched === false) && shouldBlockView) {

What alternative solutions did you explore? (Optional)

1. In this file, create a new state:

        const [isFetchingData, setIsFetchingData] = useState(false)

2. Then create a new variable:

        const prevHasConnectionsDataBeenFetched = usePrevious(hasConnectionsDataBeenFetched);

3. Then add a new useEffect, that will trigger setIsFetchingData(false) one the API is done:

        useEffect(() => {
            if (prevHasConnectionsDataBeenFetched === undefined && isBoolean(hasConnectionsDataBeenFetched)) {
                setIsFetchingData(false)
            }
        }, [hasConnectionsDataBeenFetched, prevHasConnectionsDataBeenFetched]);

4. Then update:
   

   App/src/pages/workspace/withPolicyConnections.tsx

   Line 43 in 4ba10bf

   if (isConnectionDataFetchNeeded && shouldBlockView) {

   
   to:

        if ((isConnectionDataFetchNeeded || isFetchingData) && shouldBlockView) {

5. Finaly, remove this optimistic data:
   

   App/src/libs/actions/PolicyConnections.ts

   Lines 12 to 16 in 52b8fdd

   	{
   	onyxMethod: Onyx.METHOD.MERGE,
   	key: hasConnectionsDataBeenFetchedKey,
   	value: false,
   	},

   
   By removing this optimistic data, we can know if the API is in progress or called successfully. If it is in progress, the hasConnectionsDataBeenFetchedKey is undefined. If it is called successfully without the error, hasConnectionsDataBeenFetchedKey is true. Otherwise, hasConnectionsDataBeenFetchedKey is false. We make use of this fact in step 3 above.

See Test branch

[dominictb]

Updated proposal to fix the minor typo issue

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @Zakpak0 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[jasperhuangg]

@Zakpak0's proposal seems simple and fits within our existing patterns. Assigned!

[dominictb]

@jasperhuangg The selected proposal does not fix the issue, I still can reproduce the bug when applying it. Can you help confirm @DylanDylann?

[DylanDylann]

@Krishna2323 Please provide a test branch or detail implementation then I can test this approach. Thanks

[dominictb]

@DylanDylann I've updated my alternative solution with detailed code changes, explanations, and a test branch. This alternative is essentially the second approach within my main solution.

[melvin-bot]

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

[jasperhuangg]

Proposals still under review

[jasperhuangg]

@DylanDylann bump on proposal review!

[DylanDylann]

@dominictb's alternative solution looks good to me

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[jasperhuangg]

It makes sense to me as well, thank you for the investigation and discussion @DylanDylann. @dominictb let's move forward with implementing your alternative proposal!

[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.0.17-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: update logic to show loading indicator in accounting page #46563

If no regressions arise, payment will be issued on 2024-08-14. 🎊

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

• @DylanDylann requires payment automatic offer (Reviewer)
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @OfstadC you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[OfstadC]

Payment Summary

• @DylanDylann paid $250 via Upwork
• @dominictb paid $250 via Upwork