[HOLD for payment 2024-04-15] [$250] Remove NewRequestAmountPage.js and copy any changes since Nov 27 into IOURequestStepAmount.js

[tgolen]

This is a part of #29107. You can look at that issue for more context behind the cleanup process.

Problem

The app has two redundant components:

Old Component: NewRequestAmountPage
New Component IOURequestStepAmount

Solution

Following the examples (example 1, example 2), the Old Component needs to be completely removed from the codebase

1. Look at the history of the Old Component
2. If there are any changes since Nov 27, 2023 which have not been added to the New Component, copy those changes
3. Replace all uses of the Old Component with the New Component
4. Remove all traces of Old Component
5. Be sure to update all routes and navigation to use the new :action param (instead of being hard-coded with "create")
6. Update any logic like isEditing to use the new action param from the route

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01400d372c60004a9e
• Upwork Job ID: 1747632925329993728
• Last Price Increase: 2024-01-24
• Automatic offers:
• DylanDylann | Contributor | 28128915
• dukenv0307 | Contributor | 28135067

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[neonbhai]

Proposal

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

Remove NewRequestAmountPage.js and copy any changes since Nov 27 into IOURequestStepAmount.js

What is the root cause of that problem?

Clean up of NewRequestAmountPage

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

We will be deleting the component here:

App/src/pages/iou/steps/NewRequestAmountPage.js

Lines 57 to 61 in 93e68b5

	function NewRequestAmountPage({route, iou, report, selectedTab}) {
	const styles = useThemeStyles();
	const {translate} = useLocalize();
	const prevMoneyRequestID = useRef(iou.id);

This file will be removed. Looking at history (link), we had three changes. We will incorporate these into the PR (making sure no regressions)

We will remove the screen from ModalStackNavigators.tsx here:

App/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx

Line 94 in 93e68b5

[SCREENS.MONEY_REQUEST.AMOUNT]: () => require('../../../pages/iou/steps/NewRequestAmountPage').default as React.ComponentType,

[njmulsqb]

I would love to take this!

[dukenv0307]

Proposal

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

Remove NewRequestAmountPage.js and copy any changes since Nov 27 into IOURequestStepAmount.js

What is the root cause of that problem?

This is a refactor

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

Look at the history of the Old Component
If there are any changes since Nov 27, 2023 which have not been added to the New Component, copy those changes

1. The history change here: https://github.com/Expensify/App/blob/main/src/pages/iou/steps/NewRequestAmountPage.js

We have had three changes since Nov 27, 2023 but it already had in IOURequestStepAmount. So no need to change here

Replace all uses of the Old Component with the New Component
Remove all traces of Old Component

2. NewRequestAmountPage is only used in MoneyRequestSelectorPage and this page is also removed so we don't any updates here.

We only need to remove NewRequestAmountPage in ModalStackNavigators, remove .MONEY_REQUEST.AMOUNT from SCREENS, and remove MONEY_REQUEST_AMOUNT route

App/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx

Line 94 in a8acf44

[SCREENS.MONEY_REQUEST.AMOUNT]: () => require('../../../pages/iou/steps/NewRequestAmountPage').default as React.ComponentType,

Be sure to update all routes and navigation to use the new :action param (instead of being hard-coded with "create")

3. Update the getRoute function with an extra param action and return it instead of always returning create

getRoute: (action: ValueOf<typeof CONST.IOU.ACTION>, iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, backTo: string) =>
            getUrlWithBackToParam(`${action}/${iouType}/taxRate/${transactionID}/${reportID}`, backTo),

App/src/ROUTES.ts

Line 314 in a8acf44

getRoute: (iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, backTo = '') =>

We also need to update MONEY_REQUEST_CREATE_TAB_MANUAL route by adding a new action param instead of a constant created

App/src/ROUTES.ts

Lines 397 to 398 in 1b1eeb8

	route: 'create/:iouType/start/:transactionID/:reportID/manual',
	getRoute: (iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string) => `create/${iouType}/start/${transactionID}/${reportID}/manual` as const,

Update any logic like isEditing to use the new action param from the route

4. Re-use IOURequestStepAmount for edit case and remove EditRequestAmountPage.

We will handle edit when action param is edit and we can re-use the logic of the saveAmountAndCurrency function in EditRequestPage to update amount for edit case. And when we click on amount in transaction report detail we will navigate to IOURequestStepAmount with action param as edit.

5. Re-use IOURequestStepAmount for the edit split bill case. We can check this case by checking action and iouType param. We can re-use setDraftSplitTransaction function in EditSplitBillPage to update in IOURequestStepAmount as well

6. Edit the route of MONEY_REQUEST_CREATE_TAB_MANUAL

What alternative solutions did you explore? (Optional)

NA

[dukenv0307]

@tgolen I have a question, after refactor in edit request amount page we will use MoneyRequestAmountForm or use IOURequestStepAmount with action is edit

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01400d372c60004a9e

[melvin-bot]

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

[MahmudjonToraqulov]

I would love to take this!

[FitseTLT]

Would love to clean up this

[ghost]

Dibs

[s-alves10]

I would love to take this!

[dukenv0307]

My proposal already here #34614 (comment)

[melvin-bot]

Upwork job price has been updated to $250

[lschurr]

@sobitneupane could you review this one?

[lschurr]

Bump @sobitneupane

[sobitneupane]

@lschurr I won't be able to review proposals before weekend. Please feel free to reassign the issue if it is urgent.

[dukenv0307]

Updated proposal to remove EditRequestAmountPage.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dukenv0307]

@DylanDylann The PR is ready for review.

[DylanDylann]

Hold #35455

[DylanDylann]

@lschurr Let's remove hold label

@dukenv0307 Please update PR

[DylanDylann]

@lschurr The PR is merged, please help to add weekly label

[melvin-bot]

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

[melvin-bot]

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

• Remove NewRequestAmountPage #35833

If no regressions arise, payment will be issued on 2024-04-15. 🎊

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

• @dukenv0307 requires payment automatic offer (Contributor)
• @DylanDylann requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@dukenv0307 / @DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
• [@dukenv0307 / @DylanDylann] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@dukenv0307 / @DylanDylann] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@dukenv0307 / @DylanDylann] Determine if we should create a regression test for this bug.
• [@dukenv0307 / @DylanDylann] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[lschurr]

@dukenv0307 @DylanDylann - Do we need a regression test here?

[DylanDylann]

@lschurr This is a refactoring task, we don't need regression here.

[lschurr]

Payment summary:

• Contributor: @dukenv0307 $250 (paid in Upwork)
• Contributor+: @DylanDylann $250 (paid in Upwork)