Skip to content

Commit

Permalink
Merge pull request #28542 from Expensify/vit-fixRequestMoney
Browse files Browse the repository at this point in the history 
Make the logic for money request options in composer quick actions more robust
  • Loading branch information
@deetergp
deetergp authored Oct 2, 2023
2 parents 832ca24 + d5adca2 commit 943031a
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 25 deletions.
73 changes: 59 additions & 14 deletions src/libs/ReportUtils.js
View file
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
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', () => {
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

0 comments on commit 943031a

Please sign in to comment.