[PAID] [$250] Distance rates - Distance rate order in Rate page is random after clear cache and restart app

[IuliiaHerets]

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.45-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Distance rates
3. Add a few distance rates
4. Go to workspace chat.
5. Click + > Submit expense > Distance.
6. Enter waypoints > Next.
7. Click Rate on confirmation page.
8. Note that order of distance rates is A, B, C, D, E etc
9. Go to Troubleshoot > Clear cache and restart
10. Select Reset and refresh
11. Repeat Step 4 to 7.

Expected Result:

The order of distance rates in Rate page should remain A, B, C, D, E etc.

Actual Result:

The order of distance rates in Rate page is random after clear cache and restart app.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6625991_1728200705053.20241006_153110.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021844102961422932795
• Upwork Job ID: 1844102961422932795
• Last Price Increase: 2024-10-09
• Automatic offers:
• paultsimura | Reviewer | 104371732
• ChavdaSachin | Contributor | 104371734

Issue Owner

Current Issue Owner: @CortneyOfstad / @trjExpensify

[melvin-bot]

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

[IuliiaHerets]

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

[IuliiaHerets]

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

[ChavdaSachin]

Edited by proposal-police: This proposal was edited at 2024-10-06 17:30:16 UTC.

Proposal

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

Distance rate order in Rate page is random after clear cache and restart app

What is the root cause of that problem?

We are not sorting distanceRates here.

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

Line 67 in 99f280b

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

While we do sort rates on PolicyDistanceRatesPage here.

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

