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.33k stars 2.76k forks source link

[$250] Track tax - Tax rate does not update when editing distance rate offline #48293

Open lanitochka17 opened 2 weeks ago

lanitochka17 commented 2 weeks 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.26-2 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/4902670 Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by:Applause - Internal Team

Action Performed:

Precondition:

Expected Result:

Actual Result:

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/8b3c5a70-32ea-4bb2-bd98-bcbed30ee528

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c25cfebc45c34fc3
  • Upwork Job ID: 1830995712415705839
  • Last Price Increase: 2024-09-10
  • Automatic offers:
    • Krishna2323 | Contributor | 103905995
Issue OwnerCurrent Issue Owner: @aimane-chnaif
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @bfitzexpensify (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 commented 2 weeks ago

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

Krishna2323 commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-30 00:03:36 UTC.

Proposal

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

Track tax - Tax rate does not update when editing distance rate offline

What is the root cause of that problem?

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

Code changes ```javascript function selectDistanceRate(customUnitRateID: string) { let taxAmount; let taxRateExternalID; if (shouldShowTax) { const policyCustomUnitRate = getCustomUnitRate(policy, customUnitRateID); taxRateExternalID = policyCustomUnitRate?.attributes?.taxRateExternalID ?? '-1'; const taxableAmount = DistanceRequestUtils.getTaxableAmount(policy, customUnitRateID, TransactionUtils.getDistanceInMeters(transaction, unit)); const taxPercentage = TransactionUtils.getTaxValue(policy, transaction, taxRateExternalID) ?? ''; taxAmount = CurrencyUtils.convertToBackendAmount(TransactionUtils.calculateTaxAmount(taxPercentage, taxableAmount, rates[customUnitRateID].currency ?? CONST.CURRENCY.USD)); IOU.setMoneyRequestTaxAmount(transactionID, taxAmount); IOU.setMoneyRequestTaxRate(transactionID, taxRateExternalID); } if (currentRateID !== customUnitRateID) { IOU.setMoneyRequestDistanceRate(transactionID, customUnitRateID, policy?.id ?? '-1', !isEditing); if (isEditing) { IOU.updateMoneyRequestDistanceRate( transaction?.transactionID ?? '-1', reportID, customUnitRateID, policy, policyTags, policyCategories, shouldShowTax ? taxRateExternalID : transaction?.taxCode, shouldShowTax ? taxAmount : transaction?.taxAmount, ); } } navigateBack(); } function updateMoneyRequestDistanceRate( transactionID: string, transactionThreadReportID: string, rateID: string, policy: OnyxEntry, policyTagList: OnyxEntry, policyCategories: OnyxEntry, taxCode?: string, taxAmount?: number, ) { const transactionChanges: TransactionChanges = { customUnitRateID: rateID, ...(taxCode ? {taxCode} : {}), ...(taxAmount ? {taxAmount} : {}), }; // ... } ```

What alternative solutions did you explore? (Optional)



We can do few things here:

Result

Krishna2323 commented 2 weeks ago

Proposal Updated

nkdengineer commented 2 weeks ago

Proposal

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

Tax rate does not update after changing distance rate offline in transaction thread. The only thing that updates is the removal of "default" label in tax rate field Tax rate and tax amount field are not gray out

What is the root cause of that problem?

Tax rate does not update after changing distance rate offline in transaction thread. The only thing that updates is the removal of "default" label in tax rate field

  1. We have the logic here to update the tax rate and tax amount when we update the distance rate if tax tracking is enabled. It updates the draft transaction data used in the create flow.

https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L88-L97

When we're in the edit flow, we don't update the tax rate and tax amount in optimistic data in getUpdateMoneyRequestParams function

Tax rate and tax amount field are not gray out

Tax rate/tax amount is only grayed out if we have pending action on tax rate/tax amount https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/components/ReportActionItem/MoneyRequestView.tsx#L621

https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/components/ReportActionItem/MoneyRequestView.tsx#L637

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

Tax rate will update after changing distance rate offline in transaction thread, as it works on confirmation page (Step 7)

For this bug, we can follow the same way we do in IOURequestStepDistanceRate to update the tax rate and tax amount in optimistic data when we edit the distance rate after creating. We can create a util

function getTaxRateAndTaxAmountForTrackTax(transaction: OnyxInputOrEntry<Transaction>, policy: OnyxInputOrEntry<Policy>, isPolicyExpenseChat: boolean, customUnitRateID = '') {
    if (!transaction || !policy || !customUnitRateID || !PolicyUtils.isTaxTrackingEnabled(isPolicyExpenseChat, policy, true)) {
        return {
            taxCode: undefined,
            taxAmount: undefined,
        };
    }

    const mileageRates = DistanceRequestUtils.getMileageRates(policy, true);
    const {unit} = mileageRates[customUnitRateID] ?? DistanceRequestUtils.getDefaultMileageRate(policy);
    const policyCustomUnitRate = getCustomUnitRate(policy, customUnitRateID);
    const taxRateExternalID = policyCustomUnitRate?.attributes?.taxRateExternalID ?? '-1';
    const taxableAmount = DistanceRequestUtils.getTaxableAmount(policy, customUnitRateID, getDistanceInMeters(transaction, unit));
    const taxPercentage = getTaxValue(policy, transaction ?? undefined, taxRateExternalID) ?? '';
    const taxAmount = convertToBackendAmount(calculateTaxAmount(taxPercentage, taxableAmount, mileageRates[customUnitRateID].currency ?? CONST.CURRENCY.USD));
    return {taxCode: taxRateExternalID, taxAmount};
}

https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L88-L97

const {taxCode, taxAmount} = getTaxRateAndTaxAmountForTrackTax(transaction, policy, isFromExpenseReport, customUnitRateID);

if (hasModifiedRouteWithPendingWaypoints || hasModifiedRateWithPendingWaypoints) {
    return {
        amount: CONST.IOU.DEFAULT_AMOUNT,
        modifiedAmount: CONST.IOU.DEFAULT_AMOUNT,
        modifiedMerchant: Localize.translateLocal('iou.fieldPending'),
        ...(taxCode ? {taxCode} : {}),
        ...(taxAmount ? {taxAmount} : {}),
    };
}

...

return {
    amount: updatedAmount,
    modifiedAmount: updatedAmount,
    modifiedMerchant: updatedMerchant,
    modifiedCurrency: updatedCurrency,
    ...(taxCode ? {taxCode} : {}),
    ...(taxAmount ? {taxAmount} : {}),
};

and use this in calculateAmountForUpdatedWaypointOrRate function

Tax rate and tax amount field will gray out

For this bug, we can check if the request is distance request we will add the fallback pending action for tax rate and tax amount with the pending action of distance rate here

<OfflineWithFeedback pendingAction={isDistanceRequest ? getPendingFieldAction('taxCode') ?? getPendingFieldAction('customUnitRateID') : getPendingFieldAction('taxCode')}>

https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/components/ReportActionItem/MoneyRequestView.tsx#L621

<OfflineWithFeedback pendingAction={isDistanceRequest ? getPendingFieldAction('taxAmount') ?? getPendingFieldAction('customUnitRateID') : getPendingFieldAction('taxAmount')}>

https://github.com/Expensify/App/blob/489312e0d75cbbbf5d0d78fae48bb37635f921d6/src/components/ReportActionItem/MoneyRequestView.tsx#L637

What alternative solutions did you explore? (Optional)

NA

Krishna2323 commented 2 weeks ago

Proposal Updated

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

aimane-chnaif commented 1 week ago

Thanks for the proposals. Can you please share demo videos after applying your solution? Please make sure that Tax rate and tax amount field are not gray out bug is also fixed.

Krishna2323 commented 1 week ago

@aimane-chnaif, test branch.

https://github.com/user-attachments/assets/d76bdde3-1cac-4804-a963-249fe155ba92

nkdengineer commented 1 week ago

@aimane-chnaif The result here.

https://github.com/user-attachments/assets/c6d0e61c-f512-439f-bee5-edcd8df4e896

bfitzexpensify commented 1 week ago

I am heading out of office until September 21st, so assigning a buddy to watch over this in my absence.

Current status: working through proposals

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 5 days ago

@lschurr, @bfitzexpensify, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

trjExpensify commented 4 days ago

@aimane-chnaif what do you think of these proposals? CC: @MonilBhavsar for eyes.

melvin-bot[bot] commented 4 days ago

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

aimane-chnaif commented 4 days ago

@Krishna2323's main solution looks good. 🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 4 days ago

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

melvin-bot[bot] commented 4 days ago

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

melvin-bot[bot] commented 2 days ago

@marcaaron @lschurr @bfitzexpensify @Krishna2323 @aimane-chnaif 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!

Krishna2323 commented 13 hours ago

@aimane-chnaif, PR ready for review ^