Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[P2P Distance] Create the UpdateMileageRates chore #36974

Open
neil-marcellini opened this issue Feb 20, 2024 · 19 comments
Open

[P2P Distance] Create the UpdateMileageRates chore #36974

neil-marcellini opened this issue Feb 20, 2024 · 19 comments
Assignees
Labels
Engineering Internal Requires API changes or must be handled by Expensify staff Weekly KSv2

Comments

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Feb 20, 2024

Create the UpdateMileageRates chore

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0190810fdd9bc443e7
  • Upwork Job ID: 1760076984860508160
  • Last Price Increase: 2024-02-20
@neil-marcellini neil-marcellini added Engineering Weekly KSv2 Internal Requires API changes or must be handled by Expensify staff labels Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

Copy link

melvin-bot bot commented Feb 20, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @mananjadhav (Internal)

@twisterdotcom
Copy link
Contributor

Okay so here I need to:

  1. Create a new SO, or update this SO: https://stackoverflowteams.com/c/expensify/questions/18326
  2. Include "how to get determine the rates"

I can do this, but weekly for sure. Got some higher priority stuff.

@neil-marcellini neil-marcellini changed the title [HOLD 36965][P2P Distance] Create the UpdateMileageRates chore [P2P Distance] Create the UpdateMileageRates chore Mar 5, 2024
@neil-marcellini
Copy link
Contributor Author

We can get started on this whenever now

@twisterdotcom
Copy link
Contributor

@neil-marcellini this chore, it only ever updates default mileage rates for individuals, never for group workspaces at collect/control right?

Also, are these currencies now hardcoded? We won't recheck what our largest workspaces are in the future?

I also think that perhaps it makes no sense to have INR, RUB, BRL, SDG or UAH because in my research they didn't actually issue rates, meaning there isn't a default for those currencies.

@neil-marcellini
Copy link
Contributor Author

The goal of this chore is to update the constants in PHP that determine our IRS mileage rate from group policies and the mapping for all default P2P mileage rates.

So yes, the chore will not involve changing mileage rates for user's group policies. That's still done by users themselves via an inbox task.

The top 10 currencies are hard coded here since that top 10 list is not likely to change. Then we have a mapping RESEARCHED_CURRENCY_TO_MILEAGE_RATE which is also hard coded, but is meant to be adjusted yearly as part of the chore, via the generateDefaultP2PRates tool. As you can see if I haven't included the currencies you mentioned as not having rates. Finally we have the full mapping CURRENCY_TO_DEFAULT_MILEAGE_RATE which will be output by that tool and should also be updated once per year as part of the chore. Currencies that are not part of the researched rate mapping will have their rate set by converting the USD rate to the currency and unit. We need to have a value for every possible currency and this seems like to most reasonable default.

Lmk if you have further questions and feel free to DM me if needed.

@twisterdotcom
Copy link
Contributor

since that top 10 list is not likely to change

I think it could, but agree, not a huge issue for now.

As you can see if I haven't included the currencies you mentioned as not having rates

I do see in the screen recording here that you didn't put rates in for those:
image

But I think given these jurisdictions don't have set rates, we should just replace them with jurisdictions that do.

In that screenshot we have: USD, EUR, GBP, CAD, INR, AUD, RUB, BRL, SGD and UAH, but 5/10 of those will likely never be populated, so it's sort of a waste.

When we could take the NZD, CHF, ZAR, MXN, ILS, DKK, SEK, NOK and PLN rates I found here: #36965 (comment) - as those jurisdictions do determine rates for drivers.

I would advocate we replace INR, RUB, BRL, SGD and UAH and round out the non EUR currencies for Europe, and add in CHF, DKK, SEK, NOK and PLN in their place. I'd kind of like it if we could just include NZD, ZAR, ILS and MXN too, if it's not too much to add 3 more for regions where we do have customers.

Currencies that are not part of the researched rate mapping will have their rate set by converting the USD rate to the currency and unit. We need to have a value for every possible currency and this seems like to most reasonable default.

I agree this is a fine extra step.


