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.

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

Invert amount money request amount from negative to positive and vice-versa #18783

Merged
merged 13 commits into from
May 12, 2023
Merged
2 changes: 1 addition & 1 deletion src/components/MoneyRequestHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const defaultProps = {
};

const MoneyRequestHeader = (props) => {
const formattedAmount = CurrencyUtils.convertToDisplayString(props.report.total, props.report.currency);
const formattedAmount = CurrencyUtils.convertToDisplayString(ReportUtils.getMoneyRequestTotal(props.report), props.report.currency);
const isSettled = ReportUtils.isSettled(props.report.reportID);
const isExpenseReport = ReportUtils.isExpenseReport(props.report);
const payeeName = isExpenseReport ? ReportUtils.getPolicyName(props.report, props.policies) : ReportUtils.getDisplayNameForParticipant(props.report.managerEmail);
Expand Down
3 changes: 2 additions & 1 deletion src/components/ReportActionItem/IOUPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import reportActionPropTypes from '../../pages/home/report/reportActionPropTypes
import {showContextMenuForReport} from '../ShowContextMenuContext';
import * as OptionsListUtils from '../../libs/OptionsListUtils';
import * as CurrencyUtils from '../../libs/CurrencyUtils';
import * as ReportUtils from '../../libs/ReportUtils';

const propTypes = {
/** The active IOUReport, used for Onyx subscription */
Expand Down Expand Up @@ -146,7 +147,7 @@ const IOUPreview = (props) => {
const isCurrentUserManager = managerEmail === sessionEmail;

// If props.action is undefined then we are displaying within IOUDetailsModal and should use the full report amount
const requestAmount = props.isIOUAction ? lodashGet(props.action, 'originalMessage.amount', 0) : props.iouReport.total;
const requestAmount = props.isIOUAction ? lodashGet(props.action, 'originalMessage.amount', 0) : ReportUtils.getMoneyRequestTotal(props.iouReport);
const requestCurrency = props.isIOUAction ? lodashGet(props.action, 'originalMessage.currency', CONST.CURRENCY.USD) : props.iouReport.currency;

const getSettledMessage = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/ReportPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const defaultProps = {
};

const ReportPreview = (props) => {
const reportAmount = CurrencyUtils.convertToDisplayString(props.iouReport.total, props.iouReport.currency);
const reportAmount = CurrencyUtils.convertToDisplayString(ReportUtils.getMoneyRequestTotal(props.iouReport), props.iouReport.currency);
const managerEmail = props.iouReport.managerEmail || '';
const managerName = ReportUtils.getDisplayNameForParticipant(managerEmail, true);
const isCurrentUserManager = managerEmail === lodashGet(props.session, 'email', null);
Expand Down
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show
}

result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, iouReports);
result.iouReportAmount = ReportUtils.getIOUTotal(result, iouReports);
result.iouReportAmount = ReportUtils.getMoneyRequestTotal(result, iouReports);

if (!hasMultipleParticipants) {
result.login = personalDetail.login;
Expand Down
47 changes: 27 additions & 20 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,27 @@ function getDisplayNamesWithTooltips(participants, isMultipleParticipantReport)
});
}

/**
* @param {Object} report
* @param {String} report.iouReportID
* @param {Object} moneyRequestReports
* @returns {Number}
*/
function getMoneyRequestTotal(report, moneyRequestReports = {}) {
if (report.hasOutstandingIOU || isMoneyRequestReport(report)) {
const moneyRequestReport = moneyRequestReports[`${ONYXKEYS.COLLECTION.REPORT}${report.iouReportID}`] || report;
const total = lodashGet(moneyRequestReport, 'total', 0);

if (total !== 0) {
// There is a possibility that if the Expense report has a negative total.
// This is because there are instances where you can get a credit back on your card,
// or you enter a negative expense to “offset” future expenses
return isExpenseReport(moneyRequestReport) ? total * -1 : Math.abs(total);
}
}
return 0;
}

/**
* Get the title for a policy expense chat which depends on the role of the policy member seeing this report
*
Expand Down Expand Up @@ -900,7 +921,7 @@ function getPolicyExpenseChatName(report) {
* @returns {String}
*/
function getMoneyRequestReportName(report) {
const formattedAmount = CurrencyUtils.convertToDisplayString(report.total || 0, report.currency);
const formattedAmount = CurrencyUtils.convertToDisplayString(getMoneyRequestTotal(report), report.currency);
const payerName = isExpenseReport(report) ? getPolicyName(report) : getDisplayNameForParticipant(report.managerEmail);

return Localize.translateLocal('iou.payerOwesAmount', {payer: payerName, amount: formattedAmount});
Expand Down Expand Up @@ -1116,8 +1137,10 @@ function buildOptimisticIOUReport(payeeEmail, payerEmail, total, chatReportID, c
* @returns {Object}
*/
function buildOptimisticExpenseReport(chatReportID, policyID, payeeEmail, total, currency) {
// The amount for Expense reports are stored as negative value in the database
const storedTotal = total * -1;
const policyName = getPolicyName(allReports[`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`]);
const formattedTotal = CurrencyUtils.convertToDisplayString(total, currency);
const formattedTotal = CurrencyUtils.convertToDisplayString(storedTotal, currency);

// The expense report is always created with the policy's output currency
const outputCurrency = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, 'outputCurrency'], CONST.CURRENCY.USD);
Expand All @@ -1135,7 +1158,7 @@ function buildOptimisticExpenseReport(chatReportID, policyID, payeeEmail, total,
reportName: `${policyName} owes ${formattedTotal}`,
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
Comment on lines 1159 to 1160
Copy link
Contributor

Choose a reason for hiding this comment

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

Lol, the state and stateNum set optimistically are not the same

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are not really using state, are we?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong PR to leave this comment

total,
total: storedTotal,
};
}

