Repository navigation
[C+ Checklist Needs Completion] [$250] Distance rates - Order of distance rates is not preserved after clearing cache and restarting #42939
Description
Activity
Triggered auto assignment to @greg-schroeder (
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.@greg-schroeder 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
We think that this bug might be related to #wave-collect - Release 1
Proposal
Please re-state the problem that we are trying to solve in this issue.
Distance rates - Order of distance rates is not preserved after clearing cache and restarting
What is the root cause of that problem?
We don't sort the
distanceRatesList.
App/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx
Lines 101 to 116 in d5ca2a7
What changes do you think we should make in order to solve the problem?
We can sort the list by the
textor the value.If we decide to sort it by the text then we can add:
.sort((a, b) => localeCompare(a.text, b.text)) ?? []
If we decide to sort by the rate then we can simply add rate property in each object and then sort:
const distanceRatesList = useMemo<RateForList[]>( () => Object.values(customUnitRates) .map((value) => ({ value: value.customUnitRateID ?? '', rate: value.rate ?? 0, text: `${CurrencyUtils.convertAmountToDisplayString(value.rate, value.currency ?? CONST.CURRENCY.USD)} / ${translate( `common.${customUnit?.attributes?.unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES}`, )}`, keyForList: value.customUnitRateID ?? '', isSelected: selectedDistanceRates.find((rate) => rate.customUnitRateID === value.customUnitRateID) !== undefined, isDisabled: value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, pendingAction: value.pendingAction ?? value.pendingFields?.rate ?? value.pendingFields?.enabled ?? value.pendingFields?.currency, errors: value.errors ?? undefined, rightElement: <ListItemRightCaretWithLabel labelText={value.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />, })) .sort((a, b) => a.rate - b.rate) ?? [], [customUnit?.attributes?.unit, customUnitRates, selectedDistanceRates, translate], );
We can also sort in descending order and with any suggested field.
What alternative solutions did you explore? (Optional)
- changed the title
[-]Distance rates - Order of distance rates is not preserved after clearing cache and restarting[/-][+][$250] Distance rates - Order of distance rates is not preserved after clearing cache and restarting[/+]on Jun 1, 2024 Job added to Upwork: https://www.upwork.com/jobs/~0149693392cec4578c
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (
External)Proposal
Please re-state the problem that we are trying to solve in this issue.
Order of distance rates is not preserved after clearing cache and restarting
What is the root cause of that problem?
We haven't yet applied the sorting algorithm to the distance rate page
App/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx
Lines 101 to 105 in 0cb2dc0
What changes do you think we should make in order to solve the problem?
App/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx
Lines 101 to 105 in 0cb2dc0
we should sort distance rate by rate field, we must sort them beforehand as the rate field is not included in the distanceRatesList, ensuring sorting is done prior to the map function.
const distanceRatesList = useMemo<RateForList[]>( () => Object.values(customUnitRates) .sort((taxA, taxB) => (taxA?.rate ?? 0) - (taxB?.rate ?? 0)) .map((value) => ({ value: value.customUnitRateID ?? '',We shouldn't include a rate field in the returned value as suggested earlier, as it serves no purpose.
What alternative solutions did you explore? (Optional)
42 remaining items
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-06-28. 🎊
For reference, here are some details about the assignees on this issue:
- @allroundexperts requires payment through NewDot Manual Requests
- @Krishna2323 requires payment automatic offer (Contributor)
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:
- [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
- [@allroundexperts] 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:
- [@allroundexperts] 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:
- [@allroundexperts] Determine if we should create a regression test for this bug.
- [@allroundexperts] 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.
- [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
@allroundexperts - $250 - You can make a manual request via ND
@Krishna2323 - $250 - Paid through UpworkC+ checklist is all we need for tomorrow and we can close!
- changed the title
[-][HOLD for payment 2024-06-28] [$250] Distance rates - Order of distance rates is not preserved after clearing cache and restarting[/-][+][C+ Checklist Needs Completion] [$250] Distance rates - Order of distance rates is not preserved after clearing cache and restarting[/+]on Jun 27, 2024 @tgolen @greg-schroeder @allroundexperts @Krishna2323 this issue is now 4 weeks old, please consider:
- Finding a contributor to fix the bug
- Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
- If you have any questions, don't hesitate to start a discussion in #expensify-open-source
Thanks!
Checklist
- We never implemented sorting of distance rates on the frontend. As such, this issue exists since the beginning when this feature was added ie here.
- N/A
- N/A
- A regression test would be helpful here. The steps given in the OP look good enough to me.
Okay filing using OP steps
$250 approved for @allroundexperts
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.78-2
Reproducible in staging?: Y
Reproducible in production?: Y
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:
Expected Result:
The order of distance rates will remain as A, B, C, D, E etc
Actual Result:
The order of distance rates is no longer A, B, C, D, E etc
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6497628_1717166582761.20240531_223747.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @greg-schroeder