Skip to content

Expense - Negative sign disappears after moving expense with negative sign to self DM #65920

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

Merged
youssef-lr merged 11 commits into from
Jul 16, 2025
Merged

Expense - Negative sign disappears after moving expense with negative sign to self DM #65920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
13f08ed
fix: negative sign unreported expense
thelullabyy Jul 11, 2025
88eeb4b
fix: prettier
thelullabyy Jul 11, 2025
7d247fd
fix: prettier
thelullabyy Jul 11, 2025
519dbcd
Merge branch 'main' of https://github.com/Expensify/App into fix/6479…
thelullabyy Jul 11, 2025
97f63bb
Merge branch 'main' of https://github.com/Expensify/App into fix/6479…
thelullabyy Jul 11, 2025
b3f30d9
Merge branch 'main' of https://github.com/Expensify/App into fix/6479…
thelullabyy Jul 11, 2025
1b7fa95
Merge branch 'main' of https://github.com/Expensify/App into fix/6479…
thelullabyy Jul 14, 2025
fea1592
fix: resolved comments
thelullabyy Jul 14, 2025
be22021
Merge branch 'main' of https://github.com/Expensify/App into fix/6479…
thelullabyy Jul 15, 2025
5da0d43
chore: merge main
thelullabyy Jul 16, 2025
f3679a5
chore: merge main
thelullabyy Jul 16, 2025
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
14 changes: 7 additions & 7 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3970,7 +3970,7 @@ function getTransactionDetails(
const report = getReportOrDraftReport(transaction?.reportID);
return {
created: getFormattedCreated(transaction, createdDateFormat),
amount: getTransactionAmount(transaction, !isEmptyObject(report) && isExpenseReport(report)),
amount: getTransactionAmount(transaction, !isEmptyObject(report) && isExpenseReport(report), transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID),
attendees: getAttendees(transaction),
taxAmount: getTaxAmount(transaction, !isEmptyObject(report) && isExpenseReport(report)),
taxCode: getTaxCode(transaction),
Expand Down Expand Up @@ -4230,8 +4230,8 @@ function canEditFieldOfMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction>

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.REPORT) {
// Unreported transaction from OldDot can have the reportID as an empty string
const isUnreported = !transaction?.reportID || transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
return isUnreported
const isUnreportedExpense = !transaction?.reportID || transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
return isUnreportedExpense
? Object.values(allPolicies ?? {}).flatMap((currentPolicy) =>
getOutstandingReportsForUser(currentPolicy?.id, currentUserAccountID, reportsByPolicyID?.[currentPolicy?.id ?? CONST.DEFAULT_NUMBER_ID] ?? {}),
).length > 0
Expand Down Expand Up @@ -4482,7 +4482,7 @@ function getTransactionReportName({
}

const report = getReportOrDraftReport(transaction?.reportID, reports);
const amount = getTransactionAmount(transaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const amount = getTransactionAmount(transaction, !isEmptyObject(report) && isExpenseReport(report), transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) ?? 0;
const formattedAmount = convertToDisplayString(amount, getCurrency(transaction)) ?? '';
const comment = getMerchantOrDescription(transaction);

Expand Down Expand Up @@ -4537,7 +4537,7 @@ function getReportPreviewMessage(
return translateLocal('iou.receiptMissingDetails');
}

const amount = getTransactionAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const amount = getTransactionAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report), linkedTransaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) ?? 0;
const formattedAmount = convertToDisplayString(amount, getCurrency(linkedTransaction)) ?? '';
return translateLocal('iou.didSplitAmount', {formattedAmount, comment: getMerchantOrDescription(linkedTransaction)});
}
Expand All @@ -4559,7 +4559,7 @@ function getReportPreviewMessage(
return translateLocal('iou.receiptMissingDetails');
}

const amount = getTransactionAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const amount = getTransactionAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report), linkedTransaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) ?? 0;
const formattedAmount = convertToDisplayString(amount, getCurrency(linkedTransaction)) ?? '';
return translateLocal('iou.trackedAmount', {formattedAmount, comment: getMerchantOrDescription(linkedTransaction)});
}
Expand Down Expand Up @@ -9203,7 +9203,7 @@ function getIOUReportActionDisplayMessage(reportAction: OnyxEntry<ReportAction>,
return translateLocal(translationKey, {amount: formattedAmount, payer: ''});
}

