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.

Sign up for GitHub

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

Jump to bottom

Allow setting merchant for split bill #30721

Merged
merged 26 commits into from
Dec 25, 2023
Merged

Allow setting merchant for split bill #30721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c9ef34f
Allow setting merchant for split bill
youssef-lr Nov 1, 2023
ce57363
Style
youssef-lr Nov 3, 2023
41991a4
Fix test
youssef-lr Nov 3, 2023
9d58a0b
Merge branch 'main' into youssef_split_bill_merchant
youssef-lr Nov 9, 2023
1d349a2
Merge branch 'main' into youssef_split_bill_merchant
youssef-lr Nov 14, 2023
0667951
Fix merchant param missing
youssef-lr Nov 14, 2023
1466ce7
Lint
youssef-lr Nov 14, 2023
c6aea99
Merge branch 'main' into youssef_split_bill_merchant
marcochavezf Nov 22, 2023
8b91ea7
Remove isBillSplit condition for showing merchant
marcochavezf Nov 22, 2023
b72d665
Fix lint error
marcochavezf Nov 22, 2023
b03d7fb
Conflicts
youssef-lr Dec 6, 2023
88e6d92
Lint
youssef-lr Dec 7, 2023
d39fc86
Merge branch 'main' into youssef_split_bill_merchant
youssef-lr Dec 11, 2023
e40b8c2
Lint
youssef-lr Dec 11, 2023
7772411
Fix merchant being displayed in scanned requests
youssef-lr Dec 11, 2023
d2bc873
Copy the updates to the new component
youssef-lr Dec 11, 2023
54330f0
Remove duplicate param
youssef-lr Dec 11, 2023
4dd99d7
Add missing fields to split bill actions
youssef-lr Dec 11, 2023
a82e6fc
Merge branch 'main' into youssef_split_bill_merchant
youssef-lr Dec 13, 2023
ac6ef28
Address PR comments
youssef-lr Dec 13, 2023
b892430
Revert changes made to deprecated component
youssef-lr Dec 13, 2023
2a86eaf
Merge branch 'main' into youssef_split_bill_merchant
marcochavezf Dec 14, 2023
b716519
Fix missing props validation for translate function
marcochavezf Dec 14, 2023
83e74bf
Remove redundant prop
youssef-lr Dec 19, 2023
6526dda
Update text style of merchant and change positions
youssef-lr Dec 19, 2023
283dc3e
Merge branch 'main' into youssef_split_bill_merchant
youssef-lr Dec 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/components/MoneyTemporaryForRefactorRequestConfirmationList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@ const propTypes = {
/** Whether the money request is a distance request */
isDistanceRequest: PropTypes.bool,

/** Whether the money request is a scan request */
isScanRequest: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prop is still being passed to the component.


/** Whether we're editing a split bill */
isEditingSplitBill: PropTypes.bool,

Expand Down Expand Up @@ -191,7 +188,6 @@ const defaultProps = {
transaction: {},
mileageRate: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, rate: 0, currency: 'USD'},
isDistanceRequest: false,
isScanRequest: false,
shouldShowSmartScanFields: true,
isPolicyExpenseChat: false,
};
Expand All @@ -215,7 +211,6 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
isEditingSplitBill,
isPolicyExpenseChat,
isReadOnly,
isScanRequest,
listStyles,
mileageRate,
onConfirm,
Expand Down Expand Up @@ -244,8 +239,6 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
const isTypeSplit = iouType === CONST.IOU.TYPE.SPLIT;
const isTypeSend = iouType === CONST.IOU.TYPE.SEND;

const isSplitWithScan = isTypeSplit && isScanRequest;

const {unit, rate, currency} = mileageRate;
const distance = lodashGet(transaction, 'routes.route0.distance', 0);
const shouldCalculateDistanceAmount = isDistanceRequest && iouAmount === 0;
Expand All @@ -259,9 +252,8 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
// Do not hide fields in case of send money request
const shouldShowAllFields = isDistanceRequest || shouldExpandFields || !shouldShowSmartScanFields || isTypeSend || isEditingSplitBill;

