[$500] [Track Tax] App crashes when reselecting the same tax in confirmation page #39167

izarutskaya · 2024-03-28T11:44:27Z

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: 1.4.57-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

User is an employee of Collect workspace.
The Collect workspace has at least two tax rates.

Go to staging.new.expensify.com.
Go to workspace chat.
Start manual request flow.
In confirmation page, tap Tax rate.
Select another tax.
Tap Tax rate again.
Tap on the same tax rate in Step 5.

Expected Result:

App does not crash.

Actual Result:

App crashes.
On web, it shows console error.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Bug6429833_1711613479780.Screen_Recording_20240328_160725_New_Expensify.mp4

Bug6429833_1711613479774!QA.txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0180f0901c994bc80b
Upwork Job ID: 1775666188401147904
Last Price Increase: 2024-04-03

melvin-bot · 2024-03-28T11:44:39Z

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

izarutskaya · 2024-03-28T11:45:45Z

We think this issue might be related to the #collect project.

ShridharGoel · 2024-03-28T11:51:38Z

Proposal

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

App crashes when reselecting the same tax in confirmation page

What is the root cause of that problem?

If _.find() doesn't find any element with taxRate.modifiedName === selectedTaxRate, it will return undefined. Then, when we try to access the value property of undefined, it throws an error because undefined does not have a property named value.

const getTaxAmount = (taxRates, selectedTaxRate, amount) => {
    const percentage = _.find(OptionsListUtils.transformedTaxRates(taxRates), (taxRate) => taxRate.modifiedName === selectedTaxRate).value;
    return TransactionUtils.calculateTaxAmount(percentage, amount);
};

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

To avoid this crash, we should add a check to see if the _.find() call returns undefined before accessing the value property.

const foundTaxRate = _.find(OptionsListUtils.transformedTaxRates(taxRates), (taxRate) => taxRate.modifiedName === selectedTaxRate);
const percentage = foundTaxRate ? foundTaxRate.value : defaultValue; // Provide a default value if taxRate is not found

abzokhattab · 2024-03-28T13:35:01Z

Proposal

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

App crashes when reselecting the same tax in confirmation page

What is the root cause of that problem?

when selecting a tax in tax picker, we update the name of the selected tax to contain the Default word which is not correct. the default is specified by the admin in the tax page and thus the value shouldn't change after selecting another tax in the tax picker..

here we use getTaxRatesSection and wrongly pass the defaultTaxKey as selectedTaxRate that's why the name changes and causes the find not being able to find a tax with that name

App/src/components/TaxPicker.tsx

