[$250][P2P Distance] - Distance unit in the system message changes from "miles" to "mi" after changing rate

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Issue found when validating #40021
Version Number: 9.0.17-0
Reproducible in staging?: y
Reproducible in production?: n
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:

• Workspace has a few distance rates.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit a distance expense.
4. Go to transaction thread.
5. Click Rate.
6. Select a different rate.

Expected Result:

The distance unit in the system message will remain the same after changing the rate.

Actual Result:

The distance unit in the system message changes from "miles" to "mi" after changing distance rate.

Workaround:

unknown

Platforms:

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

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

Screenshots/Videos

Bug6563087_1722927119307.20240806_144838.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019f98f5e4f3df20f5
• Upwork Job ID: 1820884011095014188
• Last Price Increase: 2024-08-13
• Automatic offers:
• paultsimura | Contributor | 103497380
• Krishna2323 | Contributor | 103523740

Issue Owner

Current Issue Owner: @abdulrahuman5196

[melvin-bot]

Triggered auto assignment to @muttmuure (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]

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

[github-actions]

👋 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.

[Krishna2323]

Proposal

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

Distance- Distance unit in the system message changes from "miles" to "mi" after changing rate

What is the root cause of that problem?

• In calculateAmountForUpdatedWaypointOrRate we are using getDistanceMerchant which uses getDistanceForDisplay & getRateForDisplay.
  

  App/src/libs/TransactionUtils/index.ts

  Lines 777 to 779 in 984fd39

  	const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, updatedCurrency, Localize.translateLocal, (digit) =>
  	toLocaleDigit(preferredLocale, digit),
  	);

• getDistanceForDisplay & getRateForDisplay uses translations for mi/km.
  

  App/src/libs/DistanceRequestUtils.ts

  Lines 170 to 174 in 984fd39

  	const distanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.miles') : translate('common.kilometers');
  	const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer');
  	const unitString = distanceInUnits === '1' ? singularDistanceUnit : distanceUnit;
  	return `${distanceInUnits} ${unitString}`;

  
  

  App/src/libs/DistanceRequestUtils.ts

  Lines 148 to 153 in 984fd39

  	const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer');
  	const formattedRate = PolicyUtils.getUnitRateValue(toLocaleDigit, {rate});
  	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
  	const currencySymbol = CurrencyUtils.getCurrencySymbol(currency) || `${currency} `;
  	return `${currencySymbol}${formattedRate} / ${singularDistanceUnit}`;

• But we don't need mi/km in miles/kilometers format when using getDistanceMerchant in calculateAmountForUpdatedWaypointOrRate util function because the backend returns the unit in mi/km form.

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

• We should update the getDistanceMerchant function to accept a new param shouldUseShortFormForUnit.
• Then update getDistanceForDisplay and getRateForDisplay to accept a new param shouldUseShortFormForUnit.
• In getDistanceForDisplay and getRateForDisplay, if shouldUseShortFormForUnit is true we will simply use the unit as it is instead of using the translation.
• In calculateAmountForUpdatedWaypointOrRate pass shouldUseShortFormForUnit as true to DistanceRequestUtils.getDistanceMerchant.

What alternative solutions did you explore? (Optional)

[puneetlath]

Doesn't need to block either deploy. Confirming whether the back-end or the front-end is using the proper message here: https://expensify.slack.com/archives/C036QM0SLJK/p1722965084031989. Based on that, we can update one or the other.

[puneetlath]

Ok @neil-marcellini confirmed that the back-end is correct, so we need to fix the optimistic action. Making external.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@puneetlath, @abdulrahuman5196, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[neil-marcellini]

Re-assigning a new C+ due to inactivity as explained here.

[paultsimura]

@Krishna2323's proposal looks good to me.

Let's name the param useShortFormUnit to shorten its name a little.

Also, please note this inconsistency can also be seen in the following 2 scenarios. Make sure we fix them as well:

2024-08-12.-.21.21.-.Screen.Recording.2024-08-12.at.21.20.00.mp4

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[paultsimura]

Bumped @puneetlath in Slack.

[melvin-bot]

📣 @Krishna2323 🎉 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 📖

[Krishna2323]

PR will be ready by the EOD.

[Krishna2323]

@paultsimura, PR ready for review ^

[paultsimura]

Deployed to prod #47605 (comment)
Payment is due 06.09.2024

[paultsimura]

• The PR that introduced the bug has been identified. Link to the PR: No specific PR introduced this – the discrepancy between optimistic and server-side mileage units has been present from the beginning of the Distance implementation. This was just a cleanup.
• 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

Pre-requisite: have a workspace with multiple distance rates.

1. Go to a workspace chat.
2. Submit a distance expense.
3. Go to the transaction thread.
4. Go offline
5. Click "Rate"
6. Select a different rate.
7. Verify the distance unit in the system message uses the short version: "mi" or "km" after changing the distance rate.
8. Go online
9. Verify the system message does not change.

Do we agree 👍 or 👎

[paultsimura]

FYI @muttmuure – the payment is due tomorrow.

[Krishna2323]

@muttmuure, friendly bump for payments 🙏🏻

[paultsimura]

@Krishna2323 I pinged in Slack, should be completed today.

[muttmuure]

$250 - @paultsimura
$250 - @Krishna2323

[muttmuure]

Paid