[$250] Android - Category - "Default tax rate" briefly changes back to the default one

[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.43-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #49867

Action Performed:

1. Open the app
2. Log in with a new expensifail account
3. Create a workspace
4. Navigate to Workspace settings - More features
5. Enable Taxes, Rules and Workflows
6. Navigate to Workspace settings - Categories
7. Tap on any category
8. Tap on "Default tax rate"
9. Select "Tax Rate 1 - 5%"
10. Tap on "Name"
11. Add a few characters after the current name and tap on the "Save" button

Expected Result:

"Default tax rate" should be the one I have set before

Actual Result:

"Default tax rate" briefly changes back to the default one when changing category name for the first time

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

Bug6622187_1727880113686.az_recorder_20241002_162644.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021844119049804579315
• Upwork Job ID: 1844119049804579315
• Last Price Increase: 2024-11-13

Issue Owner

Current Issue Owner: @abdulrahuman5196

[melvin-bot]

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

[daledah]

Proposal

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

"Default tax rate" briefly changes back to the default one when changing category name for the first time

What is the root cause of that problem?

In

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Lines 84 to 85 in 19a137d

	const defaultTaxRateText = useMemo(() => {
	const taxID = CategoryUtils.getCategoryDefaultTaxRate(policy?.rules?.expenseRules ?? [], categoryName, policy?.taxRates?.defaultExternalID);

We use categoryName from url params to get tax rate name. However, after changing catefory name, categoryName remains old value for a brief moment before it gets updated:

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Lines 62 to 67 in 19a137d

	useEffect(() => {
	if (policyCategory?.name === categoryName || !policyCategory) {
	return;
	}
	navigation.setParams({categoryName: policyCategory?.name});
	}, [categoryName, navigation, policyCategory]);

And policy?.rules?.expenseRules would have data of new category name, which will make categoryName doesn't exist in the rules, CategoryUtils.getCategoryDefaultTaxRate returns default rate value.

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

We should use policyCategory?.name, as this value also gets updated simultaneously when changing category name.

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Line 85 in 19a137d

const taxID = CategoryUtils.getCategoryDefaultTaxRate(policy?.rules?.expenseRules ?? [], categoryName, policy?.taxRates?.defaultExternalID);

        const taxID = CategoryUtils.getCategoryDefaultTaxRate(policy?.rules?.expenseRules ?? [], policyCategory?.name ?? categoryName, policy?.taxRates?.defaultExternalID);

What alternative solutions did you explore? (Optional)

NA

[huult]

Edited by proposal-police: This proposal was edited at 2024-10-16 16:07:27 UTC.

Proposal

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

"Default tax rate" briefly changes back to the default one

What is the root cause of that problem?

When we return to the category page from the edit category page, the function defaultTaxRateText is executed before categoryName is updated, so the function defaultTaxRateText still receives the old categoryName. However, since policy?.rules?.expenseRules has been updated, the logic inside the defaultTaxRateText function will return the defaultTaxRate. Because of this, we will see Tax displaying the old tax text that is defaultTaxRate.

The reason defaultTaxRateText is executed before categoryName is updated is that policy?.rules?.expenseRules was changed when we called RENAME_WORKSPACE_CATEGORY, and it is in the dependency of the useMemo hook of the function defaultTaxRateText.

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Line 98 in 19a137d

}, [categoryName, policy?.rules?.expenseRules, policy?.taxRates, translate]);

After that, categoryName will update and set the new value in the URL parameters. Consequently, defaultTaxRateText will re-render again because categoryName changed in the dependencies of the useMemo hook, and defaultTaxRateText will update with the correct tax text.

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Line 66 in 19a137d

navigation.setParams({categoryName: policyCategory?.name});

Because defaultTaxRateText renders twice, the first time it receives the old category and returns the default tax text, and the second time it receives the new category name and returns the correct tax text. So, this issue happens, and it only happens once.

This video describes what I mentioned above.

Screen.Recording.2024-10-24.at.19.03.25.mp4

This issue happened only once after the first editing of the category name due to changes in the policy?.rules?.expenseRules data object structure. In React, useEffect or useMemo use shallow comparison: React only compares references of objects/arrays, not their contents.

You can see in the JSON below that policy?.rules?.expenseRules changes after the first editing of the category name, and it will not change for the next edited category name because the policy?.rules?.expenseRules object structure does not change. Therefore, this issue will not happen with the next edited category name.

and policy?.rules?.expenseRules

This is policy?.rules?.expenseRules data

// Before editing the category name (from SetPolicyCategoryTax response)
[
  {
    "tax": { "field_id_TAX": { "externalID": "id_TAX_RATE_1" } },
    "applyWhen": [
      { "condition": "matches", "field": "category", "value": "Taxes" }
    ]
  }
]

// After editing the category name and going back to the category page(from RenameWorkspaceCategory response)

[
  {
    "applyWhen": [
      { "condition": "matches", "field": "category", "value": "Taxes" }
    ],
    "id": "671a27db42e27",
    "tax": { "field_id_TAX": { "externalID": "id_TAX_RATE_1" } }
  }
]

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

To resolve this issue, we must call defaultTaxRateText after the category name update is complete. To do that, we must delay going back until the category name has finished updating. Something like this:

// .src/pages/workspace/categories/EditCategoryPage.tsx#L52

    const editCategory = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_CATEGORY_FORM>) => {
            ....
+              InteractionManager.runAfterInteractions(() => {
+                requestAnimationFrame(() => {
 Navigation.goBack(
                isQuickSettingsFlow
                    ? ROUTES.SETTINGS_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName, backTo)
                    : ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName),
            );
+                });
+            });
        },
        [backTo, currentCategoryName, route.params.categoryName, route.params.policyID],
    );

