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

Fix optimistic merchant #32200

Merged
merged 15 commits into from
Dec 4, 2023
Merged

Fix optimistic merchant #32200

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8a8ce84
fix optimistic merchant
dukenv0307 Nov 29, 2023
b91478f
Merge branch 'main' into fix/32183
dukenv0307 Nov 29, 2023
94f30d3
fix jest test
dukenv0307 Nov 29, 2023
55eb7f3
edit test
dukenv0307 Nov 30, 2023
b49a446
Merge branch 'main' into fix/32183
dukenv0307 Nov 30, 2023
d4baf33
add change of previous pr
dukenv0307 Nov 30, 2023
ec32acd
Merge branch 'main' into fix/32183
dukenv0307 Dec 1, 2023
eb126a1
Update src/libs/actions/IOU.js
dukenv0307 Dec 1, 2023
a1d15a9
re-order param
dukenv0307 Dec 1, 2023
6642c60
add default value for merchant param
dukenv0307 Dec 1, 2023
76569f5
remove default value
dukenv0307 Dec 1, 2023
1083e35
add merchant param in split bill API
dukenv0307 Dec 1, 2023
db14284
add merchant in optimistic data of child transaction
dukenv0307 Dec 1, 2023
f179348
merge main
dukenv0307 Dec 2, 2023
4708fed
re-order tag param
dukenv0307 Dec 2, 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
9 changes: 5 additions & 4 deletions src/components/ReportActionItem/MoneyRequestPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ function MoneyRequestPreview(props) {
const isDeleted = lodashGet(props.action, 'pendingAction', null) === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;

// Show the merchant for IOUs and expenses only if they are custom or not related to scanning smartscan
const shouldShowMerchant =
!_.isEmpty(requestMerchant) && !props.isBillSplit && requestMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT && requestMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT;
const shouldShowMerchant = !_.isEmpty(requestMerchant) && requestMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT && requestMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT;
const shouldShowDescription = !_.isEmpty(description) && !shouldShowMerchant && !isScanning;

const receiptImages = hasReceipt ? [ReceiptUtils.getThumbnailAndImageURIs(props.transaction)] : [];
Expand Down Expand Up @@ -324,7 +323,7 @@ function MoneyRequestPreview(props) {
</View>
)}
</View>
{shouldShowMerchant && (
{shouldShowMerchant && !props.isBillSplit && (
<View style={[styles.flexRow]}>
<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20, styles.breakWord]}>
{hasPendingWaypoints ? requestMerchant.replace(CONST.REGEX.FIRST_SPACE, props.translate('common.tbd')) : requestMerchant}
Expand All @@ -336,7 +335,9 @@ function MoneyRequestPreview(props) {
{!isCurrentUserManager && props.shouldShowPendingConversionMessage && (
<Text style={[styles.textLabel, styles.colorMuted]}>{props.translate('iou.pendingConversionMessage')}</Text>
)}
{shouldShowDescription && <Text style={[styles.colorMuted]}>{description}</Text>}
{(shouldShowDescription || (shouldShowMerchant && props.isBillSplit)) && (
<Text style={[styles.colorMuted]}>{shouldShowDescription ? description : requestMerchant}</Text>
)}
</View>
{props.isBillSplit && !_.isEmpty(participantAccountIDs) && requestAmount > 0 && (
<Text style={[styles.textLabel, styles.colorMuted, styles.ml1, styles.amountSplitPadding]}>
Expand Down
30 changes: 19 additions & 11 deletions src/libs/actions/IOU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,12 +919,13 @@ function requestMoney(
* @param {Number} amount - always in the smallest unit of the currency
* @param {String} comment
* @param {String} currency
* @param {String} merchant
Copy link
Contributor

Choose a reason for hiding this comment

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

also reorder this

* @param {String} category
* @param {String} existingSplitChatReportID - the report ID where the split bill happens, could be a group chat or a workspace chat
*
* @return {Object}
*/
function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, existingSplitChatReportID = '') {
function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, merchant, category, existingSplitChatReportID = '') {
dukenv0307 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 @@ -934,11 +935,6 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
const splitChatReport = existingSplitChatReport || ReportUtils.buildOptimisticChatReport(participantAccountIDs);
const isOwnPolicyExpenseChat = splitChatReport.isOwnPolicyExpenseChat;

// ReportID is -2 (aka "deleted") on the group transaction: https://github.com/Expensify/Auth/blob/3fa2698654cd4fbc30f9de38acfca3fbeb7842e4/auth/command/SplitTransaction.cpp#L24-L27
const formattedParticipants = isOwnPolicyExpenseChat
? [currentUserLogin, ReportUtils.getReportName(splitChatReport)]
: Localize.arrayToString([currentUserLogin, ..._.map(participants, (participant) => participant.login || '')]);

const splitTransaction = TransactionUtils.buildOptimisticTransaction(
amount,
currency,
Expand All @@ -947,7 +943,7 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
'',
'',
'',
`${Localize.translateLocal('iou.splitBill')} ${Localize.translateLocal('common.with')} ${formattedParticipants} [${DateUtils.getDBTime().slice(0, 10)}]`,
merchant,
undefined,
undefined,
undefined,
Expand Down Expand Up @@ -1257,9 +1253,20 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
* @param {String} currency
* @param {String} category
* @param {String} existingSplitChatReportID - Either a group DM or a workspace chat
* @param {String} merchant
*/
function splitBill(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, existingSplitChatReportID = '') {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, existingSplitChatReportID);
function splitBill(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, existingSplitChatReportID = '', merchant) {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(
participants,
currentUserLogin,
currentUserAccountID,
amount,
comment,
currency,
category,
existingSplitChatReportID,
merchant,
);
API.write(
'SplitBill',
{
Expand Down Expand Up @@ -1290,9 +1297,10 @@ function splitBill(participants, currentUserLogin, currentUserAccountID, amount,
* @param {String} comment
* @param {String} currency
* @param {String} category
* @param {String} merchant
*/
function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category) {
const {splitData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category);
function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, merchant = '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that empty string is set as merchant default value here.
And not set in all other places.
Can you make them consistent? What issue happens if default value not set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I confirmed it's redundant. Remove all default values
It's handled here:

App/src/libs/TransactionUtils.ts

Line 43 in 48b5a29

merchant = '',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

const {splitData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, currentUserAccountID, amount, comment, currency, category, '', merchant);

API.write(
'SplitBillAndOpenReport',
Expand Down
3 changes: 3 additions & 0 deletions src/pages/iou/steps/MoneyRequestConfirmPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ function MoneyRequestConfirmPage(props) {
props.iou.currency,
props.iou.category,
reportID,
props.iou.merchant,
);
return;
}
Expand All @@ -249,6 +250,7 @@ function MoneyRequestConfirmPage(props) {
trimmedComment,
props.iou.currency,
props.iou.category,
props.iou.merchant,
);
return;
}
Expand Down Expand Up @@ -280,6 +282,7 @@ function MoneyRequestConfirmPage(props) {
receiptFile,
iouType,
reportID,
props.iou.merchant,
],
);

Expand Down
4 changes: 1 addition & 3 deletions tests/actions/IOUTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,9 +1115,7 @@ describe('actions/IOU', () => {
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(
`Split bill with ${RORY_EMAIL}, ${CARLOS_EMAIL}, ${JULES_EMAIL}, and ${VIT_EMAIL} [${DateUtils.getDBTime().slice(0, 10)}]`,
);
expect(groupTransaction.merchant).toBe(CONST.TRANSACTION.DEFAULT_MERCHANT);

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