[$250] [P2P Distance] Distance rate - Error when selecting distance rate that no longer has tax rate

[IuliiaHerets]

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.21-3 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

• Track tax is enabled.
• Create a Distance rate X that has Tax rate and Tax reclaimable on.

1. Go to staging.new.expensify.com
2. Go to workspace settings > Taxes.
3. Delete tha tax rate that is assigned to Distance rate X from the precondition.
4. Go to workspace chat.
5. Submit a distance expense with another distance rate that is not Distance rate X.
6. Go to transaction thread.
7. Click Rate.
8. Select Distance rate X.

Expected Result:

App will not throw error when selecting a distance rate that no longer has tax rate (tax rate is deleted after it is assigned to distance rate).

Actual Result:

App throws error when selecting a distance rate that no longer has tax rate.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/0d7b785e-09ac-4256-9f22-019b2a42fb9f

---

https://github.com/Expensify/App/issues/47613#issuecomment-2307640614

On NewDot in the workspace settings, I propose this change to make it clearer that the Tax rate and Tax reclaimable on set on a distance rate are interlinked:

Don’t show the Tax reclaimable on field until a Tax rate is selected. Note: This would be similar to how we show certain fields in settings after a selection that comes before it, i.e monthly for scheduled submit reveals a Date field etc).

If the Tax rate being used on the distance rate is deleted in Taxes, clear both the Tax rate and Tax reclaimable on values. We would then also hide the Tax reclaimable on field again as Tax rate is empty. Note: This would prevent an “unexpected error” from occurring, as you can’t get into a situation where there’s a Tax reclaimable on value without a Tax rate selected.

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01def12cd4fe837196
• Upwork Job ID: 1828100234986953227
• Last Price Increase: 2024-08-26
• Automatic offers:
• DylanDylann | Reviewer | 103922844

Issue Owner

Current Issue Owner: @DylanDylann

[melvin-bot[bot]]

Triggered auto assignment to @OfstadC (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]

We think that this bug might be related to #wave-collect - Release 1

[IuliiaHerets]

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

[daledah]

Proposal

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

App throws error when selecting a distance rate that no longer has tax rate.

The problem also happens when the linked tax rate is disabled.

What is the root cause of that problem?

DeletePolicyTaxes API does not remove the deleted tax rate from the attached distance rate, it only clears the tax rate itself:

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

We should:

1. not display; or
2. display as disabled

the distance rate with deleted tax rate in distance rate selector.

First, modify the getMileageRates here to include a new param includeDisabledTrackTaxRates to filter rates with deleted or disabled tax rates.

Secondly, we need to know whether its corresponding tax rate is deleted or disabled by PolicyUtils.getTaxByID here.

const taxRate = PolicyUtils.getTaxByID(rate.attributes.taxRateExternalID);
const isTaxRateAvailable = taxRate && !taxRate.isDisabled;

Then:

1. Filter those distance rates accordingly by appending isTaxRateAvailable to the early return condition here.

2. Introduce a new attribute disabled to the returned millage rates here. Then show it as disabled in the sections here by isInteractive: !rate.disabled.

disabled: PolicyUtils.isTaxTrackingEnabled && !isTaxRateAvailable

[OfstadC]

I feel like it should show an error, no? Going to discuss in Slack.

[OfstadC]

Slack discussion ongoing

[OfstadC]

@trjExpensify Could you confirm expected behavior here? I don't have the tax option on Distance rates in NewDot 🤔

[trjExpensify]

Commented in thread!

[trjExpensify]

Adding @MonilBhavsar here per the thread.

[trjExpensify]

On NewDot in the workspace settings, I propose this change to make it clearer that the Tax rate and Tax reclaimable on set on a distance rate are interlinked:

• Don’t show the Tax reclaimable on field until a Tax rate is selected.

Note: This would be similar to how we show certain fields in settings after a selection that comes before it, i.e monthly for scheduled submit reveals a Date field etc).

• If the Tax rate being used on the distance rate is deleted in Taxes, clear both the Tax rate and Tax reclaimable on values. We would then also hide the Tax reclaimable on field again as Tax rate is empty.

