Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.46k stars 2.81k forks source link

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

Open lanitochka17 opened 1 week ago

lanitochka17 commented 1 week ago

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: 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 https://github.com/Expensify/App/pull/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?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/a9a66180-ca10-4649-979b-4be72ce8754e

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-10-09
Issue OwnerCurrent Issue Owner: @abdulrahuman5196
melvin-bot[bot] commented 1 week ago

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 commented 1 week ago

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

https://github.com/Expensify/App/blob/19a137d26fee56f913eaec7c9b896915ddb002a9/src/pages/workspace/categories/CategorySettingsPage.tsx#L84-L85

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:

https://github.com/Expensify/App/blob/19a137d26fee56f913eaec7c9b896915ddb002a9/src/pages/workspace/categories/CategorySettingsPage.tsx#L62-L67

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.

https://github.com/Expensify/App/blob/19a137d26fee56f913eaec7c9b896915ddb002a9/src/pages/workspace/categories/CategorySettingsPage.tsx#L85

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

What alternative solutions did you explore? (Optional)

NA

huult commented 1 week ago

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?

The defaultTaxRateText still receives the old categoryName after returning from editing the category name https://github.com/Expensify/App/blob/0c2900cb60e3e22ac5ca22e8485bb357de64db67/src/pages/workspace/categories/CategorySettingsPage.tsx#L84

And the categoryName is taken from the URL parameters, and the URL doesn't update categoryName right away. So, after navigating back, it still uses the old categoryName, causing this issue. https://github.com/Expensify/App/blob/0c2900cb60e3e22ac5ca22e8485bb357de64db67/src/pages/workspace/categories/CategorySettingsPage.tsx#L38

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

We should set the params for navigation before navigating back, something like this:

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

    const editCategory = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_CATEGORY_FORM>) => {
            ....
+            Navigation.setParams({categoryName: newCategoryName});
            Navigation.goBack();
        },
        [backTo, currentCategoryName, route.params.categoryName, route.params.policyID],
    );
POC https://github.com/user-attachments/assets/d8786a44-3dd7-4be1-af28-2df364968c5b
melvin-bot[bot] commented 3 days ago

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

melvin-bot[bot] commented 1 day ago

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

melvin-bot[bot] commented 1 day ago

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

melvin-bot[bot] commented 1 day ago

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

JmillsExpensify commented 1 day ago

Opened up for proposal review