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

Ignore hasOutstandingIOU from child iouReport #24448

Merged
merged 8 commits into from
Aug 15, 2023
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ function getOptions(
const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase();

// Filter out all the reports that shouldn't be displayed
const filteredReports = _.filter(reports, (report) => ReportUtils.shouldReportBeInOptionList(report, Navigation.getTopmostReportId(), false, null, betas, policies));
const filteredReports = _.filter(reports, (report) => ReportUtils.shouldReportBeInOptionList(report, Navigation.getTopmostReportId(), false, betas, policies));

// Sorting the reports works like this:
// - Order everything by the last message timestamp (descending)
Expand Down
22 changes: 6 additions & 16 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1136,12 +1136,10 @@ function getMoneyRequestAction(reportAction = {}) {
* Determines if a report has an IOU that is waiting for an action from the current user (either Pay or Add a credit bank account)
*
* @param {Object} report (chatReport or iouReport)
* @param {Object} allReportsDict
* @returns {boolean}
*/
function isWaitingForIOUActionFromCurrentUser(report, allReportsDict = null) {
const allAvailableReports = allReportsDict || allReports;
if (!report || !allAvailableReports) {
function isWaitingForIOUActionFromCurrentUser(report) {
if (!report) {
return false;
}

Expand All @@ -1150,15 +1148,8 @@ function isWaitingForIOUActionFromCurrentUser(report, allReportsDict = null) {
return true;
}

let reportToLook = report;
if (report.iouReportID) {
const iouReport = allAvailableReports[`${ONYXKEYS.COLLECTION.REPORT}${report.iouReportID}`];
if (iouReport) {
reportToLook = iouReport;
}
}
// Money request waiting for current user to Pay (from chat or from iou report)
if (reportToLook.ownerAccountID && (reportToLook.ownerAccountID !== currentUserAccountID || currentUserAccountID === reportToLook.managerID) && reportToLook.hasOutstandingIOU) {
// Money request waiting for current user to Pay (from expense or iou report)
if (report.hasOutstandingIOU && report.ownerAccountID && (report.ownerAccountID !== currentUserAccountID || currentUserAccountID === report.managerID)) {
return true;
}

Expand Down Expand Up @@ -2464,14 +2455,13 @@ function canAccessReport(report, policies, betas, allReportActions) {
* @param {Object} report
* @param {String} currentReportId
* @param {Boolean} isInGSDMode
* @param {Object} iouReports
* @param {String[]} betas
* @param {Object} policies
* @param {Object} allReportActions
* @param {Boolean} excludeEmptyChats
* @returns {boolean}
*/
function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouReports, betas, policies, allReportActions, excludeEmptyChats = false) {
function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions, excludeEmptyChats = false) {
const isInDefaultMode = !isInGSDMode;

// Exclude reports that have no data because there wouldn't be anything to show in the option item.
Expand All @@ -2498,7 +2488,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouRep
}

// Include reports that are relevant to the user in any view mode. Criteria include having a draft, having an outstanding IOU, or being assigned to an open task.
if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report, iouReports) || isWaitingForTaskCompleteFromAssignee(report)) {
if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report) || isWaitingForTaskCompleteFromAssignee(report)) {
return true;
}

Expand Down
6 changes: 2 additions & 4 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p
const isInDefaultMode = !isInGSDMode;

// Filter out all the reports that shouldn't be displayed
const reportsToDisplay = _.filter(allReportsDict, (report) =>
ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, allReportsDict, betas, policies, allReportActions, true),
);
const reportsToDisplay = _.filter(allReportsDict, (report) => ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions, true));

if (_.isEmpty(reportsToDisplay)) {
// Display Concierge chat report when there is no report to be displayed
Expand Down Expand Up @@ -131,7 +129,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p
return;
}

if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report, allReportsDict)) {
if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report)) {
outstandingIOUReports.push(report);
return;
}
Expand Down
82 changes: 19 additions & 63 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,97 +288,53 @@ describe('ReportUtils', () => {
it('returns false when there is no report', () => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser()).toBe(false);
});
it('returns false when there is no reports collection', () => {
it('returns false when the matched IOU report does not have an owner accountID', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
ownerAccountID: undefined,
hasOutstandingIOU: true,
};
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
it('returns false when the report has no iouReportID', () => {
const report = LHNTestUtils.getFakeReport();
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}2`, {
reportID: '2',
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns false when there is no matching IOU report', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}2`, {
reportID: '2',
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns false when the matched IOU report does not have an owner email', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, {
reportID: '1',
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns false when the matched IOU report does not have an owner email', () => {
it('returns false when the linked iou report has an oustanding IOU', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, {
reportID: '1',
ownerAccountID: 99,
hasOutstandingIOU: true,
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns true when the report has an oustanding IOU', () => {
it('returns true when the report has no oustanding IOU but is waiting for a bank account and the logged user is the report owner', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
hasOutstandingIOU: true,
hasOutstandingIOU: false,
ownerAccountID: currentUserAccountID,
isWaitingOnBankAccount: true,
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, {
reportID: '1',
ownerAccountID: 99,
hasOutstandingIOU: true,
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
it('returns false when the report has no oustanding IOU', () => {
it('returns true when the report has no oustanding IOU but is waiting for a bank account and the logged user is not the report owner', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
hasOutstandingIOU: false,
ownerAccountID: 97,
isWaitingOnBankAccount: true,
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, {
reportID: '1',
ownerAccountID: 99,
hasOutstandingIOU: false,
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
it('returns true when the report has no oustanding IOU but is waiting for a bank account', () => {
it('returns true when the report has oustanding IOU', () => {
const report = {
...LHNTestUtils.getFakeReport(),
iouReportID: '1',
hasOutstandingIOU: false,
ownerAccountID: 99,
hasOutstandingIOU: true,
isWaitingOnBankAccount: false,
};
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, {
reportID: '1',
ownerAccountID: currentUserEmail,
hasOutstandingIOU: false,
isWaitingOnBankAccount: true,
}).then(() => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
});

Expand Down
65 changes: 50 additions & 15 deletions tests/unit/SidebarOrderTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ describe('Sidebar', () => {
};
const report3 = {
...LHNTestUtils.getFakeReport([5, 6], 1),
hasOutstandingIOU: true,
hasOutstandingIOU: false,

// This has to be added after the IOU report is generated
iouReportID: null,
Expand Down Expand Up @@ -427,13 +427,12 @@ describe('Sidebar', () => {
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames).toHaveLength(4);
expect(displayNames).toHaveLength(3);
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('Email Two owes $100.00');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Three, Four');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four');
})
);
});
Expand Down Expand Up @@ -700,21 +699,31 @@ describe('Sidebar', () => {
// Given three IOU reports containing the same IOU amounts
const report1 = {
...LHNTestUtils.getFakeReport([1, 2]),
hasOutstandingIOU: true,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report2 = {
...LHNTestUtils.getFakeReport([3, 4]),
hasOutstandingIOU: true,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report3 = {
...LHNTestUtils.getFakeReport([5, 6]),
hasOutstandingIOU: true,
hasOutstandingIOU: false,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report4 = {
...LHNTestUtils.getFakeReport([5, 6]),

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report5 = {
...LHNTestUtils.getFakeReport([5, 6]),

// This has to be added after the IOU report is generated
iouReportID: null,
Expand All @@ -733,7 +742,7 @@ describe('Sidebar', () => {
...LHNTestUtils.getFakeReport([9, 10]),
type: CONST.REPORT.TYPE.IOU,
ownerAccountID: 2,
managerID: 2,
managerID: 3,
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
Expand All @@ -743,7 +752,27 @@ describe('Sidebar', () => {
...LHNTestUtils.getFakeReport([11, 12]),
type: CONST.REPORT.TYPE.IOU,
ownerAccountID: 2,
managerID: 2,
managerID: 4,
hasOutstandingIOU: true,
total: 100000,
currency: 'USD',
chatReportID: report3.reportID,
};
const iouReport4 = {
...LHNTestUtils.getFakeReport([11, 12]),
type: CONST.REPORT.TYPE.IOU,
ownerAccountID: 2,
managerID: 5,
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
chatReportID: report3.reportID,
};
const iouReport5 = {
...LHNTestUtils.getFakeReport([11, 12]),
type: CONST.REPORT.TYPE.IOU,
ownerAccountID: 2,
managerID: 6,
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
Expand All @@ -753,6 +782,8 @@ describe('Sidebar', () => {
report1.iouReportID = iouReport1.reportID;
report2.iouReportID = iouReport2.reportID;
report3.iouReportID = iouReport3.reportID;
report4.iouReportID = iouReport4.reportID;
report5.iouReportID = iouReport5.reportID;

const currentlyLoggedInUserAccountID = 13;
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
Expand All @@ -768,22 +799,26 @@ describe('Sidebar', () => {
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
[`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`]: report4,
[`${ONYXKEYS.COLLECTION.REPORT}${report5.reportID}`]: report5,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport1.reportID}`]: iouReport1,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport2.reportID}`]: iouReport2,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport3.reportID}`]: iouReport3,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport4.reportID}`]: iouReport4,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport5.reportID}`]: iouReport5,
}),
)

// Then the reports are ordered alphabetically since their amounts are the same
// Then the reports with the same amount are ordered alphabetically
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames).toHaveLength(5);
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');
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Four owes $1,000.00');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Five owes $100.00');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Six owes $100.00');
expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Email Three owes $100.00');
expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('Email Two owes $100.00');
})
);
});
Expand Down