[HOLD for payment 2024-10-14] [$250] [P2P Distance] App crashes after selecting rate that has no Tax rate & Tax reclaimable on

@aimane-chnaif

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

Action Performed:

Launch New Expensify app.
Create a new workspace.
Enable Distance rates and Taxes.
Go to Distance rates > Settings > Enable Track tax.
Add a distance rate and leave Tax rate and Tax reclaimable on fields empty.
Submit a distance expense in the workspace chat.
Go to transaction thread.
Click Rate.
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:

Screenshots/Videos

2009_1.txt

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021838672839174830384
Upwork Job ID: 1838672839174830384
Last Price Increase: 2024-10-01
Automatic offers:

nkdengineer | Contributor | 104224449

Issue Owner

Current Issue Owner: @aimane-chnaif

@OfstadC

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.

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

@OfstadC

@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

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

App/src/libs/ReportUtils.ts

Line 3477 in 513e6b3

    
           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)

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

App/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx

Lines 95 to 107 in 513e6b3

    
               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

App/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx

Lines 95 to 107 in 513e6b3

    
               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

Added to Wave Collect - will confirm before adding External

@aimane-chnaif

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.45-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: App crashes after selecting rate #50040

If no regressions arise, payment will be issued on 2024-10-14. 🎊

For reference, here are some details about the assignees on this issue:

@aimane-chnaif requires payment (Needs manual offer from BZ)
@nkdengineer requires payment automatic offer (Contributor)

@aimane-chnaif

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
[@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@OfstadC] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@aimane-chnaif

@aimane-chnaif Please complete the BZ checklist so I can issue payment on the 14th. Thank you! 😃

The PR that introduced the bug has been identified. Link to the PR: feat: add edit fields for tax tracking #33927
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: feat: add edit fields for tax tracking #33927 (comment)
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

Regression Test Proposal

Create a new workspace.
Enable Distance rates and Taxes.
Go to Distance rates > Settings > Enable Track tax.
Add a distance rate and leave Tax rate and Tax reclaimable on fields empty.
Submit a distance expense in the workspace chat.
Go to transaction thread.
Click Rate.
Select the other distance rate that does not have Tax rate and Tax reclaimable on.
Verify that app does not crash

@aimane-chnaif

Thanks @aimane-chnaif ! Could you please accept this offer so I can issue payment? 😃

@aimane-chnaif

Payment Summary

@aimane-chnaif ~~pending offer in Upwork~~ paid $250 via Upwork
@nkdengineer paid $250 via Upwork

@aimane-chnaif

Will close once @aimane-chnaif accepts Upwork offer 😃

Navigation Menu

Repository navigation

[HOLD for payment 2024-10-14] [$250] [P2P Distance] App crashes after selecting rate that has no Tax rate & Tax reclaimable on #49556

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

20 remaining items

Regression Test Proposal

Payment Summary

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	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);

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

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

[HOLD for payment 2024-10-14] [$250] [P2P Distance] App crashes after selecting rate that has no Tax rate & Tax reclaimable on #49556

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Sep 20, 2024

IuliiaHerets commented on Sep 20, 2024

IuliiaHerets commented on Sep 20, 2024

etCoderDysto commented on Sep 20, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

nkdengineer commented on Sep 20, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

OfstadC commented on Sep 23, 2024

20 remaining items

melvin-bot commented on Oct 7, 2024

melvin-bot commented on Oct 7, 2024

OfstadC commented on Oct 10, 2024

aimane-chnaif commented on Oct 14, 2024

Regression Test Proposal

OfstadC commented on Oct 14, 2024

OfstadC commented on Oct 14, 2024

Payment Summary

OfstadC commented on Oct 17, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions