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.

@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

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

We should change it to 4

Result

What alternative solutions did you explore? (Optional)

i think this is caused by this PR

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 #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

        return Number(numValue).toFixed(CONST.MAX_TAX_RATE_DECIMAL_PLACES).toString().replace('.', toLocaleDigit('.'));

       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

1. If yes, (we need to update how we display rates in the money request flow)

Remove .substring(0, value.toString().length);

2. If no, (we need to update how we display rates distance rate page)

        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:

1. Make sure you've read and understood the contributing guidelines.
2. 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.
3. 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.
4. 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>

⚠️ Invalid email. Please make sure to create an Expensify account with this email first 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 #49773. cc: @DylanDylann @daledah

@parasharrajat Hmm, In #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

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

Ok. thanks for the input. let me review the proposals in that 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 fix: fixed 4 decimals for distance rates #49773 (comment) 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: 4
  

  App/src/CONST.ts

  Line 2411 in 99f280b

  RATE_DECIMALS: 3,

• Now update fixedDecimalPoints logic here, based on if we want to hide appending zeroes or not.
  

  App/src/libs/PolicyUtils.ts

  Line 149 in 99f280b

  const fixedDecimalPoints = decimalPart.length > 2 && !decimalPart.endsWith('0') ? 3 : 2;

• An additional problem we face after such update is for currency which do not support decimals.(ex. ALL)

• 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());

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!

@parasharrajat bump on the reviews!

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

🎀 👀 🎀 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!

@thelullabyy could you please provide an update here? Thank you!

@kadiealexander I'll open a PR soon.

@parasharrajat This PR is ready for review.

I can take this as C+ ) if this is critical while @parasharrajat is OOO until Oct 25

@allgandalf are you ok with splitting the C+ bounty since @parasharrajat already did the proposal review? If so, please go ahead.

yeah no worries at all, thanks

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

@puneetlath can you take a look here once you find time, thanks :)

Looks like @allgandalf took it. I can continue reviewing this if needed.