[$250][P2P Distance] - Disabled rate is not listed in the Rate list when it is still selected

[m-natarajan]

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: v9.0.17-0 Reproducible in staging?: y Reproducible in production?: no, new feature If this was caught during regression testing, add the test name, ID and link from TestRail: n/a Email or phone of affected tester (no customers): applausetester+kh050801@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

Action Performed:

Precondition:

• Workspace has some distance rates.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit a distance expense with Distance rate A.
4. Go to workspace settings > Distance rates.
5. Disable Distance rate A used in Step 3.
6. Go back to transaction thread of the distance expense.
7. Click Rate.

   Expected Result:

   The disabled rate will appear selected in the Rate list but grayed out (like Category and Tag when selected and disabled afterward).

Actual Result:

The disabled rate is not listed in the Rate list when it is still selected.

Workaround:

unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/7a2b9e90-faa2-460a-abfb-0f5ce5ef5f00

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01113b3b52e50f7f4e
• Upwork Job ID: 1821117740687763365
• Last Price Increase: 2024-08-14
• Automatic offers:
• dominictb | Contributor | 103608790

Issue Owner

Current Issue Owner: @mananjadhav

[melvin-bot[bot]]

Triggered auto assignment to @iwiznia (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot[bot]]

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

[github-actions[bot]]

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[BhuvaneshPatil]

Edited by proposal-police: This proposal was edited at 2024-08-06 14:00:06 UTC.

PR - https://github.com/Expensify/App/pull/40021

I can raise PR if needed

Proposal

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

Distance rate which is selected but after disabling, not shown.

What is the root cause of that problem?

We are filtering the disabled rates when showing the rates list in selector. https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/libs/DistanceRequestUtils.ts#L47-L50

https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L141-L144

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

We shall pass true as second argument that will give us disabled rates as well and we will also pass enabled value. https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L141-L144

Once that is done - we can filter the disabled rate in IOURequestStepDistanceRate as following. and pass isDisabled value so it will show selected but style will be different for selected but disabled value.

    const sections = Object.values(rates).filter(rate => {
        const isSelected = currentRateID ? currentRateID === rate.customUnitRateID : rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE
        return isSelected || rate?.enabled
    }).map((rate) => {
        const rateForDisplay = DistanceRequestUtils.getRateForDisplay(rate.unit, rate.rate, rate.currency, translate, toLocaleDigit);
        return {
            text: rate.name ?? rateForDisplay,
            alternateText: rate.name ? rateForDisplay : '',
            keyForList: rate.customUnitRateID,
            value: rate.customUnitRateID,
            isSelected: currentRateID ? currentRateID === rate.customUnitRateID : rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE,
            isDisabled: !rate.enabled
        };
    });

What alternative solutions did you explore? (Optional)

demo -

https://github.com/user-attachments/assets/bb89056a-84aa-4abd-9367-c913b86c2eac

[neil-marcellini]

Not a blocker, the rate field is still behind the beta.

[BhuvaneshPatil]

@neil-marcellini Are we planning to make it external?

[github-actions[bot]]

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

[dominictb]

Proposal

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

The disabled rate is not listed in the Rate list when it is still selected.

What is the root cause of that problem?

This logic: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L71-L81 does not return the disable rate.

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

In general, we need to make sure, when we disable rate A:

1. If A is selected, display A in list. And A will have a gray color.
2. Otherwise, do not display it.

Details:

1. We need to refactor the getMileageRates function, which will receive an additional param, named selectedRateID, and for each rate returned by that function, we add an additional prop, enabled: rate.enabled:

   +function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates = false, selectedRateID: string): Record<string, MileageRate> {
   Object.entries(distanceUnit.rates).forEach(([rateID, rate]) => {
   +      if (!includeDisabledRates && rate.enabled === false && selectedRateID !== rateID) {
           return;
       }
   
       mileageRates[rateID] = {
           ...
   +          enabled: rate.enabled,
       };
   });
   }

2. Remove this: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L141-L144 because it is redundant and use:

   const rates = DistanceRequestUtils.getMileageRates(policy, false, currentRateID)

   in here instead.

