Expensify
[HOLD for payment 2024-12-07] [$250] Distance - Rate on confirmation page shows 3 decimal value for a 4-digit decimal value rate
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.41-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 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team
Action Performed:
- Go to staging.new.expensify.com
- Go to workspace settings > Distance rates
- Click on any distance rate
- Change the distance rate to 4-digit decimal value (non-zero) and save it
- Go to workspace chat
- Click + > Submit expense > Distance
- Enter waypoints and proceed to confirmation page
- Click Rate
- Select the 4-digit decimal value rate in Step 4
- Note that the rate list only shows 3 decimal value instead of 4
- Submit the distance expense
- Go to transaction thread and open the receipt
- Note that the expense preview and distance receipt shows the complete rate which is 4-digit decimal value
Expected Result:
In Step 10, the rate list should show the complete decimal value (eg, 1.2345)
Actual Result:
In Step 10, the rate list only shows 3 decimal value (eg, 1.234) In Step 13, the expense preview and distance receipt shows 4 decimal value which is the complete rate (eg, 1.2345) The rate format is not consistent in the rate list and distance receipt
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
Add any screenshot/video evidence
https://github.com/user-attachments/assets/5e3c5ab5-93de-46b4-b012-47cb78559ad3
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021841311378926667676
- Upwork Job ID: 1841311378926667676
- Last Price Increase: 2024-10-09
- Automatic offers:
- allgandalf | Contributor | 104470034
Issue Owner
Current Issue Owner: @puneetlath / @kadiealexanderIssue Owner
Current Issue Owner: @puneetlath / @kadiealexander
Triggered auto assignment to @kadiealexander (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[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
@kadiealexander 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
Proposal
Please re-state the problem that we are trying to solve in this issue.
Distance - Rate on confirmation page shows 3 decimal value for a 4-digit decimal value rate
What is the root cause of that problem?
Right here the Rate Decimal for displaying Rate is 3 which is wrong https://github.com/Expensify/App/blob/0e3b82cc9d5e5425bf7dae5652538fdb494c7799/src/CONST.ts#L2402 And same here https://github.com/Expensify/App/blob/decf5154ec7b08722d98e4c9124bfe673d30d405/src/libs/PolicyUtils.ts#L148
What changes do you think we should make in order to solve the problem?
We should change it to 4
Result
https://github.com/user-attachments/assets/a4f42c68-c603-4b63-bec2-5a33f1131b13
What alternative solutions did you explore? (Optional)
Proposal
Please re-state the problem that we are trying to solve in this issue.
We have an inconsistency in the number of decimal places for the distance rate.
What is the root cause of that problem?
In this PR https://github.com/Expensify/App/pull/49773, we decided to always display 4 decimals for all currencies. But we haven't updated this function getRateDisplayValue function.
What changes do you think we should make in order to solve the problem?
Update to display 4 decimals
https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/PolicyUtils.ts#L149
return Number(numValue).toFixed(CONST.MAX_TAX_RATE_DECIMAL_PLACES).toString().replace('.', toLocaleDigit('.'));
https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/PolicyUtils.ts#L122
return numValue.toFixed(CONST.MAX_TAX_RATE_DECIMAL_PLACES);
I see one more edge case when rate is 166.9800 (There are some zeros in the end), should we display zeros? Currently, we also display inconsistency in this case
- If yes, (we need to update how we display rates in the money request flow)
https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/PolicyUtils.ts#L152
Remove .substring(0, value.toString().length);
- If no, (we need to update how we display rates distance rate page)
https://github.com/Expensify/App/blob/69483ff0ef36e39fbf40c15f2caa3471dccbef17/src/libs/CurrencyUtils.ts#L167
minimumFractionDigits: 2,
maximumFractionDigits: 4,
Another problem in the update rate message, the rate only display 2 decimals. But this data is get from originalMessage. If we want to update it, we need to fix on the BE side or make a workaround on FE (detail later if needed)
What alternative solutions did you explore? (Optional)
📣 @thelullabyy! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
Job added to Upwork: https://www.upwork.com/jobs/~021841311378926667676
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)
This seems like a regression from https://github.com/Expensify/App/pull/49773. cc: @DylanDylann @daledah
@parasharrajat Hmm, In https://github.com/Expensify/App/pull/49773, we decided to mirror the Olddot behavior and always use a 4-digit decimal on the rate value.
About this bug, I think this is a new requirement for the update distance rate on the request money flow to use of 4-digit decimals, because this issue also happens on the Olddot
https://github.com/user-attachments/assets/1d144d86-8e44-47fe-92e0-644a94b552cc
I just glanced at proposals and noticed that @thelullabyy mentioned a valuable point
I see one more edge case when rate is 166.9800 (There are some zeros in the end), should we display zeros? Currently, we also display inconsistency in this case
Edited by proposal-police: This proposal was edited at 2024-10-05 23:59:46 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Rate on confirmation page shows 3 decimal value for a 4-digit decimal value rate
What is the root cause of that problem?
- On this PR https://github.com/Expensify/App/pull/49773#issue-2549802815 we decided to use 4 decimal digits for distance rate(irrespective of currency)
- We need some additional tough up for the change.
What changes do you think we should make in order to solve the problem?
- First update
RATE_DECIMALS: 4https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/CONST.ts#L2411 - Now update
fixedDecimalPointslogic here, based on if we want to hide appending zeroes or not. https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/libs/PolicyUtils.ts#L149 - An additional problem we face after such update is for currency which do not support decimals.(ex. ALL)
check this video
parameter: currency- ALL, rate-0.6799, distance-74.59mil, from New York to New Jersey.
https://github.com/user-attachments/assets/c54cbe01-0385-498a-9baf-1ef5cb53f896
- When we display amount on report page we roundoff the amount according to currency. but when we try to edit the amount, edit amount page actually rips off the digits after decimal points for currencies which do not support decimals. which causes inconsistency between amount displayed on report page and amount displayed on edit amount page.
To solve this problem we need to start rounding off amount on IOUStepAmount instead of ripping of the digits after decimal point.
setNewAmount(Number(currentAmount).toFixed());
https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/components/MoneyRequestAmountInput.tsx#L236
And some cleanup.
And need some additional changes from BE, the generated message when changing distance rate only has 3 decimal places so BE now need to return amount with 4 decimal places.
Optional Generally we store amount value by multiplying it with 100 to avoid storing floating values, for the same reason we should start to store rate amount by multiplying it with 10000 and make a workflow around.
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.
@parasharrajat, @kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Because this problem is about the display of currency, I like @thelullabyy's proposal. They picked a few related cases to this issue.
:ribbon: :eyes: :ribbon: C+ reviewed
Triggered auto assignment to @puneetlath, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
📣 @thelullabyy You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖
@parasharrajat
Here in my Proposal I have mentioned an additional problem which will arise afterwards , If you think that issue is out of scope here, then we could raise a new issue for the same. Otherwise I request you to assign this issue to me or at least let me and @thelullabyy work here collaboratively.
Thank you.
cc. @puneetlath
@puneetlath @parasharrajat @kadiealexander @thelullabyy 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!
@allgandalf are you ok with splitting the C+ bounty since @parasharrajat already did the proposal review? If so, please go ahead.
📣 @allgandalf 🎉 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 📖