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

[HOLD for payment 2024-05-02] CRITICAL: [P2P Distance] [$500] Create a new Rate field #36985

Closed
neil-marcellini opened this issue Feb 21, 2024 · 44 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Feb 21, 2024

Create a new Rate field following this plan. Please be sure to read and understand the relevant sections of the entire plan as well.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0184867e0f5e71241a
  • Upwork Job ID: 1760135995824672768
  • Last Price Increase: 2024-02-21
  • Automatic offers:
    • getusha | Reviewer | 0
Issue OwnerCurrent Issue Owner: @bfitzexpensify
@neil-marcellini neil-marcellini added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Feb 21, 2024
@melvin-bot melvin-bot bot changed the title [P2P Distance] Create a new Rate field [$500] [P2P Distance] Create a new Rate field Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0184867e0f5e71241a

Copy link

melvin-bot bot commented Feb 21, 2024

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

@neil-marcellini neil-marcellini added the NewFeature Something to build that is a new item. label Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Feb 21, 2024
@neil-marcellini neil-marcellini self-assigned this Feb 21, 2024
@neil-marcellini
Copy link
Contributor Author

@koko57 would you please reply to this comment so I can assign you to implement this issue? It's the largest piece of the front end implementation so I would like you to implement it, since you wrote the front end design and know it better than anyone else 🙂

@neil-marcellini neil-marcellini removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 21, 2024
@koko57
Copy link
Contributor

koko57 commented Feb 21, 2024

Taking this one! 😃

Copy link

melvin-bot bot commented Feb 21, 2024

📣 @getusha 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@neil-marcellini
Copy link
Contributor Author

neil-marcellini commented Mar 1, 2024

How's this coming along? It's still a bit of a ways out, so no rush, just curious.

@melvin-bot melvin-bot bot removed the Overdue label Mar 1, 2024
@koko57
Copy link
Contributor

koko57 commented Mar 1, 2024

Most of the UI done 😃 Same with the util functions (the one for getting the Rate for P2P will probably need to be changed a bit, I used mock data now). I will leave a detailed summary what is already done and what has to be done today EOD. I will push my changes on the branch so it can be taken over if necessary while I'm ooo.

@koko57
Copy link
Contributor

koko57 commented Mar 1, 2024

Pasting all the required steps for this issue with the current status:

  • Create a util function DistanceRequestUtils.getDistanceForDisplay [or refactor getDisplayMerchant]
    -> done
  • Create a util function DistanceRequestUtils.getRateForP2P
    -> started working on it - it will probably need to be changed when the default rates will be implemented in the App
  • Create a util function DistanceRequestUtils.getRateForDisplay
    -> done
  • Create a function DistanceRequestUtils.getMileageRates
    -> done, only needs some TS fix

  • Create new Rate field UI
    -> UI: done

    TODO:
  • use canUseP2PDistanceRequests from usePermissions to display old distance / rate field or new distance and new rate field (now I hardcoded it as a boolean) ON HOLD for: https://github.com/Expensify/App/pull/37185

  • use rate from mileageRates with transactionDraft.comment.customUnit.customUnitRateID or get it with

DistanceRequestUtils.getRateForP2P


  • Create Rate Selection page UI (with translations)

    -> partially done:

    TODO:

  • Spanish translations


  • Add logic for the merchant recalculation

    -> TODO:
  • recalculation logic
  • Create new functions in IOU utils to save the rate as NVP.LAST_SELECTED_DISTANCE_RATES and update comment.customUnit.customUnitRateID  in the transactionDraft object.

    -> started working on them, they will probably need to be changed later 

  • Add Rate field to the MoneyRequestView

    -> TODO:
  • UI + logic

All the partially done code has TODO comments. Changes are on this branch https://github.com/koko57/App/tree/feat/36985-create-new-rate-field

I'm leaving for the next week, I'll be back on 03/12
@neil-marcellini please let @waterim know when he should take it over

@neil-marcellini
Copy link
Contributor Author

Thanks very much. @waterim if you could take over and continue progress on that branch while @koko57 is out, that would be great. We're getting close to enabling P2P on the backend and frontend, and this will come right after that.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Apr 22, 2024
Copy link

melvin-bot bot commented Apr 23, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Apr 23, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 25, 2024
@melvin-bot melvin-bot bot changed the title CRITICAL: [P2P Distance] [$500] Create a new Rate field [HOLD for payment 2024-05-02] CRITICAL: [P2P Distance] [$500] Create a new Rate field Apr 25, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Apr 25, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-05-02. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Apr 25, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@getusha] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels May 1, 2024
@bfitzexpensify
Copy link
Contributor

Looks like there were a fair number of regressions here. @neil-marcellini, what do you think a fair payout for this job is?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 4, 2024
@neil-marcellini
Copy link
Contributor Author

It's not a bug, it was a new feature implemented by an expert from Callstack, and most features have some bugs initially, so I don't think we should reduce any pay for regressions. Also I believe most bugs were under a beta.

The only payment needed is for the C+ @getusha, and I think they deserve full payment.

Copy link

melvin-bot bot commented May 7, 2024

@shawnborton, @neil-marcellini, @koko57, @bfitzexpensify, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@bfitzexpensify
Copy link
Contributor

Thanks @neil-marcellini - @getusha, I sent you an offer for this job.

@melvin-bot melvin-bot bot removed the Overdue label May 7, 2024
@getusha
Copy link
Contributor

getusha commented May 10, 2024

Accepted @bfitzexpensify, thank you all! 🙇

@melvin-bot melvin-bot bot added the Overdue label May 10, 2024
@bfitzexpensify
Copy link
Contributor

Thanks! Payment complete, thanks for the work here everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
No open projects
Development

No branches or pull requests

7 participants