Skip to content

[HOLD for payment 2024-02-19] [$250] Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js #34609

Closed

Description

@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: MoneyRequestDescriptionPage
New Component IOURequestStepDescription

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/~019b522902265a76e3
  • Upwork Job ID: 1747632811433705472
  • Last Price Increase: 2024-01-24
  • Automatic offers:
    • s77rt | Reviewer | 28120097
    • DylanDylann | Contributor | 28120098

Activity

  1. added
    Help WantedApply this label when an issue is open to proposals by contributors
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 16, 2024
  2. melvin-bot commented on Jan 16, 2024

    @melvin-bot

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

  3. KrAbhas commented on Jan 16, 2024

    @KrAbhas
    Contributor

    Proposal

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

    Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js

    What is the root cause of that problem?

    Cleanup
 of navigation among screens for MoneyRequestDescriptionPage

    What changes do you think we should make in order

    We will be deleting the component here:

    function MoneyRequestDescriptionPage({iou, route, selectedTab}) {

    We will remove the screen from ModalStackNavigators.tsx

  4. njmulsqb commented on Jan 17, 2024

    @njmulsqb

    I would love to take this!

  5. DylanDylann commented on Jan 17, 2024

    @DylanDylann
    Contributor

    Proposal

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

    Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js

    What is the root cause of that problem?

    Remove deprecated component

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

    1. Remove MoneyRequestDescriptionPage component and all traces like

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

    [SCREENS.MONEY_REQUEST.DESCRIPTION]: ROUTES.MONEY_REQUEST_DESCRIPTION.route,

    App/src/ROUTES.ts

    Lines 273 to 276 in a2f5bd5

    MONEY_REQUEST_DESCRIPTION: {
    route: ':iouType/new/description/:reportID?',
    getRoute: (iouType: string, reportID = '') => `${iouType}/new/description/${reportID}` as const,
    },

    DESCRIPTION: 'Money_Request_Description',

    [SCREENS.MONEY_REQUEST.DESCRIPTION]: {
    iouType: string;
    reportID: string;
    field: string;
    threadReportID: string;
    };

    1. For new component IOURequestStepDescription
      In here

      App/src/ROUTES.ts

      Lines 342 to 346 in a2f5bd5

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

      We need to use the new :action param (instead of being hard-coded with "create") like
    MONEY_REQUEST_STEP_DESCRIPTION: { 
         route: ':action/:iouType/description/:transactionID/:reportID', 
         getRoute: (action: ValueOf<typeof CONST.IOU.ACTION>, iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, backTo = '') => 
             getUrlWithBackToParam(`${action}/${iouType}/description/${transactionID}/${reportID}`, backTo), 
     }, 
    

    And then we will create new variable
    const isDraft = action === CONST.IOU.ACTION.CREATE (we will get action from route) and pass it to setMoneyRequestDescription_temporaryForRefactor as a new param. In setMoneyRequestDescription_temporaryForRefactor we will save the new description to draft_transaction if isDraft is true else we will save it to the transaction

    1. We also need to remove EditRequestDescriptionPage and we need to update the route to navigate to IOURequestStepDescription (with action = edit) instead of EditRequestDescriptionPage
      In here
      onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION))}
    Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()));
    

    We also need to update

    Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()));

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

    In editing split flow, we also need to update to navigate to IOURequestStepDescription

    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION));

    Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(CONST.IOU.ACTION.EDIT, props.iouType, transaction.transactionID, props.reportID, Navigation.getActiveRouteWithoutParams()));
    
    1. Copy changes since 27 Nov to new components based on list commit here: https://github.com/Expensify/App/commits/main/src/pages/iou/MoneyRequestDescriptionPage.js

    What alternative solutions did you explore? (Optional)

  6. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jan 17, 2024
  7. changed the title [-]Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js[/-] [+][$500] Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js[/+] on Jan 17, 2024
  8. melvin-bot commented on Jan 17, 2024

    @melvin-bot
  9. melvin-bot commented on Jan 17, 2024

    @melvin-bot

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

  10. 50 remaining items

  11. s77rt commented on Feb 13, 2024

    @s77rt
    Member

    Checklist ^ does not apply here. This is a feature request (code refactor) and not a bug.

  12. alexpensify commented on Feb 15, 2024

    @alexpensify
    Contributor

    Flagging that I will be OOO on Monday, but will complete the payment process on Tuesday, February 20.

  13. alexpensify commented on Feb 20, 2024

    @alexpensify
    Contributor

    Payment summary is here: #34609 (comment)

    Everyone has been paid via Upwork.

  14. s77rt commented on Feb 20, 2024

    @s77rt
    Member

    @alexpensify Payment here should be $125 for the regression. I have refunded $125

  15. alexpensify commented on Feb 20, 2024

    @alexpensify
    Contributor

    Thanks for your honesty! I missed the regression notice and saw it after I hit send in Upwork. All actions are complete here, closing!

  16. melvin-bot commented on Mar 11, 2024

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

  17. alexpensify commented on Mar 11, 2024

    @alexpensify
    Contributor

    @s77rt - I believe that the regression was already addressed in another GH. Was today's notice an error? Thanks!

  18. s77rt commented on Mar 11, 2024

    @s77rt
    Member

    Yes the above notice is a false positive

  19. alexpensify commented on Mar 11, 2024

    @alexpensify
    Contributor

    Thanks!

  20. DylanDylann commented on Mar 11, 2024

    @DylanDylann
    Contributor

    Let's ignore it. It is only a reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2EngineeringExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions