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.5k stars 2.85k forks source link

[Re-test Weekly] Rules - Receipt toggle is enabled for non-USD workspaces #50882

Open IuliiaHerets opened 1 week ago

IuliiaHerets 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: v9.0.49-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: https://expensify.testrail.io/index.php?/tests/view/5090143 Email or phone of affected tester (no customers): applausetester+ds@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

As the admin of a non-USD Control workspace that has never enabled Rules in the past

  1. Navigate to More Features in the workspace editor
  2. Enable Rules
  3. Navigate to the Rules page in WS editor

Expected Result:

eReceipt toggle is locked and disabled for non-USD workspaces

Actual Result:

eReceipt toggle is locked and enabled for non-USD workspaces

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/fc763be3-ac38-437b-9018-a312871255c2

View all open jobs on GitHub

melvin-bot[bot] commented 1 week ago

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

IuliiaHerets commented 1 week ago

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

twilight2294 commented 1 week ago

Edited by proposal-police: This proposal was edited at 2024-10-16 11:45:24 UTC.

Proposal

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

Receipt toggle is enabled for non-USD workspaces

What is the root cause of that problem?

When we enable the rules toggle, we call enablePolicyRules in the optimistic data for it, we do not check if the currency is not USD and eReceipts is always set to true in the BE:

https://github.com/Expensify/App/blob/2c211a03948ebc1dfb20ee2f836d9669051acf2f/src/libs/actions/Policy/Policy.ts#L2983-L2995

This is why we have the toggle enabled.

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

We should add a field in the optimistic data to only set eReceipts to true if the currency is usd:

function enablePolicyRules(policyID: string, enabled: boolean, disableRedirect = false) {
    const policy = getPolicy(policyID);
    const onyxData: OnyxData = {
        optimisticData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
                value: {
                    eReceipts: policy?.outputCurrency === CONST.CURRENCY.USD,
                    areRulesEnabled: enabled,
                    ...(!enabled ? DISABLED_MAX_EXPENSE_VALUES : {}),
                    pendingFields: {
                        areRulesEnabled: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
                    },
                },
            },
        ],

If we want then we can even show a error on the rules page that the current currency of policy is not usd and hence we are not allowed to toggle, We can also fix similar but elsewhere, changes would be made to success data as well if necessary, we also need to update the BE in this case

What alternative solutions did you explore? (Optional)

FitseTLT commented 1 week ago

Edited by proposal-police: This proposal was edited at 2024-10-16 12:23:24 UTC.

Proposal

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

Rules - Receipt toggle is enabled for non-USD workspaces

What is the root cause of that problem?

The BE is returning policy.eReceipts as true for non-USD currency workspaces so the toggle is on.

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

Although can fix the BE code to set to eReceipt to false for non-USD currencies we can fix it in FE without a need in the BE by setting the toggle off whenever outputCurrency is not USD similar to how we disable the toggle for this case here BTW this is what we are doing in OD because even if it shows eReceipt toggle as on in ND it is off in OD for the same workspace although the policy.eReceipt field is the same for ND and OD so we can make it consistent with OD and apply a FE fix as: Change https://github.com/Expensify/App/blob/8ff185a05937366c40898f8ab6510fbd765a8838/src/pages/workspace/rules/IndividualExpenseRulesSection.tsx#L181 to

                                isOn={areEReceiptsEnabled && policyCurrency === CONST.CURRENCY.USD}

What alternative solutions did you explore? (Optional)

strepanier03 commented 1 week ago

I'm unable to repro this.

image

image

MelvinBot commented 1 week ago

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

strepanier03 commented 5 days ago

Will take next steps tomorrow.

melvin-bot[bot] commented 1 day ago

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