[HOLD for payment 2024-03-04] [$250] Remove MoneyRequestCategoryPage.js and copy any changes since Nov 27 into IOURequestStepCategory.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: MoneyRequestCategoryPage
New Component IOURequestStepCategory

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/~01aa1b22b861ac0e42
• Upwork Job ID: 1747632761920925696
• Last Price Increase: 2024-01-24
• Automatic offers:
• DylanDylann | Contributor | 28129737
• dukenv0307 | Contributor | 28130978

[melvin-bot]

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

[neonbhai]

would love to work on this!

[tgolen]

Please add a proposal for the changes that are necessary (I know I gave a brief overview, but your proposal should cover more details as I copy/pasted that into at least a dozen issues).

[neonbhai]

Proposal

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

Remove MoneyRequestCategoryPage.js and copy any changes since Nov 27 into IOURequestStepCategory.js

What is the root cause of that problem?

Clean Up
 of MoneyRequestCategoryPage

What changes do you think we should make in order

We will be deleting the component here:

App/src/pages/iou/MoneyRequestCategoryPage.js

Lines 45 to 49 in 93e68b5

	function MoneyRequestCategoryPage({route, report, iou}) {
	const styles = useThemeStyles();
	const {translate} = useLocalize();
	const reportID = lodashGet(route, 'params.reportID', '');

This file will be removed. Checking history (link), it seems like useTheme import was updated, which seems correct in IOURequestStepCategory.js.

We will remove the screen from ModalStackNavigators.tsx

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

Line 100 in 93e68b5

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



We will make sure the IOURequestStepCategory component works correctly.

[lindseydortch]

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

We want to remove the MoneyRequestCategoryPage and make sure any changes after November 27 have been included into these new components.

What is the root cause of that problem?

This is a part of the Wave 5 cleanup effort, so the goal is to refactor the navigation among screens.

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

1. A new component IOURequestStepCategory has been created and history has been checked against MoneyRequestCategoryPage.
2. There is only one change since November 27 which moved the location of the useThemeStyles import was changed to Line 10 instead of 13, so no changes need to be copied into the IOURequestStepCategory because it is already ikn there.

   App/src/pages/iou/MoneyRequestCategoryPage.js

   Line 10 in a7a3e7f

   import useThemeStyles from '@hooks/useThemeStyles';

   
   3 and 4. The only instance where this component is called and needs to be replaced with new component is
   

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

   Line 100 in a7a3e7f

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

3. The route will then need to be updated in (if applicable)

   App/src/ROUTES.ts

   Line 363 in d9ee78d

   route: ':action/:iouType/scan/:transactionID/:reportID',

4. I didn't see any updating logic in this file, so we should be able to skip this step here

What alternative solutions did you explore? (Optional)

N/A

[njmulsqb]

I would love to take this!

[melvin-bot]

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

[melvin-bot]

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

[ghost]

I want to work on it

[ishpaul777]

I'd like to work in this

[tgolen]

As a reminder: comments like "I would like to work on this" have nothing to do with who is selected to do the work. I appreciate the gusto though! Please follow the contributor guidelines.

[dukenv0307]

Proposal

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

Remove MoneyRequestCategoryPage.js and copy any changes since Nov 27 into IOURequestStepCategory.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/commits/main/src/pages/iou/MoneyRequestCategoryPage.js

We only have had one change since Nov 27, 2023 and it's migration theme change. So no need to change here

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

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

We only need to remove MoneyRequestCategoryPage in ModalStackNavigators, remove .MONEY_REQUEST.CATEGORY from SCREENS, and remove MONEY_REQUEST_CATEGORY route

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

Line 100 in a8acf44

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

App/src/ROUTES.ts

Lines 329 to 330 in a8acf44

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

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

4. Re-use IOURequestStepCategory for the edit case and remove EditRequestCategoryPage. We can get the action from the route param to update the logic when saving the date accordingly.

To handle updateCategory function for edit case we can re-use the logic of saveCategory in EditRequestPage.

And when we click on the date option in MoneyRequestView we will navigate to the route of IOURequestStepCategory with action param is edit.

5. Re-use IOURequestStepCategory 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 updateCategory fucntion.

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

Upwork job price has been updated to $250

[anmurali]

@thesahindia - can you review the two proposals above please?

[brunovjk]

Proposal

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

This belongs to Wave 5 cleanup
Phase 2 is all about removing the original components and any of the duplicated efforts.

What is the root cause of that problem?

Replace MoneyRequestCategoryPage and EditRequestCategoryPage to IOURequestStepCategory. This new component will handle both the create and edit flow based on action param.

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. From history change. We can see that only one update was made to the 'useThemeStyles path'. This change is also seen in the new component. This way there will be no need to arrange any changes from MoneyRequestCategoryPage to be transferred to IOURequestStepCategory.

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

2. There is no use of the component other than ModalStackNavigators. We must remove the MoneyRequestCategoryPage component along with:
   

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

   Line 100 in f56f180

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

3. The new component uses STEP_CATEGORY and has it own linkingConfig, SCREENS and ROUTES already configured, we must remove CATEGORY from linkingConfig, SCREENS and ROUTES

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

4. Update route and getRoute to use the new :action param.

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

5. Update Type navigation according to new route and getRoute.

       [SCREENS.MONEY_REQUEST.STEP_CATEGORY]: {
           iouType: string;
           transactionID: string;
           reportID: string;
           backTo: string;
       };
   

6. Add CONST.IOU.ACTION.CREATE at MoneyTemporaryForRefactorRequestConfirmationList to follow ROUTES.MONEY_REQUEST_STEP_CATEGORY changes.

   Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_CATEGORY.getRoute(CONST.IOU.ACTION.CREATE, iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()))
   

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

