[HOLD for payment 2024-02-14] [$250] Remove MoneyRequestTagPage.js and copy any changes since Nov 27 into IOURequestStepTags.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: MoneyRequestTagPage
New Component IOURequestStepTags

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/~01f0c8228cceba8683
• Upwork Job ID: 1747632892276064256
• Last Price Increase: 2024-01-17
• Automatic offers:
• fedirjh | Reviewer | 28119689
• dukenv0307 | Contributor | 28119690

[melvin-bot]

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

[Krishna2323]

Proposal

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

Remove MoneyRequestTagPage.js and copy any changes since Nov 27 into IOURequestStepTags.js

What is the root cause of that problem?

Component update

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

I checked all the commits after 27th Nov and the new component is updated according to the old component, we just need to remove the file and route created for it. I checked the commits thoroughly but will check again in the PR. There is only 1 commit after 27th Nov and it is about moving import of useThemeStyles.

Component to remove:
https://github.com/Expensify/App/blob/main/src/pages/iou/MoneyRequestTagPage.js

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

Line 101 in a7a3e7f

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

We also need to remove all traces from src/libs/Navigation/types.ts & src/libs/Navigation/linkingConfig.ts

App/src/libs/Navigation/linkingConfig.ts

Line 434 in a2f5bd5

[SCREENS.MONEY_REQUEST.TAG]: ROUTES.MONEY_REQUEST_TAG.route,

App/src/libs/Navigation/types.ts

Line 214 in a2f5bd5

[SCREENS.MONEY_REQUEST.TAG]: {

Note: The below 2 steps can be omitted because we only use IOURequestStepTags for IOU creation flow.

Need to update to :action here:

App/src/ROUTES.ts

Line 367 in a2f5bd5

MONEY_REQUEST_STEP_TAG: {

We only need to pass the action from one component here:

App/src/components/MoneyRequestConfirmationList.js

Line 749 in a2f5bd5

Navigation.navigate(ROUTES.MONEY_REQUEST_TAG.getRoute(props.iouType, props.reportID));

Result

[njmulsqb]

I would love to take this!

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

Proposal

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

Remove MoneyRequestTagPage.js and copy any changes since Nov 27 into IOURequestStepTags.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/MoneyRequestTagPage.js

We have had one change since Nov 27, 2023 but it's the refactoring change and has already been applied in IOURequestStepTags. So no need to change here.

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

2. MoneyRequestTagPage isn't used in any component so we don't need to have any updates here.

We only need to remove MoneyRequestTagPage in ModalStackNavigators, remove .MONEY_REQUEST.TAG from SCREENS, and remove MONEY_REQUEST_TAG route

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

Line 98 in a8acf44

[SCREENS.MONEY_REQUEST.DATE]: () => require('../../../pages/iou/MoneyRequestDatePage').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}/tag/${transactionID}/${reportID}`, backTo),

App/src/ROUTES.ts

Lines 339 to 340 in a8acf44

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

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

4. We need to update updateTag function in IOURequestStepTags for the edit case by checking action params. For edit case, we can re-use the logic of saveTag function in EditRequestPage.

And when we click on tag on MoneyRequestView we will navigate to IOURequestStepTags with action param is edit.

After this refactor we can remove EditRequestTagPage.

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

Upwork job price has been updated to $250

[stephanieelliott]

Hey @fedirjh looks like we have a few proposals here, could you please review?

[stephanieelliott]

Bump @fedirjh -- are you able to review these proposals for us?

[Krishna2323]

Proposal Updated

Added a note about why should we not include the action param.

[fedirjh]

Thanks both @Krishna2323 and @dukenv0307 for the proposals.

@Krishna2323 It seems that your proposal is not complete. It didn't cover these steps :

• Be sure to update all routes and navigation to use the new :action param (instead of being hard-coded with "create")
• Update any logic like isEditing to use the new action param from the route

---

I think @dukenv0307 proposal is the closest so far. IMO, the 4th step is a required step, edit and create should be handled by the same component IOURequestStepTags . Can you please further explain how that will be done? what are the required changes?

[dukenv0307]

@fedirjh Thanks for your review, for step 4th, withFullTransactionOrNotFound already get the transaction accordingly for the action params, so we only need to update updateTag function in IOURequestStepTags for the edit case by checking action params. For edit case we can re-use the logic of saveTag function in EditRequestPage.

And when we click on tag on MoneyRequestView we will navigate to this route edit/${iouType}/tag/${transactionID}/${reportID} instead of the old route.

After this refactor we can remove EditRequestTagPage.

[fedirjh]

@dukenv0307 That looks good to me, could you please update your proposal to include that?

[dukenv0307]

@fedirjh Updated proposal

[fedirjh]

This proposal from @dukenv0307 looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @MariaHCD, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @fedirjh 🎉 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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@fedirjh The PR is ready for review.

[stephanieelliott]

PR was CP'd to staging a few hours ago

[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.37-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• remove MoneyRequestTagPage #35136

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

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

• @fedirjh requires payment automatic offer (Reviewer)
• @dukenv0307 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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[stephanieelliott]

I don't think the BZ checklist applies here since this was more a feature update and not really a bug (please check me on that @fedirjh!)

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @dukenv0307 $250, paid via Upwokr
• Contributor+: @fedirjh $250, paid via Upwokr

Upwork job is here: https://www.upwork.com/jobs/~01f0c8228cceba8683

[melvin-bot]

⚠️ 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.