Finally, I kind of think I understand but will this chore, replace the ad-hoc chore we have in Concierge to update the USD mileage rate each year and prompt a new inbox card for group policy owners, and we can just do it with this supportal tool? Example: https://github.com/Expensify/Expensify/issues/351306

@neil-marcellini
Copy link
Contributor Author

❗❗ Heads up, I'm going to be OOO working from Spain 🇪🇸 part time until 3/28. Most days I will be working 50%, some days 100%. Please DM me if something needs urgent attention.❗❗

@twisterdotcom
Copy link
Contributor

@neil-marcellini not urgent, but would love your thoughts on the above - can we replace the currencies that won't ever have rates with ones that we know will?

@melvin-bot melvin-bot bot added the Overdue label Mar 25, 2024
@twisterdotcom
Copy link
Contributor

Neil is out, but we'll figure this out when he's back.

@melvin-bot melvin-bot bot removed the Overdue label Mar 25, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 5, 2024
@twisterdotcom
Copy link
Contributor

Bump on my Q's above @neil-marcellini

@melvin-bot melvin-bot bot removed the Overdue label Apr 8, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 16, 2024
@neil-marcellini
Copy link
Contributor Author

I would advocate we replace INR, RUB, BRL, SGD and UAH and round out the non EUR currencies for Europe, and add in CHF, DKK, SEK, NOK and PLN in their place. I'd kind of like it if we could just include NZD, ZAR, ILS and MXN too, if it's not too much to add 3 more for regions where we do have customers.

Ok, that's easy to do. I'll set a reminder to put up a quick PR for this later this week.

Finally, I kind of think I understand but will this chore, replace the ad-hoc chore we have in Concierge to update the USD mileage rate each year and prompt a new inbox card for group policy owners, and we can just do it with this supportal tool? Example: Expensify/Expensify#351306

Yes it will.

@melvin-bot melvin-bot bot removed the Overdue label Apr 17, 2024
@twisterdotcom
Copy link
Contributor

Nice, LMK when that is done and I'll finally sort this SO.

Last thing, when you say it will replace the chore, one thing we did with the chore was also surface an inbox task that allowed the admins to one-click add the new rate to their workspace. I assume this won't do that, it will just create the new rate as default on new group workspaces?

We might need to think about how we handle notifying users in the future about IRS rate changes.

@neil-marcellini
Copy link
Contributor Author

one thing we did with the chore was also surface an inbox task that allowed the admins to one-click add the new rate to their workspace.

Since it will replace that chore, it should also handle this. What triggers the inbox task to surface? I think it's when the rate is updated. Yep, and that logic lives here. It will show up if we are within the first 3 months of the new rate going into effect. So updating the constant is all we need to do to get this to show up for users.

We will not be automatically changing rates for group workspaces, the change must be made through the inbox task as it works today.

@twisterdotcom
Copy link
Contributor

Man, this is a dope change - it just works!

@melvin-bot melvin-bot bot added the Overdue label Apr 26, 2024
@twisterdotcom
Copy link
Contributor

PR above happening: https://github.com/Expensify/Web-Expensify/pull/41742

@melvin-bot melvin-bot bot removed the Overdue label Apr 26, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Apr 30, 2024
@neil-marcellini
Copy link
Contributor Author

PRs up for review to update the rates. We still need to create the chore later.

@neil-marcellini neil-marcellini removed the Reviewing Has a PR in review label May 21, 2024
@melvin-bot melvin-bot bot added the Overdue label May 21, 2024
@neil-marcellini
Copy link
Contributor Author

Not really a priority yet

@melvin-bot melvin-bot bot removed the Overdue label May 21, 2024
@melvin-bot melvin-bot bot added the Overdue label May 29, 2024
@neil-marcellini
Copy link
Contributor Author

Still haven't had time for this, probably won't for a while 🫤

@melvin-bot melvin-bot bot removed the Overdue label May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Internal Requires API changes or must be handled by Expensify staff Weekly KSv2
Projects
Status: No status
Development

No branches or pull requests

3 participants