Expand Down Expand Up @@ -1528,22 +1551,6 @@ function hasOutstandingIOU(report, currentUserLogin, iouReports) {
return report.hasOutstandingIOU;
}

/**
* @param {Object} report
* @param {String} report.iouReportID
* @param {Object} iouReports
* @returns {Number}
*/
function getIOUTotal(report, iouReports = {}) {
if (report.hasOutstandingIOU) {
const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT}${report.iouReportID}`];
if (iouReport) {
return iouReport.total;
}
}
return 0;
}

/**
* @param {Object} report
* @param {String} report.iouReportID
Expand Down Expand Up @@ -1945,7 +1952,7 @@ export {
hasExpensifyGuidesEmails,
hasOutstandingIOU,
isIOUOwnedByCurrentUser,
getIOUTotal,
getMoneyRequestTotal,
canShowReportRecipientLocalTime,
formatReportLastMessageText,
chatIncludesConcierge,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function getOrderedReportIDs(reportIDFromRoute) {
report.displayName = ReportUtils.getReportName(report);

// eslint-disable-next-line no-param-reassign
report.iouReportAmount = ReportUtils.getIOUTotal(report, moneyRequestReports);
report.iouReportAmount = ReportUtils.getMoneyRequestTotal(report, moneyRequestReports);
});

// The LHN is split into five distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
Expand Down Expand Up @@ -331,7 +331,7 @@ function getOptionData(reportID) {
}

result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, moneyRequestReports);
result.iouReportAmount = ReportUtils.getIOUTotal(result, moneyRequestReports);
result.iouReportAmount = ReportUtils.getMoneyRequestTotal(result, moneyRequestReports);

if (!hasMultipleParticipants) {
result.login = personalDetail.login;
Expand Down
8 changes: 5 additions & 3 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Onyx.connect({
if (!report) {
delete iouReports[key];
delete chatReports[key];
} else if (ReportUtils.isIOUReport(report)) {
} else if (ReportUtils.isMoneyRequestReport(report)) {
iouReports[key] = report;
} else {
chatReports[key] = report;
Expand Down Expand Up @@ -76,7 +76,9 @@ function requestMoney(report, amount, currency, payeeEmail, participant, comment
if (chatReport.iouReportID) {
if (isPolicyExpenseChat) {
moneyRequestReport = {...iouReports[`${ONYXKEYS.COLLECTION.REPORT}${chatReport.iouReportID}`]};
moneyRequestReport.total += amount;

// Because of the Expense reports are stored as negative values, we substract the total from the amount
moneyRequestReport.total = ReportUtils.isExpenseReport(moneyRequestReport) ? moneyRequestReport.total - amount : moneyRequestReport.total + amount;
} else {
moneyRequestReport = IOUUtils.updateIOUOwnerAndTotal(iouReports[`${ONYXKEYS.COLLECTION.REPORT}${chatReport.iouReportID}`], payeeEmail, amount, currency);
}
Expand Down Expand Up @@ -776,7 +778,7 @@ function setMoneyRequestDescription(comment) {
* @returns {String}
*/
function buildPayPalPaymentUrl(amount, submitterPayPalMeAddress, currency) {
return `https://paypal.me/${submitterPayPalMeAddress}/${amount / 100}${currency}`;
return `https://paypal.me/${submitterPayPalMeAddress}/${Math.abs(amount) / 100}${currency}`;
}

/**
Expand Down
14 changes: 7 additions & 7 deletions tests/unit/SidebarOrderTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ describe('Sidebar', () => {
expect(screen.queryAllByTestId('Pin Icon')).toHaveLength(1);
expect(screen.queryAllByTestId('Pencil Icon')).toHaveLength(1);
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('One, Two');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Three, Four');
})
);
Expand Down Expand Up @@ -733,11 +733,11 @@ describe('Sidebar', () => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames).toHaveLength(5);
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('One, Two');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four');
expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Two owes $100.00');
expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('One, Two');
})
);
});
Expand Down