// In Send Money flow, we don't allow the Merchant or Date to be edited. For distance requests, don't show the merchant as there's already another "Distance" menu item
const shouldShowDate = shouldShowAllFields && !isTypeSend && !isSplitWithScan;
const shouldShowMerchant = shouldShowAllFields && !isTypeSend && !isDistanceRequest && !isSplitWithScan;
const shouldShowDate = shouldShowSmartScanFields || isDistanceRequest;
const shouldShowMerchant = shouldShowSmartScanFields && !isDistanceRequest;
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved

// Fetches the first tag list of the policy
const policyTag = PolicyUtils.getTag(policyTags);
Expand Down
21 changes: 9 additions & 12 deletions src/components/ReportActionItem/MoneyRequestPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,16 @@ function MoneyRequestPreview(props) {
// Show the merchant for IOUs and expenses only if they are custom or not related to scanning smartscan
const shouldShowMerchant = !_.isEmpty(requestMerchant) && requestMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT && requestMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT;
const shouldShowDescription = !_.isEmpty(description) && !shouldShowMerchant && !isScanning;
const hasPendingWaypoints = lodashGet(props.transaction, 'pendingFields.waypoints', null);

const receiptImages = hasReceipt ? [ReceiptUtils.getThumbnailAndImageURIs(props.transaction)] : [];
let merchantOrDescription = requestMerchant;
if (!shouldShowMerchant) {
merchantOrDescription = description || '';
} else if (hasPendingWaypoints) {
merchantOrDescription = requestMerchant.replace(CONST.REGEX.FIRST_SPACE, translate('common.tbd'));
}

const hasPendingWaypoints = lodashGet(props.transaction, 'pendingFields.waypoints', null);
const receiptImages = hasReceipt ? [ReceiptUtils.getThumbnailAndImageURIs(props.transaction)] : [];

const getSettledMessage = () => {
if (isExpensifyCardTransaction) {
Expand Down Expand Up @@ -315,21 +321,12 @@ function MoneyRequestPreview(props) {
</View>
)}
</View>
{shouldShowMerchant && !props.isBillSplit && (
<View style={[styles.flexRow]}>
<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20, styles.breakWord]}>
{hasPendingWaypoints ? requestMerchant.replace(CONST.REGEX.FIRST_SPACE, translate('common.tbd')) : requestMerchant}
</Text>
</View>
)}
<View style={[styles.flexRow, styles.mt1]}>
<View style={[styles.flex1]}>
{!isCurrentUserManager && props.shouldShowPendingConversionMessage && (
<Text style={[styles.textLabel, styles.colorMuted]}>{translate('iou.pendingConversionMessage')}</Text>
)}
{(shouldShowDescription || (shouldShowMerchant && props.isBillSplit)) && (
<Text style={[styles.colorMuted]}>{shouldShowDescription ? description : requestMerchant}</Text>
)}
{(shouldShowDescription || shouldShowMerchant) && <Text style={[styles.textLabelSupporting]}>{merchantOrDescription}</Text>}
</View>
{props.isBillSplit && !_.isEmpty(participantAccountIDs) && requestAmount > 0 && (
<Text style={[styles.textLabel, styles.colorMuted, styles.ml1, styles.amountSplitPadding]}>
Expand Down
33 changes: 21 additions & 12 deletions src/libs/actions/IOU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,14 +1131,14 @@ function requestMoney(
* @param {Number} amount - always in the smallest unit of the currency
* @param {String} comment
* @param {String} currency
* @param {String} merchant
* @param {String} category
* @param {String} tag
* @param {String} existingSplitChatReportID - the report ID where the split bill happens, could be a group chat or a workspace chat
* @param {String} merchant
*
* @return {Object}
*/
function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, tag, existingSplitChatReportID = '', merchant) {
function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, merchant, category, tag, existingSplitChatReportID = '') {
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
const currentUserEmailForIOUSplit = OptionsListUtils.addSMSDomainIfPhoneNumber(currentUserLogin);
const participantAccountIDs = _.map(participants, (participant) => Number(participant.accountID));
const existingSplitChatReport =
Expand All @@ -1156,7 +1156,7 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
'',
'',
'',
merchant,
merchant || Localize.translateLocal('iou.request'),
Copy link
Contributor

@aimane-chnaif aimane-chnaif Dec 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes regression when user language is Spanish and merchant is not set

Screen.Recording.2023-12-25.at.4.37.27.AM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay, we're going to deprecate the Request merchant soon on P2P requests. Here's the plan from a private issue:

Make manual IOU expenses only have manually entered amount and description:
a. Don't show Merchant or Date fields in a hidden "detailed" section
b. Don't show Merchant: Request this is confusing and seems broken

Show Merchant and/or Date on SmartScanned IOU expenses only if they succeeded.
a. Don't show Merchant: Unknown
b. Don't show a RBR if the merchant isn't read
c. If the SmartScan fails to get one or the other, just hide that field -- exactly like a manual request

For expenses on expense reports, however, require Merchant, Date, and Amount -- and show a RBR if any are missing, for both manual and SmartScanned expenses

undefined,
undefined,
undefined,
Expand Down Expand Up @@ -1357,7 +1357,7 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
'',
CONST.IOU.TYPE.SPLIT,
splitTransaction.transactionID,
merchant,
merchant || Localize.translateLocal('iou.request'),
undefined,
undefined,
undefined,
Expand Down Expand Up @@ -1476,23 +1476,23 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
* @param {Number} amount - always in smallest currency unit
* @param {String} comment
* @param {String} currency
* @param {String} merchant
* @param {String} category
* @param {String} tag
* @param {String} existingSplitChatReportID - Either a group DM or a workspace chat
* @param {String} merchant
*/
function splitBill(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, tag, existingSplitChatReportID = '', merchant) {
function splitBill(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, merchant, category, tag, existingSplitChatReportID = '') {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(
participants,
currentUserLogin,
currentUserAccountID,
amount,
comment,
currency,
merchant,
category,
tag,
existingSplitChatReportID,
merchant,
);
API.write(
'SplitBill',
Expand Down Expand Up @@ -1525,22 +1525,23 @@ function splitBill(participants, currentUserLogin, currentUserAccountID, amount,
* @param {Number} amount - always in smallest currency unit
* @param {String} comment
* @param {String} currency
* @param {String} merchant
* @param {String} category
* @param {String} tag
* @param {String} merchant
*/
function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, tag, merchant) {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, tag, '', merchant);
function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, merchant, category, tag) {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, merchant, category, tag);

API.write(
'SplitBillAndOpenReport',
{
reportID: splitData.chatReportID,
amount,
splits: JSON.stringify(splits),
currency,
merchant,
comment,
category,
merchant,
tag,
transactionID: splitData.transactionID,
reportActionID: splitData.reportActionID,
Expand All @@ -1562,10 +1563,12 @@ function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccou
* @param {String} currentUserLogin
* @param {Number} currentUserAccountID
* @param {String} comment
* @param {String} category
* @param {String} tag
* @param {Object} receipt
* @param {String} existingSplitChatReportID - Either a group DM or a workspace chat
*/
function startSplitBill(participants, currentUserLogin, currentUserAccountID, comment, receipt, existingSplitChatReportID = '') {
function startSplitBill(participants, currentUserLogin, currentUserAccountID, comment, category, tag, receipt, existingSplitChatReportID = '') {
const currentUserEmailForIOUSplit = OptionsListUtils.addSMSDomainIfPhoneNumber(currentUserLogin);
const participantAccountIDs = _.map(participants, (participant) => Number(participant.accountID));
const existingSplitChatReport =
Expand Down Expand Up @@ -1774,6 +1777,8 @@ function startSplitBill(participants, currentUserLogin, currentUserAccountID, co
splits: JSON.stringify(splits),
receipt,
comment,
category,
tag,
isFromGroupDM: !existingSplitChatReport,
...(existingSplitChatReport ? {} : {createdReportActionID: splitChatCreatedReportAction.reportActionID}),
},
Expand Down Expand Up @@ -1981,6 +1986,8 @@ function completeSplitBill(chatReportID, reportAction, updatedTransaction, sessi
created: transactionCreated,
merchant: transactionMerchant,
comment: transactionComment,
category: transactionCategory,
tag: transactionTag,
} = ReportUtils.getTransactionDetails(updatedTransaction);

API.write(
Expand All @@ -1992,6 +1999,8 @@ function completeSplitBill(chatReportID, reportAction, updatedTransaction, sessi
created: transactionCreated,
merchant: transactionMerchant,
comment: transactionComment,
category: transactionCategory,
tag: transactionTag,
splits: JSON.stringify(splits),
},
{optimisticData, successData, failureData},
Expand Down
18 changes: 13 additions & 5 deletions src/pages/iou/request/step/IOURequestStepConfirmation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,17 @@ function IOURequestStepConfirmation({

// If we have a receipt let's start the split bill by creating only the action, the transaction, and the group DM if needed
if (iouType === CONST.IOU.TYPE.SPLIT && receiptFile) {
const existingSplitChatReportID = CONST.REGEX.NUMBER.test(report.reportID) ? reportID : '';
IOU.startSplitBill(selectedParticipants, currentUserPersonalDetails.login, currentUserPersonalDetails.accountID, trimmedComment, receiptFile, existingSplitChatReportID);
const existingSplitChatReportID = CONST.REGEX.NUMBER.test(reportID) ? reportID : '';
IOU.startSplitBill(
selectedParticipants,
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
trimmedComment,
transaction.category,
transaction.tag,
receiptFile,
existingSplitChatReportID,
);
return;
}

Expand All @@ -209,10 +218,10 @@ function IOURequestStepConfirmation({
transaction.amount,
trimmedComment,
transaction.currency,
transaction.merchant,
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
transaction.category,
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
transaction.tag,
report.reportID,
transaction.merchant,
);
return;
}
Expand All @@ -226,9 +235,9 @@ function IOURequestStepConfirmation({
transaction.amount,
trimmedComment,
transaction.currency,
transaction.merchant,
transaction.category,
transaction.tag,
transaction.merchant,
);
return;
}
Expand Down Expand Up @@ -337,7 +346,6 @@ function IOURequestStepConfirmation({
bankAccountRoute={ReportUtils.getBankAccountRoute(report)}
iouMerchant={transaction.merchant}
iouCreated={transaction.created}
isScanRequest={requestType === CONST.IOU.REQUEST_TYPE.SCAN}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change caused a regression #33588

isDistanceRequest={requestType === CONST.IOU.REQUEST_TYPE.DISTANCE}
shouldShowSmartScanFields={_.isEmpty(lodashGet(transaction, 'receipt.source', ''))}
/>
Expand Down
10 changes: 6 additions & 4 deletions tests/actions/IOUTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,7 @@ describe('actions/IOU', () => {
*/
const amount = 400;
const comment = 'Yes, I am splitting a bill for $4 USD';
const merchant = 'Yema Kitchen';
let carlosChatReport = {
reportID: NumberUtils.rand64(),
type: CONST.REPORT.TYPE.CHAT,
Expand Down Expand Up @@ -923,6 +924,7 @@ describe('actions/IOU', () => {
amount,
comment,
CONST.CURRENCY.USD,
merchant,
);
return waitForBatchedUpdates();
})
Expand Down Expand Up @@ -1102,10 +1104,10 @@ describe('actions/IOU', () => {
expect(vitTransaction.comment.comment).toBe(comment);
expect(groupTransaction.comment.comment).toBe(comment);

expect(carlosTransaction.merchant).toBe(CONST.TRANSACTION.DEFAULT_MERCHANT);
expect(julesTransaction.merchant).toBe(CONST.TRANSACTION.DEFAULT_MERCHANT);
expect(vitTransaction.merchant).toBe(CONST.TRANSACTION.DEFAULT_MERCHANT);
expect(groupTransaction.merchant).toBe(CONST.TRANSACTION.DEFAULT_MERCHANT);
expect(carlosTransaction.merchant).toBe(merchant);
expect(julesTransaction.merchant).toBe(merchant);
expect(vitTransaction.merchant).toBe(merchant);
expect(groupTransaction.merchant).toBe(merchant);

expect(carlosTransaction.comment.source).toBe(CONST.IOU.TYPE.SPLIT);
expect(julesTransaction.comment.source).toBe(CONST.IOU.TYPE.SPLIT);
Expand Down
Loading