[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 https://github.com/Expensify/App/issues/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[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:

https://github.com/Expensify/App/blob/93e68b5fff30ba950d4f2192f468e334531da826/src/pages/iou/MoneyRequestCategoryPage.js#L45-L49

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 https://github.com/Expensify/App/blob/93e68b5fff30ba950d4f2192f468e334531da826/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx#L100



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. https://github.com/Expensify/App/blob/a7a3e7feec0c4da822779c4c0f0657f2fcbeeb62/src/pages/iou/MoneyRequestCategoryPage.js#L10 3 and 4. The only instance where this component is called and needs to be replaced with new component is https://github.com/Expensify/App/blob/a7a3e7feec0c4da822779c4c0f0657f2fcbeeb62/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx#L100
3. The route will then need to be updated in (if applicable) https://github.com/Expensify/App/blob/d9ee78d6614dbb8b620b5b362dbbc198dec6b8c5/src/ROUTES.ts#L363
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[bot]]

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

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

[ishpaul777]

Proposal

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

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?

No RC, clean up task

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

We will be deleting the component MoneyRequestCategoryPage and copy all changes after November 27 in IOURequestStepCategory component and make sure everything works correctly.

[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

https://github.com/Expensify/App/blob/a8acf44090c9aab4e9af7967a79758472ada8fc3/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx#L100

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),

https://github.com/Expensify/App/blob/a8acf44090c9aab4e9af7967a79758472ada8fc3/src/ROUTES.ts#L329-L330

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[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: https://github.com/Expensify/App/blob/f56f1809846822f35d3bca8762a302086df1b774/src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx#L100

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.

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

[thesahindia]

Hi @anmurali, please reassign. I am not in a good shape and I won't be able to look at this soon.

[DylanDylann]

I have the context of this epic. I can take over this issue as C+ contributor

[melvin-bot[bot]]

📣 @DylanDylann 🎉 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 📖

[anmurali]

Reassigned to you @DylanDylann

[dukenv0307]

Updated proposal to cover the edit split bill case.

[DylanDylann]

As discussed here, we need to use IOURequestStepCategory in the create/edit flow for both request money and split bill flow

@brunovjk I appreciate your proposal with detailed implementation but your proposal doesn't all cases as above discussion

@dukenv0307 Your proposal look good to me

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

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