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
2.97k stars 2.48k forks source link

[CHECKLIST?][HOLD for payment 2024-05-03] [$250] Complete the final cleanup for distance request refactoring #40870

Closed tgolen closed 10 hours ago

tgolen commented 1 week ago

This is a part of https://github.com/Expensify/App/issues/29107. You can look at that issue for more context behind the cleanup process.

Currently, we migrated the request flow to the new refactor and there are some deprecated things that need to be cleaned up

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fe66ac3d8b2232c0
  • Upwork Job ID: 1783102152724148224
  • Last Price Increase: 2024-04-24
Issue OwnerCurrent Issue Owner: @jliexpensify
melvin-bot[bot] commented 1 week ago

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @jliexpensify (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.

tgolen commented 1 week ago

@brunovjk Can you please comment on this issue so I can assign it to you? We already spoke about your working on this one.

brunovjk commented 1 week ago

Awesome!

tgolen commented 1 week ago

Hm, I'm not sure why it didn't create an upwork job 🤔 @bfitzexpensify do you know why?

melvin-bot[bot] commented 1 week ago

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

brunovjk commented 1 week ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Refactor navigation among screens related to money request

What is the root cause of that problem?

Clean up request flow: Removing the original components and any of the duplicated efforts.

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

  • Edit ROUTES.ts and remove MONEYREQUEST* routes,

https://github.com/Expensify/App/blob/3a3cef693bdb3f594ac02961a4fe42473b3d7adb/src/ROUTES.ts#L313

https://github.com/Expensify/App/blob/f30d4a0f0ba24e0383949a67e478b168fab728c0/src/ROUTES.ts#L317

  • Edit linkgingConfig.js and remove Money_Request* screens

https://github.com/Expensify/App/blob/52a59cd6057849b661d453300343f79d8de9cb14/src/libs/Navigation/linkingConfig/config.ts#L610

  • Edit ModalStackNavigators.js remove old Money_Request* components

https://github.com/Expensify/App/blob/52a59cd6057849b661d453300343f79d8de9cb14/src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx#L95


  • Edit IOU.js and remove _temporaryForRefactor from the name of any methods
  1. setMoneyRequestCurrency_temporaryForRefactor > IOU.js + src/pages/iou/request/step/IOURequestStepTaxAmountPage.tsx
  2. setMoneyRequestBillable_temporaryForRefactor > IOU.js + src/pages/iou/request/step/IOURequestStepConfirmation.tsx
  3. setMoneyRequestParticipants_temporaryForRefactor > IOU.js + src/pages/iou/request/step/IOURequestStepConfirmation.tsx + src/pages/iou/request/step/IOURequestStepParticipants.tsx

  • remove EditRequestReceiptPage and related route

Remove only the component, the routes, screen and nagivation were discussed previously https://github.com/search?q=repo%3AExpensify%2FApp+EditRequestReceiptPage&type=code

  • remove EditSplitBillPage

https://github.com/search?q=repo%3AExpensify%2FApp+EditSplitBillPage&type=code

  • remove EditRequestTagPage

https://github.com/search?q=repo%3AExpensify%2FApp+EditRequestTagPage&type=code


  • Remove ONYXKEYS.IOU (also need to remove some files that relate to IOU like src/pages/iou/propTypes/index.js)

https://github.com/Expensify/App/blob/dadf0ba7824816e62e9d72bb1269228a7d405784/src/ONYXKEYS.ts#L575

Clear ONYXKEYS.IOU used in OU.ts Remove resetMoneyRequestInfo and resetMoneyRequestInfo

  • remove MoneyRequestWaypointPage

Remove src/pages/iou/MoneyRequestWaypointPage.tsx and related route, screen and navigation type https://github.com/search?q=repo%3AExpensify%2FApp+MoneyRequestWaypointPage&type=code

  • move src/pages/iou/steps/MoneyRequestAmountForm.tsx to src/pages/iou/

Move and update it uses: https://github.com/search?q=repo%3AExpensify%2FApp+%40pages%2Fiou%2Fsteps%2FMoneyRequestAmountForm&type=code


  • Rename MoneyTemporaryForRefactorRequestParticipantsSelector.js to MoneyRequestParticipantsSelector

This should be handled by the TS team https://github.com/Expensify/App/issues/29107#issuecomment-2074311126

What alternative solutions did you explore? (Optional)

N/A

brunovjk commented 1 week ago

Thanks @tgolen, I already had a proposal ready, I posted it just as a record, no problem? I will start working in PR this afternoon, ETA until 04/30, hopefully sooner.

melvin-bot[bot] commented 1 week ago

Job added to Upwork: https://www.upwork.com/jobs/~01fe66ac3d8b2232c0

melvin-bot[bot] commented 1 week ago

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

tgolen commented 1 week ago

Sure, thanks! Looks like I got the upwork job created now.

DylanDylann commented 1 week ago

@brunovjk

ETA until 04/30, hopefully sooner.

This is so straightforward. Could you complete PR sooner? Please ping me if there is any problem

brunovjk commented 1 week ago

Sure! You're right, I'll try to get it ready for today.

jliexpensify commented 1 week ago

Invited everyone, Payment Summary:

Upworks Job

melvin-bot[bot] commented 1 week ago

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

If no regressions arise, payment will be issued on 2024-05-03. :confetti_ball:

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 2 days ago

Payment Summary

Upwork Job

BugZero Checklist (@jliexpensify)

jliexpensify commented 21 hours ago

@DylanDylann any need for a checklist?

jliexpensify commented 21 hours ago

Everyone paid and job removed.

DylanDylann commented 20 hours ago

@jliexpensify the checklist is unnecessary. Let's close

jliexpensify commented 10 hours ago

Thanks!