Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distance - App crashes after selecting rate that has no Tax rate & Tax reclaimable on #49556

Open
2 of 6 tasks
IuliiaHerets opened this issue Sep 20, 2024 · 5 comments
Open
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@IuliiaHerets
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.39-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

  1. Launch New Expensify app.
  2. Create a new workspace.
  3. Enable Distance rates and Taxes.
  4. Go to Distance rates > Settings > Enable Track tax.
  5. Add a distance rate and leave Tax rate and Tax reclaimable on fields empty.
  6. Submit a distance expense in the workspace chat.
  7. Go to transaction thread.
  8. Click Rate.
  9. Select the other distance rate that does not have Tax rate and Tax reclaimable on.

Expected Result:

App will not crash.

Actual Result:

App crashes after selecting distance rate that has no Tax rate and Tax reclaimable on.

Crash happens when the rate has

  • no tax rate and no tax reclaimable
  • No tax rate and has tax reclaimable

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

2009_1.txt

Bug6610205_1726857577201.ScreenRecording_09-21-2024_02-34-46_1.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

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
Copy link
Author

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

@IuliiaHerets
Copy link
Author

@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

@etCoderDysto
Copy link
Contributor

Proposal

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

Distance - App crashes after selecting rate that has no Tax rate & Tax reclaimable on

What is the root cause of that problem?

We are not adding optional chaining before value below

originalMessage.taxRate = transactionChanges?.taxCode && policy?.taxRates?.taxes[transactionChanges?.taxCode].value;

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

Add optional chaining before value

originalMessage.taxRate = transactionChanges?.taxCode && policy?.taxRates?.taxes[transactionChanges?.taxCode]?.value;

What alternative solutions did you explore? (Optional)

@nkdengineer
Copy link
Contributor

Proposal

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

App crashes after selecting distance rate that has no Tax rate and Tax reclaimable on.

What is the root cause of that problem?

When track tax is enabled and we select the rate that has no tax rate, taxRateExternalID here is -1

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, taxAmount, taxRateExternalID);

Then here we passed it to transactionChanges because updatedTaxCode is not empty

App/src/libs/actions/IOU.ts

Lines 3176 to 3180 in 513e6b3

const transactionChanges: TransactionChanges = {
customUnitRateID: rateID,
...(typeof updatedTaxAmount === 'number' ? {taxAmount: updatedTaxAmount} : {}),
...(updatedTaxCode ? {taxCode: updatedTaxCode} : {}),
};

Then the app crashes here because policy?.taxRates?.taxes[transactionChanges?.taxCode] is undefined with transactionChanges?.taxCode as -1

App/src/libs/ReportUtils.ts

Lines 3475 to 3477 in 8383570

if (didTaxCodeChange && !didAmountOrCurrencyChange) {
originalMessage.oldTaxRate = policy?.taxRates?.taxes[TransactionUtils.getTaxCode(oldTransaction)]?.value;
originalMessage.taxRate = transactionChanges?.taxCode && policy?.taxRates?.taxes[transactionChanges?.taxCode].value;

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

Safely get the value field here

policy?.taxRates?.taxes[transactionChanges?.taxCode]?.value

App/src/libs/ReportUtils.ts

Lines 3475 to 3477 in 8383570

if (didTaxCodeChange && !didAmountOrCurrencyChange) {
originalMessage.oldTaxRate = policy?.taxRates?.taxes[TransactionUtils.getTaxCode(oldTransaction)]?.value;
originalMessage.taxRate = transactionChanges?.taxCode && policy?.taxRates?.taxes[transactionChanges?.taxCode].value;

And we can fallback taxRateExternalID as empty string

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, taxAmount, taxRateExternalID);

or add a check here to not add taxCode to transactionChanges if it's -1

)updatedTaxCode && updatedTaxCode !== '-1') ? {taxCode: updatedTaxCode} : {}

App/src/libs/actions/IOU.ts

Lines 3176 to 3180 in 513e6b3

const transactionChanges: TransactionChanges = {
customUnitRateID: rateID,
...(typeof updatedTaxAmount === 'number' ? {taxAmount: updatedTaxAmount} : {}),
...(updatedTaxCode ? {taxCode: updatedTaxCode} : {}),
};

What alternative solutions did you explore? (Optional)

NA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

4 participants