Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hold Requests: Individual Money Request Page, Reason Interstitial, Banner #31300

Closed
3 tasks
robertjchen opened this issue Nov 14, 2023 · 18 comments
Closed
3 tasks
Assignees
Labels
Reviewing Has a PR in review Task Weekly KSv2

Comments

@robertjchen
Copy link
Contributor

robertjchen commented Nov 14, 2023

image

Suggested Implementation Plan

  • Implement new Hold option to the three dots menu when viewing individual Money Request
  • Implement new interstitial to provide reason for the hold
  • Implement hold banner when viewing held Money Request

Update Overflow Menu in MoneyRequestView

Overflow Menu (2nd Mockup from Left)

In <MoneyRequestHeader, add a new Hold entry to threeDotsMenuItems with a clock icon.

When selected it should call the locally defined method holdMoneyRequest()

Lastly, this Hold menu item will only be conditionally rendered based on ReportUtils.isOnHold(moneyRequestReport). If so, render the 'Hold' text for the popover menu option, or 'Unhold' otherwise.

Holding a request (member & admin)

holdMoneyRequest() is called after the user taps Hold in the Overflow Menu.

In the function, we'll check if ReportUtils.isOnHold(moneyRequestReport) evaluates to true, after which we will Navigation.navigate(ROUTES.getMoneyRequestHoldRoute( … )) to an interstitial for the user to provide a reason for the hold request.

Navigation Changes for Hold Reason Interstitial

Reason Interstitial

Under src/pages/iou, create a new HoldReasonPage where we'll capture the user-provided reasoning for the hold.

The structure of the page should should resemble the following:

<ScreenWrapper>
            <HeaderWithBackButton>
              <Form submitButtonText={translate('iou.holdRequest'')}>
                <Text> 
                  <TextInput>
                     …

The key implementation details here to make note of are that:

  • The page is auto-focused when navigating to the page.
  • That error message is displayed after submission of an empty field for the reason.

When the user confirms the reason, we Navigate back to the MoneyRequestPage, passing the user-provided reason.

Back on MoneyRequestPage, fire off an API call with the following parameters:

API.write( 'HoldRequest',
{
	transactionID, // the money request being held
	comment        // the reason given for the hold
...
}

Onyx Data Response

Optimistic

// report action contains the following structure
{
    accountID: '<accountID>',
    date: DateUtils.getDBTime(),
    comment: ''
},
const createdReportAction = ReportUtils.buildOptimisticCreatedReportAction();
{
    onyxMethod: Onyx.METHOD.SET,
    key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${policyReport.reportID}`,
    value: {
        value: {[createdReportAction.reportActionID]: createdReportAction},
    },
},
{
    onyxMethod: Onyx.METHOD.MERGE,
    key: ONYXKEYS.COLLECTION.TRANSACTION,
    value: {
        [currentTransactionID]: {
            comment: {
                hold: createdReportActionID,
            },
        },
    },
},

Failure

{
    onyxMethod: Onyx.METHOD.MERGE,
    key: ONYXKEYS.COLLECTION.TRANSACTION,
    value: {
        [currentTransactionID]: {
            comment: {
                held: null
            },
        },
    },
},

Banner

In components/ReportActionItem/MoneyRequestView.js, right before the <View for <ReportActionItemImage add a new <View for the hold banner.

  • Add a <TextPill with a red background containing the props.translate('iou.Hold') text.
  • Add a <Text containing props.translate('iou.requestOnHold')

This <View is conditionally rendered based on whether ReportUtils.isOnHold(props.parentReport) evaluates to true

@BartoszGrajdek
Copy link
Contributor

Hello, I'm Bartosz Grajdek from Software Mansion, the expert agency, and I would like to work on this task.

@robertjchen
Copy link
Contributor Author

thanks @BartoszGrajdek ! Let me know if you have any questions 🙇

@robertjchen
Copy link
Contributor Author

In progress, addressing open questions on Slack

@melvin-bot melvin-bot bot removed the Overdue label Nov 29, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2023
@robertjchen
Copy link
Contributor Author

In progress!

@robertjchen
Copy link
Contributor Author

Discussing on Slack!

@robertjchen
Copy link
Contributor Author

@BartoszGrajdek
Copy link
Contributor

This issue is ready to go, what's blocking us now is this issue

@robertjchen
Copy link
Contributor Author

robertjchen commented Jan 5, 2024

Working with Bartosz to iron out a few final items in the draft PR as well as make corresponding backend changes: #33124

@robertjchen
Copy link
Contributor Author

Please see main issue for full-picture update: https://github.com/Expensify/Expensify/issues/274076#issuecomment-1878519877

@robertjchen
Copy link
Contributor Author

Working on corresponding backend changes

@robertjchen
Copy link
Contributor Author

Please see https://github.com/Expensify/Expensify/issues/274076#issuecomment-1899437428 for the latest consolidated update!

@robertjchen
Copy link
Contributor Author

Backend changes for this under review, hoping it have it out tomorrow/Wednesday 🙏

@melvin-bot melvin-bot bot added the Overdue label Jan 31, 2024
@robertjchen
Copy link
Contributor Author

First backend bugfix went out and was confirmed! Second backend bugfix now on staging 🎉

@melvin-bot melvin-bot bot removed the Overdue label Feb 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@robertjchen robertjchen added the Reviewing Has a PR in review label Feb 13, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Overdue Weekly KSv2 labels Feb 13, 2024
@robertjchen
Copy link
Contributor Author

This is on staging, followup issues to be created for any bugs/improvements.

Copy link

melvin-bot bot commented Feb 20, 2024

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

Copy link

melvin-bot bot commented Feb 20, 2024

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

Copy link

melvin-bot bot commented Feb 20, 2024

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

Copy link

melvin-bot bot commented Feb 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reviewing Has a PR in review Task Weekly KSv2
Projects
None yet
Development

No branches or pull requests

2 participants