What alternative solutions did you explore? (Optional)

alternative solutions 1

Or we can use the policyCategory data to get the new category name after editing to retrieve the default tax rate, because policyCategory has updated values that match with expenseRules

-     const taxID = CategoryUtils.getCategoryDefaultTaxRate(policy?.rules?.expenseRules ?? [], categoryName, policy?.taxRates?.defaultExternalID);
+    const taxID = CategoryUtils.getCategoryDefaultTaxRate(
+            policy?.rules?.expenseRules ?? [],
+            !policyCategory?.name 
+                ? categoryName 
+                : (policyCategory?.name !== categoryName 
+                    ? policyCategory?.name 
+                    : categoryName)
+            policy?.taxRates?.defaultExternalID,
+        );

POC

Screen.Recording.2024-10-03.at.23.19.38.mov

[melvin-bot]

@JmillsExpensify Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

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

[melvin-bot]

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

[JmillsExpensify]

Opened up for proposal review

[melvin-bot]

@JmillsExpensify, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@JmillsExpensify @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[JmillsExpensify]

Still waiting on proposal review.

[abdulrahuman5196]

Checking now

[abdulrahuman5196]

I don't see the issue anymore, can we check if its still reproducible?

Screen.Recording.2024-10-17.at.6.54.31.PM.mov

[huult]

Still able to be reproduced

Screen.Recording.2024-10-17.at.20.40.44.mov

[huult]

@abdulrahuman5196 Can you create a new workspace and follow the steps in this ticket? This issue just happened once.

[huult]

Proposal updated

• Update RCA
• Add alternative solutions

@abdulrahuman5196 I would like to share this video with you to show how we can reproduce this issue.

Screen.Recording.2024-10-24.at.19.44.03.mov

[abdulrahuman5196]

Thank you for this. Let me try again a couple of times today and update here.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[JmillsExpensify]

Still in progress

[melvin-bot]

@JmillsExpensify @abdulrahuman5196 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[wildan-m]

Proposal

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

Android: "Default tax rate" temporarily reverts to the default value.

What is the root cause of that problem?

There is a chance that we navigated before execute Onyx.update first. The blinking actually also affect the category name itself, not only rate.

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

In this case, it's important to execute Onyx.update first before proceeding with the navigation action. We can achieve this by using Navigation.setNavigationActionToMicrotaskQueue as it's a commonly used pattern.

https://github.com/search?q=repo%3AExpensify%2FApp+Navigation.setNavigationActionToMicrotaskQueue%28&type=code

Modify this code:

App/src/pages/workspace/categories/EditCategoryPage.tsx

Lines 52 to 56 in d365d39

	Navigation.goBack(
	isQuickSettingsFlow
	? ROUTES.SETTINGS_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName, backTo)
	: ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName),
	);

To

            Navigation.setNavigationActionToMicrotaskQueue(()=>{
                Navigation.goBack(
                    isQuickSettingsFlow
                        ? ROUTES.SETTINGS_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName, backTo)
                        : ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName),
                );
            });

or only to this if category name changes

    const editCategory = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_CATEGORY_FORM>) => {
            const newCategoryName = values.categoryName.trim();
            const navigateBack = () => {
                Navigation.goBack(
                    isQuickSettingsFlow
                        ? ROUTES.SETTINGS_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName, backTo)
                        : ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName),
                );
            };
    
            // Do not call the API if the edited category name is the same as the current category name
            if (currentCategoryName !== newCategoryName) {
                Category.renamePolicyCategory(route.params.policyID, {oldName: currentCategoryName, newName: values.categoryName});
                Navigation.setNavigationActionToMicrotaskQueue(navigateBack);
                return;
            }
    
            navigateBack();
        },
        [isQuickSettingsFlow, currentCategoryName, route.params.categoryName, route.params.policyID, backTo],
    );

Branch for this solution

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

@JmillsExpensify, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[JmillsExpensify]

@abdulrahuman5196 any thoughts on the latest proposal posted?

[abdulrahuman5196]

Hi, Checking now

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[abdulrahuman5196]

Checking now again

[abdulrahuman5196]

Hi, I am still unable to repro the issue. I reached out in C+ channel to see, if any one is able to repro this and takeover.
https://expensify.slack.com/archives/C02NK2DQWUX/p1731523034411719

[allgandalf]

I am able to reproduce taking over! @JmillsExpensify please assign me here

Screen.Recording.2024-11-14.at.12.21.22.AM.mov

[wildan-m]

I consistently reproduce it by creating a new workspace for each test rather than reusing the same one.

[abdulrahuman5196]

@JmillsExpensify Kindly assign this issue to @allgandalf . Unassigning myself.

[allgandalf]

bump for assignment @JmillsExpensify