Lines 55 to 58 in 062d53c

    
           const sections = useMemo( 
        
               () => OptionsListUtils.getTaxRatesSection(taxRates, selectedOptions as OptionsListUtils.Category[], searchValue, selectedTaxRate), 
        
               [taxRates, searchValue, selectedOptions, selectedTaxRate],

App/src/libs/OptionsListUtils.ts

Lines 1268 to 1271 in 58515c5

    
           function getTaxRatesSection(taxRates: TaxRatesWithDefault | undefined, selectedOptions: Category[], searchInputValue: string, defaultTaxKey?: string): CategorySection[] { 
        
               const policyRatesSections = []; 
        
               const taxes = transformedTaxRates(taxRates, defaultTaxKey);

App/src/libs/OptionsListUtils.ts

Lines 1236 to 1237 in 58515c5

    
           function transformedTaxRates(taxRates: TaxRatesWithDefault | undefined, defaultKey?: string): Record<string, TaxRate> { 
        
               const defaultTaxKey = defaultKey ?? taxRates?.defaultExternalID;

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

we need to remove the selectedTaxRate when calling the getTaxRatesSection function here because the transformedTaxRates will use the taxRates?.defaultExternalID if the defaultTax is not passed:

    const sections = useMemo(() => OptionsListUtils.getTaxRatesSection(taxRates, selectedOptions as OptionsListUtils.Category[], searchValue), [taxRates, searchValue, selectedOptions]);

POC

Screen.Recording.2024-03-28.at.3.38.08.PM.mov

bernhardoj · 2024-03-28T13:36:29Z

Proposal

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

The app crashes or does nothing on the web when selecting a selected tax rate.

What is the root cause of that problem?

When we select a tax rate, it will try to find the selected tax rate on the policy tax rates collection by its name.

App/src/pages/iou/request/step/IOURequestStepTaxRatePage.js

Lines 43 to 46 in 4fbd218

    
           const getTaxAmount = (taxRates, selectedTaxRate, amount) => { 
        
               const percentage = _.find(OptionsListUtils.transformedTaxRates(taxRates), (taxRate) => taxRate.modifiedName === selectedTaxRate).value; 
        
               return TransactionUtils.calculateTaxAmount(percentage, amount); 
        
           };

Every time we select a tax, it will append a • Default text to be shown.

App/src/components/TaxPicker.tsx

Line 57 in 4fbd218

    
           () => OptionsListUtils.getTaxRatesSection(taxRates, selectedOptions as OptionsListUtils.Category[], searchValue, selectedTaxRate),

App/src/libs/OptionsListUtils.ts

Lines 1219 to 1221 in 4fbd218

    
           function transformedTaxRates(taxRates: TaxRatesWithDefault | undefined, defaultKey?: string): Record<string, TaxRate> { 
        
               const defaultTaxKey = defaultKey ?? taxRates?.defaultExternalID; 
        
               const getModifiedName = (data: TaxRate, code: string) => `${data.name} (${data.value})${defaultTaxKey === code ? ` • ${Localize.translateLocal('common.default')}` : ''}`;

It doesn't alter the real tax name, so when we try to find the tax, it can't find it. For example, if we have tax rate test (2%) and it's selected, comparing test (2%) with test (2%) • Default will always be false. That's why the code can't find anything and throws an error when trying to access the value property.

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

We can remove • Default from the name or add • Default when it's the selected tax, but I think the real problem here is that the • Default word shouldn't be appended to a selected tax. Default !== selected. The real problem is we are passing the selected tax rate as the default key,

App/src/components/TaxPicker.tsx

Lines 56 to 57 in 4fbd218

    
           const sections = useMemo( 
        
               () => OptionsListUtils.getTaxRatesSection(taxRates, selectedOptions as OptionsListUtils.Category[], searchValue, selectedTaxRate),

and I'm pretty sure that's because we are passing the default rate as the selected tax rate here,

App/src/pages/workspace/taxes/WorkspaceTaxesSettingsWorkspaceCurrency.tsx

Lines 57 to 58 in 4fbd218

    
           <TaxPicker 
        
               selectedTaxRate={policy?.taxRates?.defaultExternalID}

App/src/pages/workspace/taxes/WorkspaceTaxesSettingsForeignCurrency.tsx

Lines 57 to 58 in 4fbd218

    
           <TaxPicker 
        
               selectedTaxRate={policy?.taxRates?.foreignTaxDefault}

and it makes sense since for the above page (and foreign currency tax rate), we select the default value, so the selected is the same as the default for both of those pages. To fix it, we should introduce a new props to TaxPicker called defaultTaxKeyand we will use that instead of the selected value.

App/src/components/TaxPicker.tsx

Lines 56 to 57 in 4fbd218

    
           const sections = useMemo( 
        
               () => OptionsListUtils.getTaxRatesSection(taxRates, selectedOptions as OptionsListUtils.Category[], searchValue, selectedTaxRate),

abzokhattab · 2024-03-28T13:40:19Z

Proposal updated

POC video was added.

melvin-bot · 2024-04-03T23:26:25Z

Job added to Upwork: https://www.upwork.com/jobs/~0180f0901c994bc80b

sakluger · 2024-04-03T23:26:29Z

Added to the wave-collect project and added the external label.

melvin-bot · 2024-04-03T23:26:30Z

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

getusha · 2024-04-05T11:30:45Z

Reviewing proposals now.

melvin-bot · 2024-04-05T11:32:03Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

getusha · 2024-04-05T12:03:14Z

Not able to reproduce on main & staging it appears to be fixed.

melvin-bot · 2024-04-08T18:26:15Z

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

sakluger · 2024-04-09T22:39:30Z

Thanks for retesting @getusha. I'll close it for now.

izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 28, 2024

melvin-bot bot assigned sakluger Mar 28, 2024

melvin-bot bot added the Overdue label Apr 1, 2024

sakluger added the External Added to denote the issue can be worked on by a contributor label Apr 3, 2024

melvin-bot bot changed the title ~~Tax - App crashes when reselecting the same tax in confirmation page~~ [$500] Tax - App crashes when reselecting the same tax in confirmation page Apr 3, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 3, 2024

melvin-bot bot assigned getusha Apr 3, 2024

melvin-bot bot removed the Overdue label Apr 3, 2024

getusha mentioned this issue Apr 5, 2024

[PAID] [$250] Taxes - App crashes when visiting transaction thread with invalid tax rate on a fresh session #39695

Closed

6 tasks

trjExpensify changed the title ~~[$500] Tax - App crashes when reselecting the same tax in confirmation page~~ [$500] [Track Tax] App crashes when reselecting the same tax in confirmation page Apr 5, 2024

melvin-bot bot added the Overdue label Apr 8, 2024

sakluger closed this as completed Apr 9, 2024

melvin-bot bot removed the Overdue label Apr 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$500] [Track Tax] App crashes when reselecting the same tax in confirmation page #39167

[$500] [Track Tax] App crashes when reselecting the same tax in confirmation page #39167

izarutskaya commented Mar 28, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Mar 28, 2024

izarutskaya commented Mar 28, 2024

ShridharGoel commented Mar 28, 2024 •

edited

Loading

abzokhattab commented Mar 28, 2024 •

edited

Loading

bernhardoj commented Mar 28, 2024 •

edited

Loading

abzokhattab commented Mar 28, 2024 •

edited

Loading

melvin-bot bot commented Apr 3, 2024

sakluger commented Apr 3, 2024

melvin-bot bot commented Apr 3, 2024

getusha commented Apr 5, 2024

melvin-bot bot commented Apr 5, 2024

getusha commented Apr 5, 2024

melvin-bot bot commented Apr 8, 2024

sakluger commented Apr 9, 2024

[$500] [Track Tax] App crashes when reselecting the same tax in confirmation page #39167

[$500] [Track Tax] App crashes when reselecting the same tax in confirmation page #39167

Comments

izarutskaya commented Mar 28, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 28, 2024

izarutskaya commented Mar 28, 2024

ShridharGoel commented Mar 28, 2024 • edited Loading

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?

abzokhattab commented Mar 28, 2024 • edited Loading

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?

POC

bernhardoj commented Mar 28, 2024 • edited Loading

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?

abzokhattab commented Mar 28, 2024 • edited Loading

Proposal updated

melvin-bot bot commented Apr 3, 2024

sakluger commented Apr 3, 2024

melvin-bot bot commented Apr 3, 2024

getusha commented Apr 5, 2024

melvin-bot bot commented Apr 5, 2024

getusha commented Apr 5, 2024

melvin-bot bot commented Apr 8, 2024

sakluger commented Apr 9, 2024

izarutskaya commented Mar 28, 2024 •

edited by melvin-bot bot

Loading

ShridharGoel commented Mar 28, 2024 •

edited

Loading

abzokhattab commented Mar 28, 2024 •

edited

Loading

bernhardoj commented Mar 28, 2024 •

edited

Loading

abzokhattab commented Mar 28, 2024 •

edited

Loading