Lines 102 to 106 in 99f280b

	const distanceRatesList = useMemo<RateForList[]>(
	() =>
	Object.values(customUnitRates)
	.sort((rateA, rateB) => (rateA?.rate ?? 0) - (rateB?.rate ?? 0))
	.map((value) => ({

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

Implement sorting after reading rates from ONYX here.

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

Line 67 in 99f280b

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

Implement sorting similar to PolicyDistanceRatesPage

What alternative solutions did you explore? (Optional)

Alternatively we could implement sorting in getMileageRates function here instead of sorting on IOURequestStemDistancePage.

App/src/libs/DistanceRequestUtils.ts

Line 36 in 99f280b

function getMileageRates(policy: OnyxInputOrEntry<Policy>, includeDisabledRates = false, selectedRateID?: string): Record<string, MileageRate> {

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.

[cretadn22]

Proposal

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

Distance rate order in Rate page is random after clear cache and restart app

What is the root cause of that problem?

We don't currently sort the distance rate list in IOURequestStepDistanceRate.

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

Lines 73 to 78 in 99f280b

	const sections = Object.values(rates).map((rate) => {
	const rateForDisplay = DistanceRequestUtils.getRateForDisplay(rate.unit, rate.rate, rate.currency, translate, toLocaleDigit);
	return {
	text: rate.name ?? rateForDisplay,
	alternateText: rate.name ? rateForDisplay : '',

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

We need to sort the distance rate list in IOURequestStepDistanceRate, similar to how it's done in PolicyDistanceRatesPage.

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

Line 105 in 99f280b

.sort((rateA, rateB) => (rateA?.rate ?? 0) - (rateB?.rate ?? 0))

What alternative solutions did you explore? (Optional)

[ChavdaSachin]

Updated Proposal

Updated Proposal added alternate solution.

[cretadn22]

Note for the reviewer:

1. My proposal provides the correct sorting function first (using the same approach as in PolicyDistanceRatesPage). @ChavdaSachin had the same idea but made his update after I submitted my proposal

2. Bringing the sorting logic into the getMileageRates function has no impact. It’s more complicated than my proposal but doesn’t offer any additional benefits

[ChavdaSachin]

@cretadn22 thanks for your insights,
But I believe Proposals are about sharing ideas how a problem could be solved isn't it?
And I believe sorting is not any sort of complex problem where it's must to share pseudo code to make reviewer believe in your solution.
But anyways I will respect whatever thought reviewer might have.

[nkdengineer]

Proposal

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

The order of distance rates in Rate page is random after clear cache and restart app.

What is the root cause of that problem?

I notice that other proposals aren't clear why the display rate is different when we reset cache and restart

When we add new distances, these objects are added to Onyx in turn then this displays the current order that we added is A, B, C, D, E

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

Line 73 in 6d8e739

const sections = Object.values(rates).map((rate) => {

After we reset the cache and restarted, BE returned another order of rates object then it displayed in a different order

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

Line 73 in 6d8e739

const sections = Object.values(rates).map((rate) => {

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

Sort the distance rates by rate value as we do here

const sections = Object.values(rates)
        .sort((rateA, rateB) => (rateA.rate ?? 0) - (rateB.rate ?? 0))
        .map((rate) => {

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

Lines 73 to 75 in 6d8e739

	const sections = Object.values(rates).map((rate) => {
	const rateForDisplay = DistanceRequestUtils.getRateForDisplay(rate.unit, rate.rate, rate.currency, translate, toLocaleDigit);

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

@CortneyOfstad Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[CortneyOfstad]

@paultsimura we have a few proposals above for you to review when you have a chance — thanks!

[paultsimura]

Thanks, I'll review soon 👀

[paultsimura]

Thanks for the proposals, everyone. Considering the triviality of the sorting mechanism here, all 3 proposals look essentially the same to me.
Therefore, I would go with the first one by @ChavdaSachin.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[cretadn22]

This was @ChavdaSachin's initial proposal, and they updated it after mine.

@paultsimura Do you think this is enough for a proposal? It doesn't mention how we sort the list. While the issue is straightforward, it's clear the problem is the lack of a sort function and I believe the proposal need to outline how we sort the list. This is the main key point we need to find out instead of only say "implement sorting"

[paultsimura]

Yes @cretadn22 – I've checked the initial version of that proposal, and it directs to the file with missing sorting, it also suggests what data we need to sort.
As I've mentioned in my review, the sorting itself is quite straightforward, so the first version of that proposal was enough for me.

I'll leave the rest of the consideration for @madmax330 though.

[madmax330]

I agree with @paultsimura, I don't how we're sorting the data is the most important part of the proposal.

[melvin-bot]

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

[CortneyOfstad]

Hey @trjExpensify! I am heading OoO this afternoon (10/15 to 10/23) so reassigning to keep this moving! The PR here has not been launched to staging just yet 👍

Thanks!

[trjExpensify]

Sounds good!

[melvin-bot]

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

[melvin-bot]

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

• Sort Distance Rates on IOURequestsStepDistanceRate.tsx #50721

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

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

• @paultsimura requires payment automatic offer (Reviewer)
• @ChavdaSachin requires payment automatic offer (Contributor)

[melvin-bot]

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:

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

[paultsimura]

This can be treated as a feature request.

• The PR that introduced the bug has been identified. Link to the PR: N/A
• 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: N/A
• 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
• Determine if we should create a regression test for this bug: Yes
• 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.

Regression Test Proposal

Note: This is a beta feature, so enable canUseP2PDistanceRequests beta before proceeding.

1. Go to workspace settings > Distance rates
2. Add a few distance rates
3. Verify they are sorted based on the rate values in ascending order.
4. Go to a workspace chat.
5. Click + > Submit expense > Distance.
6. Enter waypoints > Next.
7. Click Rate on the confirmation page.
8. Verify that the Distance rates are sorted based on the rate values in ascending order.
9. Submit the request
10. Navigate to the request details page
11. Click "Rate" to open the rate editor
12. Verify that the Distance rates are always sorted based on the rate values in ascending order.

Do we agree 👍 or 👎

[CortneyOfstad]

I'm back from OoO — thanks for holding down the fort @trjExpensify!

Agree with your proposal @paultsimura!

[CortneyOfstad]

Payment Summary

@paultsimura — paid $250 via Upwork
@ChavdaSachin — paid $250 via Upwork

Regression Test

https://github.com/Expensify/Expensify/issues/439059