[$250] Tax - Enable/disable option shows rates text in plural instead rate #51017

lanitochka17 · 2024-10-17T13:24:45Z

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.50-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Go to https://staging.new.expensify.com/home
Tap profile icon - workspaces -- workspace
Tap more features - enable taxes
Create few tax rates
Disable a tax rate
Long press and select one disabled and one enabled tax rate
Tap dropdown on top and view options

Expected Result:

Enable/disable option must not show rates text in plural and it must show rate

Actual Result:

Enable/disable option shows rates text in plural instead rate when only one enabled and disabled rate are selected

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6637276_1729142094339.rate.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021846961178055724837
Upwork Job ID: 1846961178055724837
Last Price Increase: 2024-10-24

Issue Owner

Current Issue Owner: @getusha

melvin-bot · 2024-10-17T13:24:57Z

Triggered auto assignment to @isabelastisser (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 · 2024-10-17T13:25:12Z

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

github-actions · 2024-10-17T13:33:20Z

rzghnsy Your proposal will be dismissed because you did not follow the proposal template.

nyomanjyotisa · 2024-10-17T13:34:56Z

Edited by proposal-police: This proposal was edited at 2024-10-17 13:37:10 UTC.

Proposal

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

Tax - Enable/disable option shows rates text in plural instead rate

What is the root cause of that problem?

We show plural text base on isMultiple variable, without checking the enable/disable status of the selected tax

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

Line 186 in 987ff1b

const isMultiple = selectedTaxesIDs.length > 1;

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

Line 201 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),

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

Line 211 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),

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