const amount = getTransactionAmount(transaction, !isEmptyObject(iouReport) && isExpenseReport(iouReport)) ?? 0;
const amount = getTransactionAmount(transaction, !isEmptyObject(iouReport) && isExpenseReport(iouReport), transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) ?? 0;
const formattedAmount = convertToDisplayString(amount, getCurrency(transaction)) ?? '';
const isRequestSettled = isSettled(IOUReportID);
const isApproved = isReportApproved({report: iouReport});
Expand Down
14 changes: 8 additions & 6 deletions src/libs/TransactionUtils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ function getUpdatedTransaction({
shouldUpdateReceiptState?: boolean;
policy?: OnyxEntry<Policy>;
}): Transaction {
const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;

// Only changing the first level fields so no need for deep clone now
const updatedTransaction = lodashDeepClone(transaction);
let shouldStopSmartscan = false;
Expand All @@ -429,7 +431,7 @@ function getUpdatedTransaction({
shouldStopSmartscan = true;
}
if (Object.hasOwn(transactionChanges, 'amount') && typeof transactionChanges.amount === 'number') {
updatedTransaction.modifiedAmount = isFromExpenseReport ? -transactionChanges.amount : transactionChanges.amount;
updatedTransaction.modifiedAmount = isFromExpenseReport || isUnReportedExpense ? -transactionChanges.amount : transactionChanges.amount;
shouldStopSmartscan = true;

@ikevin127 ikevin127 Jul 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@thelullabyy Not supposed to remove shouldStopSmartscan = true; from here, right ?

@thelullabyy thelullabyy Jul 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noticing this. I've reverted the code

}
if (Object.hasOwn(transactionChanges, 'currency')) {
Expand Down Expand Up @@ -458,7 +460,7 @@ function getUpdatedTransaction({

const distanceInMeters = getDistanceInMeters(transaction, unit);
const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0);
const updatedAmount = isFromExpenseReport ? -amount : amount;
const updatedAmount = isFromExpenseReport || isUnReportedExpense ? -amount : amount;
const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, transaction.currency, translateLocal, (digit) =>
toLocaleDigit(IntlStore.getCurrentLocale(), digit),
);
Expand Down Expand Up @@ -497,7 +499,7 @@ function getUpdatedTransaction({

const distanceInMeters = getDistanceInMeters(transaction, oldMileageRate?.unit);
const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0);
const updatedAmount = isFromExpenseReport ? -amount : amount;
const updatedAmount = isFromExpenseReport || isUnReportedExpense ? -amount : amount;
const updatedCurrency = updatedMileageRate.currency ?? CONST.CURRENCY.USD;
const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, updatedCurrency, translateLocal, (digit) =>
toLocaleDigit(IntlStore.getCurrentLocale(), digit),
Expand Down Expand Up @@ -583,7 +585,7 @@ function getDescription(transaction: OnyxInputOrEntry<Transaction>): string {
*/
function getAmount(transaction: OnyxInputOrEntry<Transaction>, isFromExpenseReport = false, isFromTrackedExpense = false): number {
// IOU requests cannot have negative values, but they can be stored as negative values, let's return absolute value
if (!isFromExpenseReport || isFromTrackedExpense) {
if (!isFromExpenseReport && !isFromTrackedExpense) {
const amount = transaction?.modifiedAmount ?? 0;
if (amount) {
return Math.abs(amount);
Expand Down Expand Up @@ -687,9 +689,9 @@ function isUnreportedAndHasInvalidDistanceRateTransaction(transaction: OnyxInput
// eslint-disable-next-line deprecation/deprecation
const policy = policyParam ?? getPolicy(report?.policyID);
const {rate} = DistanceRequestUtils.getRate({transaction, policy});
const isUnreported = !transaction.reportID || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
const isUnreportedExpense = !transaction.reportID || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;

if (isUnreported && !rate) {
if (isUnreportedExpense && !rate) {
return true;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/libs/actions/Transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,12 @@ function changeTransactionsReport(transactionIDs: string[], reportID: string, po
let transactionsMoved = false;

transactions.forEach((transaction) => {
const isUnreported = !transaction.reportID || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
const isUnreportedExpense = !transaction.reportID || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;

// We'll handle optimistically creating the selfDM as part of https://github.com/Expensify/App/issues/60288
const selfDMReportID = findSelfDMReportID() ?? CONST.REPORT.UNREPORTED_REPORT_ID;

const oldIOUAction = getIOUActionForReportID(isUnreported ? selfDMReportID : transaction.reportID, transaction.transactionID);
const oldIOUAction = getIOUActionForReportID(isUnreportedExpense ? selfDMReportID : transaction.reportID, transaction.transactionID);
if (!transaction.reportID || transaction.reportID === reportID) {
return;
}
Expand Down Expand Up @@ -699,7 +699,7 @@ function changeTransactionsReport(transactionIDs: string[], reportID: string, po
created: oldIOUAction?.created ?? DateUtils.getDBTime(),
};

const trackExpenseActionableWhisper = isUnreported ? getTrackExpenseActionableWhisper(transaction.transactionID, selfDMReportID) : undefined;
const trackExpenseActionableWhisper = isUnreportedExpense ? getTrackExpenseActionableWhisper(transaction.transactionID, selfDMReportID) : undefined;

if (oldIOUAction) {
optimisticData.push({
Expand All @@ -712,7 +712,7 @@ function changeTransactionsReport(transactionIDs: string[], reportID: string, po

optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${isUnreported ? selfDMReportID : oldReportID}`,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${isUnreportedExpense ? selfDMReportID : oldReportID}`,
value: {
[oldIOUAction.reportActionID]: {
previousMessage: oldIOUAction.message,
Expand Down Expand Up @@ -753,7 +753,7 @@ function changeTransactionsReport(transactionIDs: string[], reportID: string, po
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${isUnreported ? selfDMReportID : oldReportID}`,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${isUnreportedExpense ? selfDMReportID : oldReportID}`,
value: {
[oldIOUAction.reportActionID]: oldIOUAction,
...(trackExpenseActionableWhisper ? {[trackExpenseActionableWhisper.reportActionID]: trackExpenseActionableWhisper} : {}),
Expand All @@ -778,7 +778,7 @@ function changeTransactionsReport(transactionIDs: string[], reportID: string, po
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${oldIOUAction.childReportID}`,
value: {
parentReportID: isUnreported ? selfDMReportID : oldReportID,
parentReportID: isUnreportedExpense ? selfDMReportID : oldReportID,
optimisticMoneyRequestReportActionID: oldIOUAction.reportActionID,
policyID: allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oldIOUAction.reportActionID}`]?.policyID,
},
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/IOUUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,4 +383,16 @@ describe('Check valid amount for IOU/Expense request', () => {
const expenseAmount = TransactionUtils.getAmount(expenseTransaction, true, false);
expect(expenseAmount).toBeLessThan(0);
});

test('Unreported expense amount should retain negative sign', () => {
const unreportedTransaction = TransactionUtils.buildOptimisticTransaction({
transactionParams: {
amount: 100,
currency: 'USD',
reportID: CONST.REPORT.UNREPORTED_REPORT_ID,
},
});
const unreportedAmount = TransactionUtils.getAmount(unreportedTransaction, true, false);
expect(unreportedAmount).toBeLessThan(0);
});
});