Repository navigation
Add Approve button for collect policies in the Settlement button #28976
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
Changes from all commits
e8f91d6
ba6a799
f5323e3
8cbedc0
dc9d510
6ad33ff
8ccd619
4dcd3e6
f75fbf6
238d074
6fd3c42
b24a0a0
e29e619
32b2828
c41c509
35715d7
e62259b
eef5854
257e54f
1f5aa19
6c4463a
64481a9
82e1362
5a91cb2
3355f18
2dfeadb
3c7db13
5595e79
7579f12
0ed35bf
264c9fb
846e710
1c6e756
f65129e
e98f261
235a271
d861e74
516f0cc
e29d407
cccdfb5
0f41e95
28de2c1
28b3d32
9c273f5
5a4779c
c1c32e6
64ca6f4
70fe9ff
1efdd33
55d1716
1c0d6fb
7bd8233
785fd16
8887d86
b898da5
da7d29e
67f4eec
5b81b63
9988511
00fd837
36f14d7
57d88ea
3f887bd
9320a90
e17d75b
d72d9c6
793b2f8
7b510b0
e1c5982
f81e743
4e73080
2fd3579
60685f3
e05e34a
455dbaf
8e5644c
faf147b
12b405f
2159282
778e349
634c431
e8a4484
70c60c1
977852b
11cb327
0f451e0
32151d0
25769ef
5747565
cf823e9
8dd0057
a59f4ec
bbfd56b
73f5a2d
9efe3e2
37502f6
6b3cf50
ff2703e
91ea177
65fd180
ae787de
b1ead6d
02c7526
3cce8d4
16fd5a5
52c421d
1584932
6275670
398a2f3
e4e8cce
da0fe2b
1128243
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,19 +75,24 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, nextSt | |
| const isSettled = ReportUtils.isSettled(moneyRequestReport.reportID); | ||
| const policyType = lodashGet(policy, 'type'); | ||
| const isPolicyAdmin = policyType !== CONST.POLICY.TYPE.PERSONAL && lodashGet(policy, 'role') === CONST.POLICY.ROLE.ADMIN; | ||
| const isGroupPolicy = _.contains([CONST.POLICY.TYPE.CORPORATE, CONST.POLICY.TYPE.TEAM], policyType); | ||
| const isManager = ReportUtils.isMoneyRequestReport(moneyRequestReport) && lodashGet(session, 'accountID', null) === moneyRequestReport.managerID; | ||
| const isPayer = policyType === CONST.POLICY.TYPE.CORPORATE ? isPolicyAdmin && isApproved : isPolicyAdmin || (ReportUtils.isMoneyRequestReport(moneyRequestReport) && isManager); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| const isPayer = isGroupPolicy | ||
| ? // In a group policy, the admin approver can pay the report directly by skipping the approval step | ||
| isPolicyAdmin && (isApproved || isManager) | ||
|
marcochavezf marked this conversation as resolved.
|
||
| : isPolicyAdmin || (ReportUtils.isMoneyRequestReport(moneyRequestReport) && isManager); | ||
| const isDraft = ReportUtils.isDraftExpenseReport(moneyRequestReport); | ||
| const shouldShowSettlementButton = useMemo( | ||
| const shouldShowPayButton = useMemo( | ||
| () => isPayer && !isDraft && !isSettled && !moneyRequestReport.isWaitingOnBankAccount && reimbursableTotal !== 0 && !ReportUtils.isArchivedRoom(chatReport), | ||
| [isPayer, isDraft, isSettled, moneyRequestReport, reimbursableTotal, chatReport], | ||
| ); | ||
| const shouldShowApproveButton = useMemo(() => { | ||
| if (policyType !== CONST.POLICY.TYPE.CORPORATE) { | ||
| if (!isGroupPolicy) { | ||
| return false; | ||
| } | ||
| return isManager && !isDraft && !isApproved && !isSettled; | ||
| }, [policyType, isManager, isDraft, isApproved, isSettled]); | ||
| }, [isGroupPolicy, isManager, isDraft, isApproved, isSettled]); | ||
| const shouldShowSettlementButton = shouldShowPayButton || shouldShowApproveButton; | ||
| const shouldShowSubmitButton = isDraft && reimbursableTotal !== 0; | ||
| const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE; | ||
| const shouldShowNextSteps = isFromPaidPolicy && nextStep && !_.isEmpty(nextStep.message); | ||
|
|
@@ -120,22 +125,13 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, nextSt | |
| onPress={(paymentType) => IOU.payMoneyRequest(paymentType, chatReport, moneyRequestReport)} | ||
| enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS} | ||
| addBankAccountRoute={bankAccountRoute} | ||
| shouldHidePaymentOptions={!shouldShowPayButton} | ||
| shouldShowApproveButton={shouldShowApproveButton} | ||
| style={[styles.pv2]} | ||
| formattedAmount={formattedAmount} | ||
| /> | ||
| </View> | ||
| )} | ||
| {shouldShowApproveButton && !isSmallScreenWidth && ( | ||
| <View style={styles.pv2}> | ||
| <Button | ||
| success | ||
| medium | ||
| text={translate('iou.approve')} | ||
| style={[styles.mnw120, styles.pv2, styles.pr0]} | ||
| onPress={() => IOU.approveMoneyRequest(moneyRequestReport)} | ||
| /> | ||
| </View> | ||
| )} | ||
| {shouldShowSubmitButton && !isSmallScreenWidth && ( | ||
| <View style={styles.pv2}> | ||
| <Button | ||
|
|
@@ -159,21 +155,12 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, nextSt | |
| onPress={(paymentType) => IOU.payMoneyRequest(paymentType, chatReport, moneyRequestReport)} | ||
| enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS} | ||
| addBankAccountRoute={bankAccountRoute} | ||
| shouldHidePaymentOptions={!shouldShowPayButton} | ||
| shouldShowApproveButton={shouldShowApproveButton} | ||
| formattedAmount={formattedAmount} | ||
| /> | ||
| </View> | ||
| )} | ||
| {shouldShowApproveButton && isSmallScreenWidth && ( | ||
| <View style={[styles.ph5, styles.pb2]}> | ||
| <Button | ||
| success | ||
| medium | ||
| text={translate('iou.approve')} | ||
| style={[styles.w100, styles.pr0]} | ||
| onPress={() => IOU.approveMoneyRequest(moneyRequestReport)} | ||
| /> | ||
| </View> | ||
| )} | ||
| {shouldShowSubmitButton && isSmallScreenWidth && ( | ||
| <View style={[styles.ph5, styles.pb2]}> | ||
| <Button | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,7 +147,9 @@ function MoneyRequestView({report, parentReport, parentReportActions, policyCate | |
| if (!isDistanceRequest) { | ||
| amountDescription += ` • ${translate('iou.cash')}`; | ||
| } | ||
| if (isCancelled) { | ||
| if (ReportUtils.isReportApproved(report)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming from #35666 |
||
| amountDescription += ` • ${translate('iou.approved')}`; | ||
| } else if (isCancelled) { | ||
| amountDescription += ` • ${translate('iou.canceled')}`; | ||
| } else if (isSettled) { | ||
| amountDescription += ` • ${translate('iou.settledExpensify')}`; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import lodashGet from 'lodash/get'; | ||
| import PropTypes from 'prop-types'; | ||
| import React from 'react'; | ||
| import React, {useMemo} from 'react'; | ||
| import {View} from 'react-native'; | ||
| import {withOnyx} from 'react-native-onyx'; | ||
| import _ from 'underscore'; | ||
|
|
@@ -47,6 +47,18 @@ const propTypes = { | |
| /** The report's policyID, used for Onyx subscription */ | ||
| policyID: PropTypes.string.isRequired, | ||
|
|
||
| /** The policy tied to the money request report */ | ||
| policy: PropTypes.shape({ | ||
| /** Name of the policy */ | ||
| name: PropTypes.string, | ||
|
|
||
| /** Type of the policy */ | ||
| type: PropTypes.string, | ||
|
|
||
| /** The role of the current user in the policy */ | ||
| role: PropTypes.string, | ||
| }), | ||
|
|
||
| /* Onyx Props */ | ||
| /** chatReport associated with iouReport */ | ||
| chatReport: reportPropTypes, | ||
|
|
@@ -101,6 +113,7 @@ const defaultProps = { | |
| accountID: null, | ||
| }, | ||
| isWhisper: false, | ||
| policy: {}, | ||
| }; | ||
|
|
||
| function ReportPreview(props) { | ||
|
|
@@ -111,14 +124,16 @@ function ReportPreview(props) { | |
| const managerID = props.iouReport.managerID || 0; | ||
| const isCurrentUserManager = managerID === lodashGet(props.session, 'accountID'); | ||
| const {totalDisplaySpend, reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(props.iouReport); | ||
| const policyType = lodashGet(props.policy, 'type'); | ||
|
|
||
| const iouSettled = ReportUtils.isSettled(props.iouReportID); | ||
| const iouCanceled = ReportUtils.isArchivedRoom(props.chatReport); | ||
| const numberOfRequests = ReportActionUtils.getNumberOfMoneyRequests(props.action); | ||
| const moneyRequestComment = lodashGet(props.action, 'childLastMoneyRequestComment', ''); | ||
| const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(props.chatReport); | ||
| const isDraftExpenseReport = isPolicyExpenseChat && ReportUtils.isDraftExpenseReport(props.iouReport); | ||
|
|
||
| const isApproved = ReportUtils.isReportApproved(props.iouReport); | ||
| const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(props.iouReport); | ||
| const transactionsWithReceipts = ReportUtils.getTransactionsWithReceipts(props.iouReportID); | ||
| const numberOfScanningReceipts = _.filter(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)).length; | ||
| const hasReceipts = transactionsWithReceipts.length > 0; | ||
|
|
@@ -172,8 +187,9 @@ function ReportPreview(props) { | |
| if (isScanning) { | ||
| return props.translate('common.receipt'); | ||
| } | ||
| if (ReportUtils.isControlPolicyExpenseChat(props.chatReport) && ReportUtils.isReportApproved(props.iouReport)) { | ||
| return props.translate('iou.managerApproved', {manager: ReportUtils.getDisplayNameForParticipant(managerID, true)}); | ||
| const payerOrApproverName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true); | ||
| if (isApproved) { | ||
| return props.translate('iou.managerApproved', {manager: payerOrApproverName}); | ||
| } | ||
| const managerName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true); | ||
| let paymentVerb = hasNonReimbursableTransactions ? 'iou.payerSpent' : 'iou.payerOwes'; | ||
|
|
@@ -184,10 +200,24 @@ function ReportPreview(props) { | |
| }; | ||
|
|
||
| const bankAccountRoute = ReportUtils.getBankAccountRoute(props.chatReport); | ||
| const shouldShowSettlementButton = ReportUtils.isControlPolicyExpenseChat(props.chatReport) | ||
| ? props.policy.role === CONST.POLICY.ROLE.ADMIN && ReportUtils.isReportApproved(props.iouReport) && !iouSettled && !iouCanceled | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here, |
||
| : !_.isEmpty(props.iouReport) && isCurrentUserManager && !isDraftExpenseReport && !iouSettled && !iouCanceled && !props.iouReport.isWaitingOnBankAccount && reimbursableSpend !== 0; | ||
|
|
||
| const isGroupPolicy = ReportUtils.isGroupPolicyExpenseChat(props.chatReport); | ||
| const isPolicyAdmin = policyType !== CONST.POLICY.TYPE.PERSONAL && lodashGet(props.policy, 'role') === CONST.POLICY.ROLE.ADMIN; | ||
| const isPayer = isGroupPolicy | ||
| ? // In a group policy, the admin approver can pay the report directly by skipping the approval step | ||
| isPolicyAdmin && (isApproved || isCurrentUserManager) | ||
|
marcochavezf marked this conversation as resolved.
|
||
| : isPolicyAdmin || (isMoneyRequestReport && isCurrentUserManager); | ||
| const shouldShowPayButton = useMemo( | ||
| () => isPayer && !isDraftExpenseReport && !iouSettled && !props.iouReport.isWaitingOnBankAccount && reimbursableSpend !== 0 && !iouCanceled, | ||
| [isPayer, isDraftExpenseReport, iouSettled, reimbursableSpend, iouCanceled, props.iouReport], | ||
| ); | ||
| const shouldShowApproveButton = useMemo(() => { | ||
| if (!isGroupPolicy) { | ||
| return false; | ||
| } | ||
| return isCurrentUserManager && !isDraftExpenseReport && !isApproved && !iouSettled; | ||
| }, [isGroupPolicy, isCurrentUserManager, isDraftExpenseReport, isApproved, iouSettled]); | ||
| const shouldShowSettlementButton = shouldShowPayButton || shouldShowApproveButton; | ||
| return ( | ||
| <View style={[styles.chatItemMessage, ...props.containerStyles]}> | ||
| <PressableWithoutFeedback | ||
|
|
@@ -251,6 +281,8 @@ function ReportPreview(props) { | |
| onPress={(paymentType) => IOU.payMoneyRequest(paymentType, props.chatReport, props.iouReport)} | ||
| enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS} | ||
| addBankAccountRoute={bankAccountRoute} | ||
| shouldHidePaymentOptions={!shouldShowPayButton} | ||
| shouldShowApproveButton={shouldShowApproveButton} | ||
| style={[styles.mt3]} | ||
| kycWallAnchorAlignment={{ | ||
| horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the case where we might pass an incorrect index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah since we're now showing the
Approveoption in the settlement button, there's a case where an admin manager approves the report and immediately we show thePayoptions. Given theApprovebutton is the last selected index and it's not available anymore, theselectedItemcan be undefined causing an error. So in this case we default to the first availablePayoption.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm tbh I really wouldn't be able to tell that from looking at this fallback. Is there some way we can make that more explicit so it feels less like a guard against some unknown number of scenarios?
e.g. could the report state be used to track that the report was approved and now needs to be paid? Or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I'm looking at it with more attention, you're right that this line of code is not explicit about that edge case. I like the idea of using the report state to change the behavior of the payment options here. Given the PR is already tested and approved, and also we're planning to make the Approve button "sticky" (it was not included here because it was considered a premature optimization), we can revise this later. Meanwhile, I will merge this PR so we can test the collect approvers flow more thoroughly.