-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Allow split actions to be edited and implement IOU action completeSplitBill #29064
Changes from 57 commits
8c8d3ff
12b9043
7fba50a
bd7c4b2
4b95154
8f3f426
60bca0e
914985f
b3c4a3e
d62883c
08d7577
f516e3b
5812464
7950ce7
4096fe2
dd69105
46b6398
f134e3f
01d37b7
f495a29
5ad7a0a
322e20f
11883ed
20be465
9311976
1e71502
e3dc941
45d90fb
3231d67
cc9a41c
b87312d
6a8de3f
c25228c
ba0174c
4e54b1b
63733b2
a518d61
cdf376b
95dc96f
cc00910
5aad7a8
be6be0f
b274a70
56dc410
2f976ef
f80a658
6a28df3
1e21419
c7b5482
26a7311
48d684b
362a1ae
f8484ae
583a607
bf021a5
5867982
7a6f4b2
a740150
f73674d
0126d5f
9107a5a
272fb9b
5cd7d9b
95b3b9f
7945675
ddc2214
1a51105
69c0df9
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 |
---|---|---|
|
@@ -141,8 +141,8 @@ const propTypes = { | |
/** Whether the money request is a distance request */ | ||
isDistanceRequest: PropTypes.bool, | ||
|
||
/** Whether the receipt associated with this report is being scanned */ | ||
isScanning: PropTypes.bool, | ||
/** Whether we should show the amount, date, and merchant fields. */ | ||
shouldShowSmartScanFields: PropTypes.bool, | ||
|
||
/** A flag for verifying that the current report is a sub-report of a workspace chat */ | ||
isPolicyExpenseChat: PropTypes.bool, | ||
|
@@ -182,17 +182,19 @@ const defaultProps = { | |
transaction: {}, | ||
mileageRate: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, rate: 0, currency: 'USD'}, | ||
isDistanceRequest: false, | ||
isScanning: false, | ||
shouldShowSmartScanFields: true, | ||
isPolicyExpenseChat: false, | ||
}; | ||
|
||
function MoneyRequestConfirmationList(props) { | ||
// Destructure functions from props to pass it as a dependecy to useCallback/useMemo hooks. | ||
// Prop functions pass props itself as a "this" value to the function which means they change every time props change. | ||
const {onSendMoney, onConfirm, onSelectParticipant, transaction} = props; | ||
const {onSendMoney, onConfirm, onSelectParticipant} = props; | ||
const {translate, toLocaleDigit} = useLocalize(); | ||
const transaction = props.isEditingSplitBill ? props.draftTransaction || props.transaction : props.transaction; | ||
|
||
const isTypeRequest = props.iouType === CONST.IOU.MONEY_REQUEST_TYPE.REQUEST; | ||
const isSplitBill = props.iouType === CONST.IOU.MONEY_REQUEST_TYPE.SPLIT; | ||
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. Should keep the name consistent and use |
||
|
||
const {unit, rate, currency} = props.mileageRate; | ||
const distance = lodashGet(transaction, 'routes.route0.distance', 0); | ||
|
@@ -202,13 +204,9 @@ function MoneyRequestConfirmationList(props) { | |
const shouldShowCategories = | ||
props.isPolicyExpenseChat && Permissions.canUseCategories(props.betas) && (props.iouCategory || OptionsListUtils.hasEnabledOptions(_.values(props.policyCategories))); | ||
|
||
// A flag for showing SmartScan fields: date, merchant, and amount, only when we don't have a receiptPath (e.g. manual request) | ||
// or in the split details page which is ReadOnly | ||
const shouldShowSmartScanFields = (!props.receiptPath || props.isReadOnly) && !props.isScanning; | ||
|
||
// A flag and a toggler for showing the rest of the form fields | ||
const [shouldExpandFields, toggleShouldExpandFields] = useReducer((state) => !state, false); | ||
const shouldShowAllFields = props.isDistanceRequest || shouldExpandFields || !shouldShowSmartScanFields; | ||
const shouldShowAllFields = props.isDistanceRequest || shouldExpandFields || props.isEditingSplitBill || !props.shouldShowSmartScanFields; | ||
|
||
// Fetches the first tag list of the policy | ||
const policyTag = PolicyUtils.getTag(props.policyTags); | ||
|
@@ -232,10 +230,30 @@ function MoneyRequestConfirmationList(props) { | |
|
||
const isFocused = useIsFocused(); | ||
const [formError, setFormError] = useState(''); | ||
|
||
const [didConfirm, setDidConfirm] = useState(false); | ||
const [didConfirmSplit, setDidConfirmSplit] = useState(false); | ||
|
||
const shouldDisplayFieldError = useMemo(() => { | ||
if (!props.isEditingSplitBill) { | ||
return false; | ||
} | ||
|
||
return (props.hasSmartScanFailed && TransactionUtils.hasMissingSmartscanFields(transaction)) || (didConfirmSplit && TransactionUtils.areRequiredFieldsEmpty(transaction)); | ||
}, [props.isEditingSplitBill, props.hasSmartScanFailed, transaction, didConfirmSplit]); | ||
|
||
useEffect(() => { | ||
if (shouldDisplayFieldError && props.hasSmartScanFailed) { | ||
setFormError('iou.receiptScanningFailed'); | ||
return; | ||
} | ||
if (shouldDisplayFieldError && didConfirmSplit) { | ||
setFormError('iou.error.genericSmartscanFailureMessage'); | ||
return; | ||
} | ||
// reset the form error whenever the screen gains or loses focus | ||
setFormError(''); | ||
}, [isFocused]); | ||
}, [isFocused, transaction, shouldDisplayFieldError, props.hasSmartScanFailed, didConfirmSplit]); | ||
|
||
useEffect(() => { | ||
if (!shouldCalculateDistanceAmount) { | ||
|
@@ -262,25 +280,28 @@ function MoneyRequestConfirmationList(props) { | |
[props.iouAmount, props.iouCurrencyCode], | ||
); | ||
|
||
const [didConfirm, setDidConfirm] = useState(false); | ||
// If completing a split bill fails, set didConfirm to false to allow the user to edit the fields again | ||
if (props.isEditingSplitBill && didConfirm) { | ||
setDidConfirm(false); | ||
} | ||
Comment on lines
+290
to
+293
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. If completing a split bill fails, we don't even set didConfirm to true. So this is not needed? 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. Yeah I wasn't hiding the modal when I implemented this. Will remove. |
||
|
||
const splitOrRequestOptions = useMemo(() => { | ||
let text; | ||
if (props.receiptPath && props.hasMultipleParticipants && props.iouAmount === 0) { | ||
if (isSplitBill && props.iouAmount === 0) { | ||
text = translate('iou.split'); | ||
} else if (props.receiptPath || isDistanceRequestWithoutRoute) { | ||
} else if ((props.receiptPath && isTypeRequest) || isDistanceRequestWithoutRoute) { | ||
text = translate('iou.request'); | ||
} else { | ||
const translationKey = props.hasMultipleParticipants ? 'iou.splitAmount' : 'iou.requestAmount'; | ||
const translationKey = isSplitBill ? 'iou.splitAmount' : 'iou.requestAmount'; | ||
text = translate(translationKey, {amount: formattedAmount}); | ||
} | ||
return [ | ||
{ | ||
text: text[0].toUpperCase() + text.slice(1), | ||
value: props.hasMultipleParticipants ? CONST.IOU.MONEY_REQUEST_TYPE.SPLIT : CONST.IOU.MONEY_REQUEST_TYPE.REQUEST, | ||
value: props.iouType, | ||
}, | ||
]; | ||
}, [props.hasMultipleParticipants, props.iouAmount, props.receiptPath, translate, formattedAmount, isDistanceRequestWithoutRoute]); | ||
}, [isSplitBill, isTypeRequest, props.iouType, props.iouAmount, props.receiptPath, formattedAmount, isDistanceRequestWithoutRoute, translate]); | ||
|
||
const selectedParticipants = useMemo(() => _.filter(props.selectedParticipants, (participant) => participant.selected), [props.selectedParticipants]); | ||
const payeePersonalDetails = useMemo(() => props.payeePersonalDetails || props.currentUserPersonalDetails, [props.payeePersonalDetails, props.currentUserPersonalDetails]); | ||
|
@@ -417,11 +438,28 @@ function MoneyRequestConfirmationList(props) { | |
return; | ||
} | ||
|
||
if (props.isEditingSplitBill && TransactionUtils.areRequiredFieldsEmpty(transaction)) { | ||
setDidConfirmSplit(true); | ||
setFormError('iou.error.genericSmartscanFailureMessage'); | ||
return; | ||
} | ||
|
||
setDidConfirm(true); | ||
onConfirm(selectedParticipants); | ||
} | ||
}, | ||
[selectedParticipants, onSendMoney, onConfirm, props.iouType, props.isDistanceRequest, isDistanceRequestWithoutRoute, props.iouCurrencyCode, props.iouAmount], | ||
[ | ||
selectedParticipants, | ||
onSendMoney, | ||
onConfirm, | ||
props.isEditingSplitBill, | ||
props.iouType, | ||
props.isDistanceRequest, | ||
isDistanceRequestWithoutRoute, | ||
props.iouCurrencyCode, | ||
props.iouAmount, | ||
transaction, | ||
], | ||
); | ||
|
||
const footerContent = useMemo(() => { | ||
|
@@ -510,23 +548,40 @@ function MoneyRequestConfirmationList(props) { | |
isAuthTokenRequired={!_.isEmpty(receiptThumbnail)} | ||
/> | ||
)} | ||
{shouldShowSmartScanFields && ( | ||
{props.shouldShowSmartScanFields && ( | ||
<MenuItemWithTopDescription | ||
shouldShowRightIcon={!props.isReadOnly && !props.isDistanceRequest} | ||
title={formattedAmount} | ||
description={translate('iou.amount')} | ||
onPress={() => !props.isDistanceRequest && Navigation.navigate(ROUTES.MONEY_REQUEST_AMOUNT.getRoute(props.iouType, props.reportID))} | ||
onPress={() => { | ||
if (props.isDistanceRequest) { | ||
return; | ||
} | ||
if (props.isEditingSplitBill) { | ||
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.AMOUNT)); | ||
return; | ||
} | ||
Navigation.navigate(ROUTES.MONEY_REQUEST_AMOUNT.getRoute(props.iouType, props.reportID)); | ||
}} | ||
style={[styles.moneyRequestMenuItem, styles.mt2]} | ||
titleStyle={styles.moneyRequestConfirmationAmount} | ||
disabled={didConfirm || props.isReadOnly} | ||
brickRoadIndicator={shouldDisplayFieldError && transaction.modifiedAmount === 0 && CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR} | ||
error={shouldDisplayFieldError && transaction.modifiedAmount === 0 && translate('common.error.enterAmount')} | ||
youssef-lr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/> | ||
)} | ||
<MenuItemWithTopDescription | ||
shouldShowRightIcon={!props.isReadOnly} | ||
shouldParseTitle | ||
title={props.iouComment} | ||
description={translate('common.description')} | ||
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_DESCRIPTION.getRoute(props.iouType, props.reportID))} | ||
onPress={() => { | ||
if (props.isEditingSplitBill) { | ||
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION)); | ||
return; | ||
} | ||
Navigation.navigate(ROUTES.MONEY_REQUEST_DESCRIPTION.getRoute(props.iouType, props.reportID)); | ||
}} | ||
style={[styles.moneyRequestMenuItem]} | ||
titleStyle={styles.flex1} | ||
disabled={didConfirm || props.isReadOnly} | ||
|
@@ -549,15 +604,23 @@ function MoneyRequestConfirmationList(props) { | |
)} | ||
{shouldShowAllFields && ( | ||
<> | ||
{shouldShowSmartScanFields && ( | ||
{props.shouldShowSmartScanFields && ( | ||
<MenuItemWithTopDescription | ||
shouldShowRightIcon={!props.isReadOnly && isTypeRequest} | ||
shouldShowRightIcon={!props.isReadOnly} | ||
title={props.iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)} | ||
description={translate('common.date')} | ||
style={[styles.moneyRequestMenuItem]} | ||
titleStyle={styles.flex1} | ||
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID))} | ||
onPress={() => { | ||
if (props.isEditingSplitBill) { | ||
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DATE)); | ||
return; | ||
} | ||
Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID)); | ||
}} | ||
disabled={didConfirm || props.isReadOnly} | ||
brickRoadIndicator={shouldDisplayFieldError && _.isEmpty(transaction.modifiedCreated) && CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR} | ||
error={shouldDisplayFieldError && _.isEmpty(transaction.modifiedCreated) && translate('common.error.enterDate')} | ||
/> | ||
)} | ||
{props.isDistanceRequest && ( | ||
|
@@ -571,15 +634,23 @@ function MoneyRequestConfirmationList(props) { | |
disabled={didConfirm || props.isReadOnly || !isTypeRequest} | ||
/> | ||
)} | ||
{shouldShowSmartScanFields && ( | ||
{props.shouldShowSmartScanFields && ( | ||
<MenuItemWithTopDescription | ||
shouldShowRightIcon={!props.isReadOnly && isTypeRequest} | ||
shouldShowRightIcon={!props.isReadOnly} | ||
title={props.iouMerchant} | ||
description={translate('common.merchant')} | ||
style={[styles.moneyRequestMenuItem]} | ||
titleStyle={styles.flex1} | ||
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_MERCHANT.getRoute(props.iouType, props.reportID))} | ||
onPress={() => { | ||
if (props.isEditingSplitBill) { | ||
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.MERCHANT)); | ||
return; | ||
} | ||
Navigation.navigate(ROUTES.MONEY_REQUEST_MERCHANT.getRoute(props.iouType, props.reportID)); | ||
}} | ||
disabled={didConfirm || props.isReadOnly} | ||
brickRoadIndicator={shouldDisplayFieldError && _.isEmpty(transaction.modifiedMerchant) && CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR} | ||
error={shouldDisplayFieldError && _.isEmpty(transaction.modifiedMerchant) && translate('common.error.enterMerchant')} | ||
/> | ||
)} | ||
{shouldShowCategories && ( | ||
|
@@ -640,6 +711,9 @@ export default compose( | |
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, | ||
selector: DistanceRequestUtils.getDefaultMileageRate, | ||
}, | ||
draftTransaction: { | ||
key: ({transactionID}) => `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`, | ||
}, | ||
transaction: { | ||
key: ({transactionID}) => `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,7 @@ function areRequiredFieldsEmpty(transaction: Transaction): boolean { | |
/** | ||
* Given the edit made to the money request, return an updated transaction object. | ||
*/ | ||
function getUpdatedTransaction(transaction: Transaction, transactionChanges: TransactionChanges, isFromExpenseReport: boolean): Transaction { | ||
function getUpdatedTransaction(transaction: Transaction, transactionChanges: TransactionChanges, isFromExpenseReport: boolean, shouldUpdateReceiptState = true): Transaction { | ||
// Only changing the first level fields so no need for deep clone now | ||
const updatedTransaction = {...transaction}; | ||
let shouldStopSmartscan = false; | ||
|
@@ -143,7 +143,13 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra | |
updatedTransaction.tag = transactionChanges.tag; | ||
} | ||
|
||
if (shouldStopSmartscan && transaction?.receipt && Object.keys(transaction.receipt).length > 0 && transaction?.receipt?.state !== CONST.IOU.RECEIPT_STATE.OPEN) { | ||
if ( | ||
shouldUpdateReceiptState && | ||
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. I'm not sure why this function handles updating a receipt, the function name is |
||
shouldStopSmartscan && | ||
transaction?.receipt && | ||
Object.keys(transaction.receipt).length > 0 && | ||
transaction?.receipt?.state !== CONST.IOU.RECEIPT_STATE.OPEN | ||
) { | ||
updatedTransaction.receipt.state = CONST.IOU.RECEIPT_STATE.OPEN; | ||
} | ||
|
||
|
@@ -396,6 +402,7 @@ export { | |
getValidWaypoints, | ||
isDistanceRequest, | ||
getWaypoints, | ||
areRequiredFieldsEmpty, | ||
hasMissingSmartscanFields, | ||
getWaypointIndex, | ||
waypointHasValidAddress, | ||
|
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.
@youssef-lr This was not added to the prop type definition for this component.
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.
Working on a follow up PR to clean things up, we wanted this merged quickly so we can test 🙏🏼