[HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate #41594

m-natarajan · 2024-05-03T15:58:19Z

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.70-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause Internal Team
Slack conversation:

Action Performed:

Precondition:

There is only one default distance rate in the workspace.

Go to staging.new.expensify.com
Go to Account settings > Workspaces > any workspace.
Go to Distance rates.
Go offline.
Add a distance rate,
Delete the default workspace rate (not the newly-added rate).
Click on the checkbox of the newly added distance rate.
Select Delete option from the dropdown.
Note that the distance rate cannot be deleted because there must be at least one distance rate.
Click on the distance rate row.
Delete it from the three-dot menu.

Expected Result:

App will show the same pop-up in Step 9 that the distance rate cannot be deleted as there is only one distance rate.

Actual Result:

Distance rate can be deleted from 3-dot menu when there is only one distance rate.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6470164_1714748306876.bandicam_2024-05-03_22-51-33-950.mp4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0144c3fd2f5960b3c2
Upwork Job ID: 1786441064248098816
Last Price Increase: 2024-05-03
Automatic offers:

cretadn22 | Contributor | 0

Issue Owner

Current Issue Owner: @sobitneupane / @kevinksullivan

melvin-bot · 2024-05-03T15:58:31Z

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

m-natarajan · 2024-05-03T15:59:12Z

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

m-natarajan · 2024-05-03T15:59:16Z

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

melvin-bot · 2024-05-03T17:01:56Z

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

melvin-bot · 2024-05-03T17:02:01Z

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

Krishna2323 · 2024-05-03T17:32:36Z

Proposal

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

Distance rate can be deleted from 3-dot menu when there is only one rate

What is the root cause of that problem?

The condition canDeleteRate isn't correct.

App/src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

Lines 50 to 51 in 75b3863

    
           const canDeleteRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1 || !rate.enabled; 
        
           const canDisableRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1;

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

Remove || !rate.enabled check because user should not be able to delete the receipt until there is one more rate with enabled state. Also, I think we should canDeleteRate and canDisableRate filter out the rates that has pending action value delete. We might also want to filter out the rates with pending action value add. If we are going to filter out rates with pending action value add then we would also need to update that in PolicyDistanceRatesPage.

Optionally, we can filter out as stated above without removing || !rate.enabled check

Also check for other policy pages that has similar functionality

What alternative solutions did you explore? (Optional)

Only change the canDisableRate condition to exclude the rates with pending action delete and also add if needed.

Krishna2323 · 2024-05-03T17:38:55Z

Proposal Upadated

Updated main solution

Krishna2323 · 2024-05-03T17:42:51Z

Proposal Upadated

Added alternative

cretadn22 · 2024-05-03T18:05:25Z

Proposal

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

Users can delete all distance rate

What is the root cause of that problem?

App/src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

Lines 50 to 51 in 387ab0e

    
           const canDeleteRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1 || !rate.enabled; 
        
           const canDisableRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1;

We don't filter out the distance rate with a pendingAction of DELETE

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

See the logic in PolicyDistanceRatesPage, It works well

App/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx

Lines 66 to 68 in 387ab0e

    
           const canDisableOrDeleteSelectedRates = useMemo( 
        
               () => allSelectableRates.filter((rate: Rate) => !selectedDistanceRates.some((selectedRate) => selectedRate.customUnitRateID === rate.customUnitRateID)).some((rate) => rate.enabled), 
        
               [allSelectableRates, selectedDistanceRates],

To ensure consistency, we should implement the same solution for PolicyDistanceRatesPage: (but using the current rate instead of selectedDistanceRates)

Introduce a new variable

const allSelectableRates = Object.values(customUnit.rates).filter((rate) => rate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
const canDisableOrDeleteSelectedRates = allSelectableRates.filter((rate: Rate) => rateID !== rate.customUnitRateID).some((rate) => rate.enabled)

Utilize canDisableOrDeleteSelectedRates instead of canDeleteRate and canDisableRate, similar to what was done in the PolicyDistanceRatesPage

Moreover, creating a utility function could help maintain a DRY (Don't Repeat Yourself) codebase.

What alternative solutions did you explore? (Optional)

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.

jainilparikh · 2024-05-04T09:31:42Z

Proposal

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

User is able to delete ALL distance rates when application is offline. Expected scenario is that there should be at-least one distance rate and user should NOT be allowed to delete the last remaining distance rate.

What is the root cause of that problem?

Let's assume that there are N distance rates in the list. Since the user is offline, when the user deletes one of the rates, it become's "disabled" but internally it is still considered to be "enabled". Hence, this condition:

 const canDeleteRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1 || !rate.enabled;

More specifically:

Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length

continues to have a value of N and not N - 1. Because of this, when the user attempt's to delete the last rate, since the value of the above condition is still N, canDeleteRate will be false, hence allowing the user to delete the last entry.

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

set enabled as false in optimisticRates and failureRates in deletePolicyDistanceRates function in

App/src/libs/actions/Policy.ts

Lines 4792 to 4808 in 951c7b3

    
           function deletePolicyDistanceRates(policyID: string, customUnit: CustomUnit, rateIDsToDelete: string[]) { 
        
               const currentRates = customUnit.rates; 
        
               const optimisticRates: Record<string, Rate> = {}; 
        
               const successRates: Record<string, Rate> = {}; 
        
               const failureRates: Record<string, Rate> = {}; 
        
               for (const rateID of Object.keys(currentRates)) { 
        
                   if (rateIDsToDelete.includes(rateID)) { 
        
                       optimisticRates[rateID] = { 
        
                           ...currentRates[rateID], 
        
                           pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, 
        
                       }; 
        
                       failureRates[rateID] = { 
        
                           ...currentRates[rateID], 
        
                           pendingAction: null, 
        
                           errors: ErrorUtils.getMicroSecondOnyxError('common.genericErrorMessage'), 
        
                       };

What alternative solutions did you explore? (Optional)

Option 2:
Adding RED_BRICK_ROAD_PENDING_ACTION in the existing canDeleteRate and canDisableRate conditions

Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled && distanceRate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length

Option 3:
Using a logic similar to what's used in PolicyDistanceRatesPage by creating a allSelectableRates variable.

const allSelectableRates = Object.values(customUnit.rates).filter((rate) => rate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
const canDisableOrDeleteSelectedRates = allSelectableRates.filter((rate: Rate) => rateID !== rate.customUnitRateID).some((rate) => rate.enabled)

The problem with the above approaches however is that it does not solve the root of the problem, i.e setting the enabled flag to false for that rate.

melvin-bot · 2024-05-07T01:57:51Z

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

sobitneupane · 2024-05-07T09:23:11Z

Will review the proposals shortly.

sobitneupane · 2024-05-08T08:36:16Z

Thanks for the proposal everyone.

All the proposals are almost same. @Krishna2323 Your proposal needs few refinement. Can you please separate all the options as different solutions in your proposal. You can take this proposal as an example.

Why should we remove pending action add as well? Regarding!rate.enabled, I believe if the rate is disabled there is already an enabled rate (as there is always at least one enabled rate). So, we can safely remove the disabled rate.

sobitneupane · 2024-05-08T08:46:48Z

Thanks for the proposal @cretadn22

Your proposal looks good to be. Instead of canDisableOrDeleteSelectedRates, let's name the variable canDisableorDeleteRate and I believe simply adding rate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE condition in the following should be enough.

App/src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

Line 50 in 387ab0e

    
           const canDeleteRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled).length > 1 || !rate.enabled;

const canDisableorDeleteRate = Object.values(customUnit.rates).filter((distanceRate) => distanceRate.enabled && distanceRate.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).length > 1 || !rate.enabled;

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-05-08T08:47:00Z

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

Krishna2323 · 2024-05-08T08:55:24Z

@sobitneupane, isn't @cretadn22's proposal same as mine with small refactor?

sobitneupane · 2024-05-08T09:02:54Z

@Krishna2323 I would say your proposal is rather unclear. You have added multiple things in one place, many of which seem unnecessary.

Krishna2323 · 2024-05-08T09:11:28Z

Remove || !rate.enabled check because user should not be able to delete the receipt until there is one more rate with enabled state. Also, I think we should canDeleteRate and canDisableRate filter out the rates that has pending action value delete.

@sobitneupane, I think this is pretty clear, I mentioned about removing || !rate.enabled check and filtering out with pending action check.

I try to provide solutions for various scenarios because I've encountered situations where the expected outcome changed during discussions, and those who were actively involved at the time took advantage of that.

cc: @tgolen

melvin-bot · 2024-05-23T22:29:41Z

Triggered auto assignment to @kevinksullivan (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 · 2024-06-06T12:32:43Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-06-06T12:32:48Z

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

add optional chaining #42502

If no regressions arise, payment will be issued on 2024-06-13. 🎊

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

@sobitneupane requires payment through NewDot Manual Requests
@cretadn22 requires payment automatic offer (Contributor)

melvin-bot · 2024-06-06T12:32:49Z

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:

[@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
[@sobitneupane] 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:
[@sobitneupane] 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:
[@sobitneupane] Determine if we should create a regression test for this bug.
[@sobitneupane] 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.
[@slafortune / @kevinksullivan] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

slafortune · 2024-06-13T18:41:10Z

@cretadn22 Paid ✔️
@sobitneupane can you please complete the checklist?

melvin-bot · 2024-06-17T18:10:10Z

@tgolen, @slafortune, @sobitneupane, @cretadn22 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

tgolen · 2024-06-19T13:44:33Z

bump @sobitneupane to complete the checklist, please.

sobitneupane · 2024-06-20T09:28:06Z

Regression Test Proposal

Precondition: There is only one default distance rate in the workspace.

Go to Settings > Workspaces > any workspace.
Go to Distance rates.
Go offline.
Add a distance rate,
Delete the default workspace rate (not the newly-added rate).
Click on the checkbox of the newly added distance rate.
Select Delete option from the dropdown.
Verify that the distance rate cannot be deleted because there must be at least one distance rate.
Click on the distance rate row.
Delete it from the three-dot menu.
Verify that the distance rate cannot be deleted because there must be at least one distance rate.

Do we agree 👍 or 👎

sobitneupane · 2024-06-20T09:31:51Z

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:

[@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

#38237

[@sobitneupane] 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:

#38237 (comment)

[@sobitneupane] Determine if we should create a regression test for this bug.

yes

[@sobitneupane] 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.

#41594 (comment)

slafortune · 2024-06-20T20:04:41Z

@sobitneupane C+ is due $250 via NewDot

JmillsExpensify · 2024-06-22T16:54:47Z

$250 approved for @sobitneupane

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 3, 2024

melvin-bot bot assigned slafortune May 3, 2024

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

melvin-bot bot changed the title ~~Distance rate can be deleted from 3-dot menu when there is only one rate~~ [$250] Distance rate can be deleted from 3-dot menu when there is only one rate May 3, 2024

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

melvin-bot bot assigned sobitneupane May 3, 2024

slafortune added this to [#whatsnext] #wave-collect May 3, 2024

slafortune moved this to Release 1: Spring 2024 (May) in [#whatsnext] #wave-collect May 3, 2024

melvin-bot bot added the Overdue label May 7, 2024

melvin-bot bot removed the Overdue label May 7, 2024

melvin-bot bot assigned tgolen May 8, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 23, 2024

slafortune added Weekly KSv2 and removed Daily KSv2 labels May 23, 2024

slafortune self-assigned this May 23, 2024

bfitzexpensify mentioned this issue May 30, 2024

[$250] WS - App crashes when changing workspace ID to invalid ID while distance rate RHP is open #40481

Closed

6 tasks

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 6, 2024

melvin-bot bot changed the title ~~[$250] Distance rate can be deleted from 3-dot menu when there is only one rate~~ [HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate Jun 6, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jun 6, 2024

slafortune unassigned kevinksullivan Jun 7, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 12, 2024

melvin-bot bot added the Overdue label Jun 17, 2024

melvin-bot bot removed the Overdue label Jun 19, 2024

sobitneupane mentioned this issue Jun 20, 2024

feat: policy distance rates edit #38237

Merged

50 tasks

slafortune closed this as completed Jun 20, 2024

github-project-automation bot moved this from Release 1: Spring 2024 (May) to Done in [#whatsnext] #wave-collect Jun 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate #41594

[HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate #41594

m-natarajan commented May 3, 2024 •

edited by slafortune

Loading

melvin-bot bot commented May 3, 2024

m-natarajan commented May 3, 2024

m-natarajan commented May 3, 2024

melvin-bot bot commented May 3, 2024

melvin-bot bot commented May 3, 2024

Krishna2323 commented May 3, 2024 •

edited

Loading

Krishna2323 commented May 3, 2024

Krishna2323 commented May 3, 2024

cretadn22 commented May 3, 2024

jainilparikh commented May 4, 2024 •

edited

Loading

melvin-bot bot commented May 7, 2024

sobitneupane commented May 7, 2024

sobitneupane commented May 8, 2024 •

edited

Loading

sobitneupane commented May 8, 2024 •

edited

Loading

melvin-bot bot commented May 8, 2024

Krishna2323 commented May 8, 2024

sobitneupane commented May 8, 2024 •

edited

Loading

Krishna2323 commented May 8, 2024

melvin-bot bot commented May 23, 2024

melvin-bot bot commented Jun 6, 2024

melvin-bot bot commented Jun 6, 2024

melvin-bot bot commented Jun 6, 2024

slafortune commented Jun 13, 2024 •

edited

Loading

melvin-bot bot commented Jun 17, 2024

tgolen commented Jun 19, 2024

sobitneupane commented Jun 20, 2024 •

edited

Loading

sobitneupane commented Jun 20, 2024

slafortune commented Jun 20, 2024

JmillsExpensify commented Jun 22, 2024

[HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate #41594

[HOLD for payment 2024-06-13] [$250] Distance rate can be deleted from 3-dot menu when there is only one rate #41594

Comments

m-natarajan commented May 3, 2024 • edited by slafortune Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented May 3, 2024

m-natarajan commented May 3, 2024

m-natarajan commented May 3, 2024

melvin-bot bot commented May 3, 2024

melvin-bot bot commented May 3, 2024

Krishna2323 commented May 3, 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)

Krishna2323 commented May 3, 2024

Proposal Upadated

Krishna2323 commented May 3, 2024

Proposal Upadated

cretadn22 commented May 3, 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)

jainilparikh commented May 4, 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)

melvin-bot bot commented May 7, 2024

sobitneupane commented May 7, 2024

sobitneupane commented May 8, 2024 • edited Loading

sobitneupane commented May 8, 2024 • edited Loading

melvin-bot bot commented May 8, 2024

Krishna2323 commented May 8, 2024

sobitneupane commented May 8, 2024 • edited Loading

Krishna2323 commented May 8, 2024

melvin-bot bot commented May 23, 2024

melvin-bot bot commented Jun 6, 2024

melvin-bot bot commented Jun 6, 2024

melvin-bot bot commented Jun 6, 2024

slafortune commented Jun 13, 2024 • edited Loading

melvin-bot bot commented Jun 17, 2024

tgolen commented Jun 19, 2024

sobitneupane commented Jun 20, 2024 • edited Loading

sobitneupane commented Jun 20, 2024

slafortune commented Jun 20, 2024

JmillsExpensify commented Jun 22, 2024

m-natarajan commented May 3, 2024 •

edited by slafortune

Loading

Krishna2323 commented May 3, 2024 •

edited

Loading

jainilparikh commented May 4, 2024 •

edited

Loading

sobitneupane commented May 8, 2024 •

edited

Loading

sobitneupane commented May 8, 2024 •

edited

Loading

sobitneupane commented May 8, 2024 •

edited

Loading

slafortune commented Jun 13, 2024 •

edited

Loading

sobitneupane commented Jun 20, 2024 •

edited

Loading