Skip to content

Commit

Permalink
Merge pull request Expensify#24448 from Expensify/aldo_ignore-has-out…
Browse files Browse the repository at this point in the history 
…standing-iou-child-report

Ignore hasOutstandingIOU from child iouReport
  • Loading branch information
@aldo-expensify
aldo-expensify authored Aug 15, 2023
2 parents 3204802 + 08d447c commit 7f13cdd
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 99 deletions.
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.js
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 7f13cdd

Please sign in to comment.