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

Make the logic for money request options in composer quick actions more robust #28542

Merged
merged 6 commits into from
Oct 2, 2023
Merged

Make the logic for money request options in composer quick actions more robust #28542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db0ef88
Fix the request money option
mountiny Sep 29, 2023
a4ac8f9
Merge branch 'main' into vit-fixRequestMoney
mountiny Sep 30, 2023
c376fd3
Add tests to cover the getMoneRequestOptions better
mountiny Sep 30, 2023
0d493e3
clean up the getMoneyRequestOptions
mountiny Sep 30, 2023
9777a2c
Clean up the getMoneyRequestOptions logic
mountiny Sep 30, 2023
d5adca2
Update tests
mountiny Sep 30, 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
73 changes: 59 additions & 14 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2082,6 +2082,7 @@ function buildOptimisticIOUReport(payeeAccountID, payerAccountID, total, chatRep
currency,
managerID: payerAccountID,
ownerAccountID: payeeAccountID,
participantAccountIDs: [payeeAccountID, payerAccountID],
reportID: generateReportID(),
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: isSendingMoney ? CONST.REPORT.STATE_NUM.SUBMITTED : CONST.REPORT.STATE_NUM.PROCESSING,
Expand Down Expand Up @@ -3224,20 +3225,67 @@ function hasIOUWaitingOnCurrentUserBankAccount(chatReport) {
}

/**
* Users can request money in policy expense chats only if they are in a role of a member in the chat (in other words, if it's their policy expense chat)
* Users can request money:
* - in policy expense chats only if they are in a role of a member in the chat (in other words, if it's their policy expense chat)
* - in an open or submitted expense report tied to a policy expense chat the user owns
* - employee can request money in submitted expense report only if the policy has Instant Submit settings turned on
* - in an IOU report, which is not settled yet
* - in DM chat
*
* @param {Object} report
* @param {Array<Number>} participants
* @returns {Boolean}
*/
function canRequestMoney(report) {
// Prevent requesting money if pending iou waiting for their bank account already exists.
function canRequestMoney(report, participants) {
// User cannot request money in chat thread or in task report
if (isChatThread(report) || isTaskReport(report)) {
return false;
}

// Prevent requesting money if pending IOU report waiting for their bank account already exists
if (hasIOUWaitingOnCurrentUserBankAccount(report)) {
return false;
}
return !isPolicyExpenseChat(report) || report.isOwnPolicyExpenseChat;

// In case of expense reports, we have to look at the parent workspace chat to get the isOwnPolicyExpenseChat property
let isOwnPolicyExpenseChat = report.isOwnPolicyExpenseChat || false;
if (isExpenseReport(report) && getParentReport(report)) {
isOwnPolicyExpenseChat = getParentReport(report).isOwnPolicyExpenseChat;
}

// In case there are no other participants than the current user and it's not user's own policy expense chat, they can't request money from such report
if (participants.length === 0 && !isOwnPolicyExpenseChat) {
return false;
}

// User can request money in any IOU report, unless paid, but user can only request money in an expense report
// which is tied to their workspace chat.
if (isMoneyRequestReport(report)) {
return ((isExpenseReport(report) && isOwnPolicyExpenseChat) || isIOUReport(report)) && !isReportApproved(report) && !isSettled(report.reportID);
}

// In case of policy expense chat, users can only request money from their own policy expense chat
return !isPolicyExpenseChat(report) || isOwnPolicyExpenseChat;
}

/**
* Helper method to define what money request options we want to show for particular method.
* There are 3 money request options: Request, Split and Send:
* - Request option should show for:
* - DMs
* - own policy expense chats
* - open and processing expense reports tied to own policy expense chat
* - unsettled IOU reports
* - Send option should show for:
* - DMs
* - Split options should show for:
* - chat/ policy rooms with more than 1 participants
* - groups chats with 3 and more participants
* - corporate workspace chats
*
* None of the options should show in chat threads or if there is some special Expensify account
* as a participant of the report.
*
* @param {Object} report
* @param {Array<Number>} reportParticipants
* @param {Array} betas
Expand All @@ -3251,31 +3299,28 @@ function getMoneyRequestOptions(report, reportParticipants, betas) {

const participants = _.filter(reportParticipants, (accountID) => currentUserPersonalDetails.accountID !== accountID);

// Verify if there is any of the expensify accounts amongst the participants in which case user cannot take IOU actions on such report
const hasExcludedIOUAccountIDs = lodashIntersection(reportParticipants, CONST.EXPENSIFY_ACCOUNT_IDS).length > 0;
const hasSingleParticipantInReport = participants.length === 1;
const hasMultipleParticipants = participants.length > 1;

if (hasExcludedIOUAccountIDs || (participants.length === 0 && !report.isOwnPolicyExpenseChat)) {
return [];
}

// Additional requests should be blocked for money request reports if it is approved or reimbursed
if (isMoneyRequestReport(report) && (isReportApproved(report) || isSettled(report.reportID))) {
if (hasExcludedIOUAccountIDs) {
return [];
}

// User created policy rooms and default rooms like #admins or #announce will always have the Split Bill option
// unless there are no participants at all (e.g. #admins room for a policy with only 1 admin)
// DM chats will have the Split Bill option only when there are at least 3 people in the chat.
// There is no Split Bill option for Workspace chats
if (isChatRoom(report) || (hasMultipleParticipants && !isPolicyExpenseChat(report)) || isControlPolicyExpenseChat(report)) {
// There is no Split Bill option for Workspace chats, IOU or Expense reports which are threads
if ((isChatRoom(report) && participants.length > 0) || (hasMultipleParticipants && !isPolicyExpenseChat(report) && !isMoneyRequestReport(report)) || isControlPolicyExpenseChat(report)) {
return [CONST.IOU.MONEY_REQUEST_TYPE.SPLIT];
}

// DM chats that only have 2 people will see the Send / Request money options.
// Workspace chats should only see the Request money option, as "easy overages" is not available.
// IOU and open or processing expense reports should show the Request option.
// Workspace chats should only see the Request money option or Split option in case of Control policies
return [
...(canRequestMoney(report) ? [CONST.IOU.MONEY_REQUEST_TYPE.REQUEST] : []),
...(canRequestMoney(report, participants) ? [CONST.IOU.MONEY_REQUEST_TYPE.REQUEST] : []),

// Send money option should be visible only in DMs
...(Permissions.canUseIOUSend(betas) && isChatReport(report) && !isPolicyExpenseChat(report) && hasSingleParticipantInReport ? [CONST.IOU.MONEY_REQUEST_TYPE.SEND] : []),
Expand Down
148 changes: 137 additions & 11 deletions tests/unit/ReportUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,22 +434,82 @@ describe('ReportUtils', () => {
afterAll(() => Onyx.clear());

describe('return empty iou options if', () => {
it('participants contains excluded iou emails', () => {
it('participants aray contains excluded expensify iou emails', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: typo in the word array

const allEmpty = _.every(CONST.EXPENSIFY_ACCOUNT_IDS, (accountID) => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({}, [currentUserAccountID, accountID], []);
return moneyRequestOptions.length === 0;
});
expect(allEmpty).toBe(true);
});

it('no participants except self', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({}, [currentUserAccountID], []);
it('it is a room with no participants except self', () => {
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], []);
expect(moneyRequestOptions.length).toBe(0);
});

it('its not your policy expense chat', () => {
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: false,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], []);
expect(moneyRequestOptions.length).toBe(0);
});

it('its paid IOU report', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.IOU,
statusNum: CONST.REPORT.STATUS.REIMBURSED,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], []);
expect(moneyRequestOptions.length).toBe(0);
});

it('its approved Expense report', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS.APPROVED,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], []);
expect(moneyRequestOptions.length).toBe(0);
});

it('its paid Expense report', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.EXPENSE,
statusNum: CONST.REPORT.STATUS.REIMBURSED,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], []);
expect(moneyRequestOptions.length).toBe(0);
});

it('it is an expense report tied to a policy expense chat user does not own', () => {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}100`, {
reportID: '100',
isOwnPolicyExpenseChat: false,
}).then(() => {
const report = {
...LHNTestUtils.getFakeReport(),
parentReportID: '100',
type: CONST.REPORT.TYPE.EXPENSE,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(0);
});
});
});

describe('return only iou split option if', () => {
it('a chat room', () => {
it('it is a chat room with more than one participant', () => {
const onlyHaveSplitOption = _.every(
[CONST.REPORT.CHAT_TYPE.POLICY_ADMINS, CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE, CONST.REPORT.CHAT_TYPE.DOMAIN_ALL, CONST.REPORT.CHAT_TYPE.POLICY_ROOM],
(chatType) => {
Expand All @@ -464,21 +524,40 @@ describe('ReportUtils', () => {
expect(onlyHaveSplitOption).toBe(true);
});

it('has multiple participants exclude self', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({}, [currentUserAccountID, ...participantsAccountIDs], []);
it('has multiple participants excluding self', () => {
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, ...participantsAccountIDs], []);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SPLIT)).toBe(true);
});

it(' does not have iou send permission', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({}, [currentUserAccountID, ...participantsAccountIDs], []);
it('user has send money permission', () => {
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, ...participantsAccountIDs], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SPLIT)).toBe(true);
});

it("it's a group chat report", () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.CHAT,
participantsAccountIDs: [currentUserAccountID, ...participantsAccountIDs],
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, ...participantsAccountIDs], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SPLIT)).toBe(true);
});
});

describe('return only iou request option if', () => {
it('a policy expense chat', () => {
describe('return only money request option if', () => {
it("it is user's own policy expense chat", () => {
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
Expand All @@ -488,10 +567,57 @@ describe('ReportUtils', () => {
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
});

it("it is an expense report tied to user's own policy expense chat", () => {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}101`, {
reportID: '101',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: true,
}).then(() => {
const report = {
...LHNTestUtils.getFakeReport(),
parentReportID: '101',
type: CONST.REPORT.TYPE.EXPENSE,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
});
});

it('it is an IOU report in submitted state', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.IOU,
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
statusNum: CONST.REPORT.STATUS.SUBMITTED,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, participantsAccountIDs[0]], []);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
});

it('it is an IOU report in submitted state even with send money permissions', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.IOU,
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
statusNum: CONST.REPORT.STATUS.SUBMITTED,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, participantsAccountIDs[0]], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
});
});

it('return both iou send and request money in DM', () => {
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions({type: 'chat'}, [currentUserAccountID, participantsAccountIDs[0]], [CONST.BETAS.IOU_SEND]);
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.CHAT,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, participantsAccountIDs[0]], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(2);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.REQUEST)).toBe(true);
expect(moneyRequestOptions.includes(CONST.IOU.MONEY_REQUEST_TYPE.SEND)).toBe(true);
Expand Down