Note: This would prevent an “unexpected error” from occurring, as you can’t get into a situation where there’s a Tax reclaimable on value without a Tax rate selected.

[MonilBhavsar]

@daledah if you want to update your proposal based on this^

[daledah]

Edited by proposal-police: This proposal was edited at 2024-08-19T10:00:00Z.

Updated Proposal

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

App throws error when selecting a distance rate that no longer has tax rate.

The problem also happens when the linked tax rate is disabled.

What is the root cause of that problem?

DeletePolicyTaxes API does not remove the deleted tax rate from the attached distance rate, it only clears the tax rate itself:

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

Don’t show the Tax reclaimable on field until a Tax rate is selected.

Add the condition of having a tax rate selected here: https://github.com/Expensify/App/blob/d48a513f2960a6164d9376ec1d290123a5bf3d5b/src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx#L157

isDistanceTrackTaxEnabled && !!taxRate

If the Tax rate being used on the distance rate is deleted in Taxes, clear both the Tax rate and Tax reclaimable on values.

We need to update the BE to handle this and implement optimistic data in the FE.

In deletePolicyTaxes action here, we need to check if there's any linked rate with tax IDs from taxesToDelete:

const customUnits = policy?.customUnits ?? {};
const ratesToUpdate = Object.values(customUnits?.[Object.keys(customUnits)[0]]?.rates).filter((rate) => !!rate.attributes?.taxRateExternalID && taxesToDelete.includes(rate.attributes?.taxRateExternalID));

Then if there are, build the onyx data to clear the tax rate and value from the corresponding distance rate:

const optimisticRates: Record<string, Rate> = {};
const successRates: Record<string, Rate> = {};
const failureRates: Record<string, Rate> = {};

ratesToUpdate.forEach((rate) => {
    const rateID = rate.customUnitRateID ?? '';
    optimisticRates[rateID] = {
        ...rate,
        attributes: {
            ...rate?.attributes,
            taxRateExternalID: undefined,
            taxClaimablePercentage: undefined,
        },
        pendingFields: {
            taxRateExternalID: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
            taxClaimablePercentage: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
        },
    };
    successRates[rateID] = {...rate, pendingFields: {taxRateExternalID: null}};
    failureRates[rateID] = {
        ...rate,
        pendingFields: {taxRateExternalID: null, taxClaimablePercentage: null},
        errorFields: {
            taxRateExternalID: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
            taxClaimablePercentage: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
        },
    };
});

And use them inside onyxData here.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[MonilBhavsar]

@DylanDylann could you please take a look at the proposal based on this expected outcome https://github.com/Expensify/App/issues/47613#issuecomment-2307640614 bas

[DylanDylann]

@daledah proposal looks good to me. I see some minor problems in your implementation. However the detailed implementation will be discussed more in the PR phase.

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@OfstadC @MonilBhavsar @DylanDylann 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!

[DylanDylann]

Waiting for @MonilBhavsar to give the final assignment

[melvin-bot[bot]]

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

[DylanDylann]

@MonilBhavsar Bump on Slack

[melvin-bot[bot]]

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

[MonilBhavsar]

Sorry for missing this out. Yep the proposal partially fixes the issue. It is missing details to fix following part of the issue -

Don’t show the Tax reclaimable on field until a Tax rate is selected.

and also, if we delete the tax rate, we need to hide the tax reclaimable field. cc @daledah

[daledah]

@MonilBhavsar

Don’t show the Tax reclaimable on field until a Tax rate is selected.

I already covered this in the first part of my proposal here.

if we delete the tax rate, we need to hide the tax reclaimable field

It would automatically be hidden as the result of deleting the linked tax rate in Onyx with my suggested changes above.

[DylanDylann]

Not overdue. @MonilBhavsar Please recheck this one

[MonilBhavsar]

Sounds good 👍 Thanks for clarifying

[melvin-bot[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[bot]]

📣 @daledah 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 📖

[MonilBhavsar]

@trjExpensify a quick question here. If user deletes a tax rate associated with distance rate, should we display an error/confirmation if they really want to delete it and deleting it would also clear tax rate from distance rate, or we should delete it without any notification

[daledah]

@DylanDylann PR is up.