7. Update MoneyRequestView to follow ROUTES.MONEY_REQUEST_STEP_CATEGORY changes.

   Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_CATEGORY.getRoute(CONST.IOU.ACTION.EDIT, CONST.IOU.TYPE.REQUEST, transaction.transactionID, report.reportID, Navigation.getActiveRouteWithoutParams()))
   

8. Add an isDraft variable to control if we are CREATing or EDITing. Update IOURequestStepCategory.js to accommodate the CREATE and EDIT of the Category field. We need to use IOU.updateMoneyRequestCategory as was done in EditRequestPage

       route: {
           params: {action, reportID, transactionID, backTo},
       },
   ...
   const isDraft = action === CONST.IOU.ACTION.CREATE;
   ...
       Navigation.goBack(isDraft && backTo || ROUTES.HOME);
   ...
       const updateCategory = (category) => {
           if (category.searchText === transaction.category) {
               IOU.resetMoneyRequestCategory_temporaryForRefactor(transactionID);
           } else {
               IOU.setMoneyRequestCategory_temporaryForRefactor(transactionID, category.searchText, isDraft);
           }
           if(!isDraft && category.searchText !== transaction.category){
               IOU.updateMoneyRequestCategory(transaction.transactionID, reportID, category.searchText);
           }
           navigateBack();
       };
   

9. Update setMoneyRequestCategory_temporaryForRefactor to use isDraft :

   ...
    * @param {Boolean} isDraft - Indicates whether it's a draft or not
    */
   function setMoneyRequestCategory_temporaryForRefactor(transactionID, category, isDraft) {
       const key = isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION;
       Onyx.merge(`${key}${transactionID}`, { category });
   }
   

What alternative solutions did you explore? (Optional)

N/A

[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]

Updated proposal to remove EditRequestCategoryPage

[melvin-bot]

@amyevans @anmurali @DylanDylann this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[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]

@DylanDylann The PR is ready for review.

[amyevans]

How's the review stage coming along @dukenv0307 @DylanDylann? Seems like progress might be a little stagnant, @dukenv0307 can you address @DylanDylann's feedback from last week in the PR? Thanks!

[dukenv0307]

I will update after returning from the traditional holiday on the 15th.

[melvin-bot]

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

[anmurali]

Rotating for backup during my OOO till Feb 29th

[stephanieelliott]

PR is being actively reviewed

[stephanieelliott]

PR was deployed to staging a few hours ago!

[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.

[stephanieelliott]

This was deployed to prod on 2/28, seems the automation didn't work so am updating manually

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @dukenv0307 $250 PAID via Upwork
• Contributor+: @DylanDylann $250 PAID via Upwork

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

[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.