Create new variables to check is there multiple enabled tax selected and is there multiple disabled tax selected

        const isMultipleEnabled = selectedTaxesIDs.filter((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled).length > 1;
        const isMultipleDisabled = selectedTaxesIDs.filter((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled).length > 1;

// `Disable rates` when at least one enabled rate is selected.
        if (selectedTaxesIDs.some((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled)) {
            options.push({
                icon: Expensicons.DocumentSlash,
                text: isMultipleEnabled ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),
                value: CONST.POLICY.BULK_ACTION_TYPES.DISABLE,
                onSelected: () => toggleTaxes(false),
            });
        }

        // `Enable rates` when at least one disabled rate is selected.
        if (selectedTaxesIDs.some((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled)) {
            options.push({
                icon: Expensicons.Document,
                text: isMultipleDisabled ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),
                value: CONST.POLICY.BULK_ACTION_TYPES.ENABLE,
                onSelected: () => toggleTaxes(true),
            });
        }

What alternative solutions did you explore? (Optional)

Nodebrute · 2024-10-17T13:45:10Z

Proposal

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

Enable/disable option shows rates text in plural instead rate

What is the root cause of that problem?

Here we only see if selected tags are more than 1 instead of checking disable/enable tags count

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

Line 186 in 987ff1b

const isMultiple = selectedTaxesIDs.length > 1;

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

We can create 2 new variables

        const disableTaxes = selectedTaxesIDs.filter((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled);
        const enabledTaxes = selectedTaxesIDs.filter((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled);

and then in here we can check

   if (disableTaxes.length > 0)

and in here we can change this to

 text: disableTaxes.length === 1 ?  translate('workspace.taxes.actions.disable') : translate('workspace.taxes.actions.disableMultiple'),

We can do same for enable tags

What alternative solutions did you explore? (Optional)

ChavdaSachin · 2024-10-17T13:51:09Z

Edited by proposal-police: This proposal was edited at 2024-10-17 13:57:28 UTC.

Proposal

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

Tax - Enable/disable option shows rates text in plural instead rate

What is the root cause of that problem?

Irrespective of number of available rates to enable/disable, we check only for number of selections here.

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

Line 186 in 987ff1b

const isMultiple = selectedTaxesIDs.length > 1;

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

Line 201 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),

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

Line 211 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),

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

Rather create new objects enabledRates and disabledRates

        const enbledRates = selectedTaxesIDs.filter((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled);
        const disabledRates = selectedTaxesIDs.filter((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled);

now use enabledRates?.length here to saw the option

        if (enabledRates?.length) {

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

Line 198 in 987ff1b

    
           if (selectedTaxesIDs.some((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled)) {

and use enabledRates?.length > 1 instead of isMultiple here

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

Line 201 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),

Similar for Enable option use 'disabledRates?.length' here.

        if (disabledRates?.length) {

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

Line 208 in 987ff1b

    
           if (selectedTaxesIDs.some((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled)) {

And use disabledRates?.length > 1 instead of isMultiple here.

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

Line 211 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),

This way we optimize the solution and do not have to filter array twice.

What alternative solutions did you explore? (Optional)

And since we make use of this logic only to determine length of selections, we could optimize it further.

Only create 1 object which holds count of enabled selections and use it to determine length of disabledSelectionCount.

        const enabledRatesCount = selectedTaxesIDs.filter((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled).length;
        const disabledRatesCount = selectedTaxesIDs.length - enabledRatesCount;

and use those variables for check here

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

Line 198 in 987ff1b

    
           if (selectedTaxesIDs.some((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled)) {

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

Line 201 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),

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

Line 208 in 987ff1b

    
           if (selectedTaxesIDs.some((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled)) {

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

Line 211 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

ChavdaSachin · 2024-10-17T13:58:56Z

Updated Proposal
Added perfectly optimized way to solve the issue as alternate solution.

twilight2294 · 2024-10-17T14:06:38Z

Proposal

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

Enable/disable option shows rates text in plural instead rate

What is the root cause of that problem?

We check if multiple are selected:

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

Line 186 in 987ff1b

const isMultiple = selectedTaxesIDs.length > 1;

If so then without checking if there are multiple selected or not, we directly show plural form of texts:

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

Line 201 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.disableMultiple') : translate('workspace.taxes.actions.disable'),

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

Line 211 in 987ff1b

    
           text: isMultiple ? translate('workspace.taxes.actions.enableMultiple') : translate('workspace.taxes.actions.enable'),

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

First we need to get the disabled and enabled taxes:

        const enabledRatesCount = selectedTaxesIDs.filter((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled).length;
        const disabledRatesCount = selectedTaxesIDs.length - enabledRatesCount;

Now according to our Readme:

Here’s an example of how to implement plural translations:

messages: () => ({ zero: 'No messages', one: 'One message', two: 'Two messages', few: (count) => ${count} messages, many: (count) => You have ${count} messages, other: (count) => You have ${count} unread messages, })

So we should not do condition rendering here, instead we should update the text to match this plural forms:

First update the en.ts and es.ts file to unify the multi tags messages:

           disableTaxTitle: () => ({
                one: 'Disable rate',
                other: 'Disable rates',
            }),
            enableTaxTitle: () => ({
                one: 'enable rate',
                other: 'enable rates',
            }),

        // `Disable rates` when at least one enabled rate is selected.
        if (selectedTaxesIDs.some((taxID) => !policy?.taxRates?.taxes[taxID]?.isDisabled)) {
            options.push({
                icon: Expensicons.DocumentSlash,
                text: translate('workspace.taxes.actions.disableTaxTitle', {count: disabledRatesCount}),
                value: CONST.POLICY.BULK_ACTION_TYPES.DISABLE,
                onSelected: () => toggleTaxes(false),
            });
        }

        // `Enable rates` when at least one disabled rate is selected.
        if (selectedTaxesIDs.some((taxID) => policy?.taxRates?.taxes[taxID]?.isDisabled)) {
            options.push({
                icon: Expensicons.Document,
                text: translate('workspace.taxes.actions.enableTaxTitle', {count: enabledRatesCount}),
                value: CONST.POLICY.BULK_ACTION_TYPES.ENABLE,
                onSelected: () => toggleTaxes(true),
            });
        }

We can even optimise the conditions during PR phase

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-10-17T17:07:16Z

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

melvin-bot · 2024-10-17T17:07:21Z

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

melvin-bot · 2024-10-22T18:17:48Z

@isabelastisser, @getusha Huh... This is 4 days overdue. Who can take care of this?

isabelastisser · 2024-10-22T19:27:20Z

@getusha, can you please review the proposals above and provide an update? Thanks!

isabelastisser · 2024-10-23T19:31:36Z

Bump @getusha for an update, thanks!

getusha · 2024-10-24T11:11:31Z

Reviewing

getusha · 2024-10-24T11:31:31Z

there are 3 cases we need to handle

1 disabled
1 enabled rate
- Disable rate
- Enable rate

1 disabled
2 enabled rate
- Disable rates
- Enable rate

2 disabled
1 enabled
- Disable rate
- Enable rates

melvin-bot · 2024-10-24T11:32:12Z

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

getusha · 2024-10-24T11:32:40Z

Not sure if this is worth fixing though @techievivek what do you think?

twilight2294 · 2024-10-24T11:35:27Z

Not sure if this is worth fixing though @techievivek what do you think?

@getusha , this is supposed to be fixed, we recently fixed a similar issue on members page as well:

fix: Plural Remove Members on the workspace when we only have one #50488

c.c. @techievivek

melvin-bot · 2024-10-24T16:00:05Z

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

isabelastisser · 2024-10-25T14:56:15Z

@twilight2294 @getusha, to clarify, is this issue fixed?

#51017 (comment)

twilight2294 · 2024-10-25T14:57:42Z

@twilight2294 @getusha, to clarify, is this issue fixed?

No, this bug still exits, I pointed out a PR which fixed a similar bug in the workspace members page

twilight2294 · 2024-10-25T14:58:05Z

we are waiting for @techievivek to reply

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 17, 2024

melvin-bot bot assigned isabelastisser Oct 17, 2024

isabelastisser added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Oct 17, 2024

melvin-bot bot changed the title ~~Tax - Enable/disable option shows rates text in plural instead rate~~ [$250] Tax - Enable/disable option shows rates text in plural instead rate Oct 17, 2024

melvin-bot bot assigned getusha Oct 17, 2024

melvin-bot bot added the Overdue label Oct 22, 2024

melvin-bot bot removed the Overdue label Oct 24, 2024

melvin-bot bot assigned techievivek Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$250] Tax - Enable/disable option shows rates text in plural instead rate #51017

[$250] Tax - Enable/disable option shows rates text in plural instead rate #51017

lanitochka17 commented Oct 17, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 17, 2024

lanitochka17 commented Oct 17, 2024

github-actions bot commented Oct 17, 2024

nyomanjyotisa commented Oct 17, 2024 •

edited

Loading

Nodebrute commented Oct 17, 2024

ChavdaSachin commented Oct 17, 2024 •

edited by github-actions bot

Loading

ChavdaSachin commented Oct 17, 2024

twilight2294 commented Oct 17, 2024

melvin-bot bot commented Oct 17, 2024

melvin-bot bot commented Oct 17, 2024

melvin-bot bot commented Oct 22, 2024

isabelastisser commented Oct 22, 2024

isabelastisser commented Oct 23, 2024

getusha commented Oct 24, 2024

getusha commented Oct 24, 2024

melvin-bot bot commented Oct 24, 2024

getusha commented Oct 24, 2024

twilight2294 commented Oct 24, 2024

melvin-bot bot commented Oct 24, 2024

isabelastisser commented Oct 25, 2024

twilight2294 commented Oct 25, 2024

twilight2294 commented Oct 25, 2024

[$250] Tax - Enable/disable option shows rates text in plural instead rate #51017

[$250] Tax - Enable/disable option shows rates text in plural instead rate #51017

Comments

lanitochka17 commented Oct 17, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 17, 2024

lanitochka17 commented Oct 17, 2024

github-actions bot commented Oct 17, 2024

nyomanjyotisa commented Oct 17, 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?

What alternative solutions did you explore? (Optional)

Nodebrute commented Oct 17, 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)

ChavdaSachin commented Oct 17, 2024 • edited by github-actions bot 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?

What alternative solutions did you explore? (Optional)

ChavdaSachin commented Oct 17, 2024

twilight2294 commented Oct 17, 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)

melvin-bot bot commented Oct 17, 2024

melvin-bot bot commented Oct 17, 2024

melvin-bot bot commented Oct 22, 2024

isabelastisser commented Oct 22, 2024

isabelastisser commented Oct 23, 2024

getusha commented Oct 24, 2024

getusha commented Oct 24, 2024

melvin-bot bot commented Oct 24, 2024

getusha commented Oct 24, 2024

twilight2294 commented Oct 24, 2024

melvin-bot bot commented Oct 24, 2024

isabelastisser commented Oct 25, 2024

twilight2294 commented Oct 25, 2024

twilight2294 commented Oct 25, 2024

lanitochka17 commented Oct 17, 2024 •

edited by melvin-bot bot

Loading

nyomanjyotisa commented Oct 17, 2024 •

edited

Loading

ChavdaSachin commented Oct 17, 2024 •

edited by github-actions bot

Loading