[$250] Web - Workspace- Inconsistency in adding an item across More features in the workspace

[lanitochka17]

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.46 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 Email or phone of affected tester (no customers): Yokabdk+new213@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Click Account Settings > Workspace
3. Create a new workspace
4. Click More Features > Enable Distance Rate > Go to Distance Rate page > Add rate > Observe that the new rate is enabled
5. Click More Features > Disable Distance Rate > Enable Distance Rate > Go to Distance Rate page > Observe that the rate you added in Step 4 is disabled
6. Click Categories > Add Category > Observe that the new category is enabled
7. Click More Features > Disable Category > Enable Category > Go to Category page > Observe that the category you added in Step 6 is disabled
8. Click More Features > Enable Taxes > Go to Taxes page > Add Rate > Observe that the new rate is enabled
9. Click More Features > Disable Taxes > Enable Taxes > Go to Taxes page > Observe that the rate you added in Step 8 is enabled

Expected Result:

Consistency when adding new items in the features of the workspace

Actual Result:

The new tax rate remains enabled after disabling the tax rate in the More Features settings, while newly added items in other features are disabled once their respective feature is turned off in the More Features settings

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/204bf80b-abb5-4042-834a-02fcb624bad6

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021845959055656223872
• Upwork Job ID: 1845959055656223872
• Last Price Increase: 2024-10-14

Issue Owner

Current Issue Owner: @jasperhuangg

[melvin-bot[bot]]

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

[lanitochka17]

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

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-10-08 13:55:48 UTC.

Proposal

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

Workspace- Inconsistency in adding an item across More features in the workspace

What is the root cause of that problem?

When we disable other features like category, we disable all items of this feature in optimisticData here

https://github.com/Expensify/App/blob/d65bc0e884392c0df08b60d5de5e7235d28e8198/src/libs/actions/Policy/Category.ts#L950-L957

And backend also updated these items as disabled when we called the enable feature API.

But we don't do this when we disable the tax feature in optimistic data and the backend also doesn't update this after reset cache and restart

https://github.com/Expensify/App/blob/d65bc0e884392c0df08b60d5de5e7235d28e8198/src/libs/actions/Policy/Policy.ts#L2776

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

When we disable the tax feature we should disable all items or only disable the tax rate that is not default tax in the optimistic data here and reset it in failure data like we did for other features

if (!enabled) {
    optimisticData.push({
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
        value: {
            taxRates: {
                taxes: {
                    ...Object.keys(policy?.taxRates?.taxes ?? {}).reduce((acc, taxKey) => {
                        acc[taxKey] = {
                            ...defaultTaxRates.taxes[taxKey],
                            isDisabled: taxKey !== policy?.taxRates?.defaultExternalID && taxKey !== policy?.taxRates?.foreignTaxDefault,
                            // or we can update isDisabled: true
                        };
                        return acc;
                    }, {} as Record<string, TaxRate>),
                },
            },
        },
    });
    failureData.push({
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
        value: {
            taxRates: policy?.taxRates?.taxes,
        },
    });
}

https://github.com/Expensify/App/blob/d65bc0e884392c0df08b60d5de5e7235d28e8198/src/libs/actions/Policy/Policy.ts#L2776

If we decided to disable all tax items, we should enable the default external tax and foreign tax when we enable the tax feature.

And backend also needs to update tax items with isDisabled as true in DB when we disable the tax feature

What alternative solutions did you explore? (Optional)

We can update other features not to disable all items in optimistic data when the feature is disabled and we also require the BE change to do that.

https://github.com/Expensify/App/blob/d65bc0e884392c0df08b60d5de5e7235d28e8198/src/libs/actions/Policy/Category.ts#L950-L957

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[mallenexpensify]

@nkdengineer and @s77rt Are steps 4-7 in the OP necessary in order to reproduce the bug? Asking cuz steps 8 and 9 are clearly a bug and I want to remove 4-7 if possible.

@s77rt are you able to reproduce?

I just tried to test in firefox (offline) and Safari (can't open) so ¯_(ツ)_/¯

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[nkdengineer]

@mallenexpensify I can reproduce this bug, the steps 4-7 only need to see the inconsistency. If we want to remove these steps, we can update the actual result and expected result like this:

• Actual result: After the user disables and enables the tax feature again, all tax rates are enabled

• Expected: After the user disables and enables the tax feature again, all tax rates should be disabled except the default tax rate

https://github.com/user-attachments/assets/88b7e9d3-c8b1-42c1-bd2f-d1424c0ac751

[s77rt]

@mallenexpensify I'm able to reproduce. The inconsistency is that when you disable any feature e.g. the distance rates then all the rates gets disabled too (except the default one). On the other hand if you disable the taxes feature, the taxes remain enabled. (You will have to switch the feature back on to see if they got disabled / left enabled)

Distance rates:

https://github.com/user-attachments/assets/0829e236-f3af-4df4-9267-1890a211798e

Taxes:

https://github.com/user-attachments/assets/c837fca6-cb8f-4ec6-9b15-dd174bf98c69

[s77rt]

@nkdengineer I think the taxes behaviour is the correct i.e. we have to fix the rest. I don't see a reason to disable any rate if the feature itself is already disabled

[s77rt]

🎀 👀 🎀 Requires BE changes

[melvin-bot[bot]]

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

[s77rt]

Asked in Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1728988242067289 to confirm if disabling the rates is something intended

[nkdengineer]

Updated alternative solution with another expected.

[mallenexpensify]

@jasperhuangg 👀 above plz, and/or on the slack post

Anyone knows why we disable custom distance rates when we disable the feature? I expected the rates to retain their enabled values.

[jasperhuangg]

Looks like this was how it was written in the back-end. When the distance rates feature is disabled, we disable all of the distance rates. We don't save a snapshot of how they look before disabling, so when they are reenabled we only reenable the default distance rate.

I don't think we need to solve this, but cc @mountiny since he implemented the back-end logic. If you agree I think we can just close this issue out.

[s77rt]

@jasperhuangg I believe there are still unanswered questions:

When the distance rates feature is disabled, we disable all of the distance rates

1. Why do we need to disable the distance rates? Isn't disabling the feature itself sufficient?
2. If there is a reason for the above question, how come we don't do the same for the taxes? (the tax rates remain unchanged)

[mountiny]

Yeah this is basically for oldDot. in OldDot, these feature toggles do not exist so disabling the X-feature would do nothing.

So we do not have to write a bunch of new code on oldDotand mobile oldApp, we decided to just turn off the underlying props of the feature so it's not blocking the user. Ie if categories are disabled, we should not require them on any expense even if that was a settings on categories

[jasperhuangg]

@mountiny Do you agree that we should :donothing here?

[mountiny]

Yes, taxes already had this kind of feature toggle in oldDot so that is why its different. Its inconsistent by design.

Gonna close it, thanks