search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.35k stars 2.78k forks source link

[P2P Distance] Create the UpdateMileageRates chore #36974

Closed neil-marcellini closed 1 month ago

neil-marcellini commented 7 months ago

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
Issue OwnerCurrent Issue Owner: @twisterdotcom
melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

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

twisterdotcom commented 7 months ago

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 commented 6 months ago

We can get started on this whenever now

twisterdotcom commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

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: https://github.com/Expensify/App/issues/36965#issuecomment-1966459335 - 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 commented 6 months ago

❗❗ 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 commented 6 months ago

@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?

twisterdotcom commented 6 months ago

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

twisterdotcom commented 5 months ago

Bump on my Q's above @neil-marcellini

neil-marcellini commented 5 months ago

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.

twisterdotcom commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

twisterdotcom commented 5 months ago

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

neil-marcellini commented 5 months ago

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

neil-marcellini commented 4 months ago

Not really a priority yet

neil-marcellini commented 4 months ago

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

neil-marcellini commented 3 months ago

Going to make this monthly

twisterdotcom commented 2 months ago

Am I up to do something here yet @neil-marcellini?

neil-marcellini commented 2 months ago

Hey yes. I'm going to start creating the chore soon, so would you please write an SO with instructions on how to do the rate updates? You'll need to list out all the steps from doing the research to entering new data in the tool, then creating a PR with the updates. The Generate Default P2P Rates tool has some instructions already so you can point to those. We also want the chore to include updating the IRS mileage rate that is used as the default for all workspaces. The USD default P2P rate should match it.

neil-marcellini commented 2 months ago

Here's an SO for you to answer

neil-marcellini commented 2 months ago

I'm going to take some notes about creating chores because I'm finding it to be a little tricky. Afterwards hopefully I can improve the documentation.

I'm following this SO How do I create a recurring chore in Bedrock?

It would be really nice to have some example chores to follow, for a few of the most common different types. I chose this one AnnualSecurityTraining and thought I needed to set the name to a constant, but then Rafe told me that I don't need to.

To be continued...

neil-marcellini commented 2 months ago

I got PRs up for review, so this should be done pretty soon. @twisterdotcom would you please answer the SO soon? We have until December 2nd to answer it, but I would like to get it out of the way. Let's chat in DMs if you have further questions about what the content of the SO should be.

twisterdotcom commented 2 months ago

I made a good start on this, can add some more links for currencies eventually but this should do for now.

neil-marcellini commented 1 month ago

Awesome thanks! I added a section to the SO about updating the IRS mileage rates as well, and gave instructions for the assignee to accomplish both updates in one PR. I think we're all done 🎉