3. Finally, return additional prop isDisabled: !rate.enabled in: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L74 to get the grey-out color.

Note: In this page, we not only use the rates data to display the list, but also use it in: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L83 and https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L93 so rates data should be consistent whole the file.

What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[dylanexpensify]

made external

[melvin-bot[bot]]

@iwiznia, @mananjadhav, @dylanexpensify Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot[bot]]

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

[mananjadhav]

I'll review the proposals by tomorrow. I was OOO.

[melvin-bot[bot]]

@iwiznia, @mananjadhav, @dylanexpensify Huh... This is 4 days overdue. Who can take care of this?

[iwiznia]

@mananjadhav are you available or should we reassign this issue?

[mananjadhav]

I'll have this reviewed today.

[mananjadhav]

I was a little confused with the selectedRateID in this proposal. But after reviewing it thoroughly I understand now. Both the proposals identify the correct root cause, but I think it's best to use this proposal by @dominictb. It involves cleaning up the existing code as well.

🎀 👀 🎀

[melvin-bot[bot]]

Current assignee @iwiznia is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot[bot]]

@iwiznia @mananjadhav @dylanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[iwiznia]

@dominictb a couple questions:

Remove this: ... because it is redundant and use:

Don't we need that there? Otherwise this component will not update if onyx data changes, no?

Finally, return additional prop isDisabled: !rate.enabled in:

Why are we transforming enabled to isDisabled? Why not keep enabled?

[dominictb]

Don't we need that there? Otherwise this component will not update if onyx data changes, no?

• I think we already had the policy data is always up-to-date here: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L132 so we don't need: https://github.com/Expensify/App/blob/627ef1a982350287aaf79e5c08c5f25a7033187a/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx#L141

Why are we transforming enabled to isDisabled? Why not keep enabled?

• We only have a logic to gray out the option if it has isDisabled is true. So if need to convert enabled to isDisabled.

[BhuvaneshPatil]

Yes, I agree with @iwiznia points here.

1. I am not sure if we should modify getMileageRates() method, because it's being used at multiple places.
2. For unit, we use what is being used in workspace not individual rate. ~Can you please confirm that~. Checked we use what unit is set for workspace to all distance rates

https://github.com/Expensify/App/blob/72c8cf7679f2ce80e39e8fd14c55bc48b5f80937/src/libs/DistanceRequestUtils.ts#L42-L61

3. And for calculateTaxAmount() we use particular tax rate. Meaning when we click on option then only that's triggered.

So I am not getting what we are achieving.

cc @mananjadhav

[BhuvaneshPatil]

Also in selected proposal, root cause is not correct.

We are not passing disabled values to component and the RCA mentions about the logic for creating list items.

[iwiznia]

I think we already had the policy data is always up-to-date here:

Ohhhhh, ok yeah, looking at those now I agree it is not needed

We only have a logic to gray out the option if it has isDisabled is true. So if need to convert enabled to isDisabled.

Got it, makes sense

[iwiznia]

I am not sure if we should modify getMileageRates() method, because it's being used at multiple places.

Like where? I'd think most places it is being used needs this logic and the ones that do not, can just not pass a selected value and the behavior would stay the same as now, no?

or unit, we use what is being used in workspace not individual rate And for calculateTaxAmount() we use particular tax rate. Meaning when we click on option then only that's triggered.

Not sure what you mean here

[mananjadhav]

Checking recent comments.

[mananjadhav]

I don't think any of the existing calls should break. We can even set that as optional param. I don't think we're modifying anything for calculateTaxAmount or unit. We're only ensuring it works as before. Still feel we should go with the shortlisted proposal.

[BhuvaneshPatil]

about RCA?

Also didn't get this point.

It involves cleaning up the existing code as well.

[melvin-bot[bot]]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[dominictb]

Working on this now!

[dylanexpensify]

pending regression period

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @iwiznia, @mananjadhav, @dylanexpensify, @dominictb eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[dominictb]

Should be ready for payment shortly.