Skip to content

Commit

Permalink
Merge pull request #31025 from Expensify/puneet-request-options
Browse files Browse the repository at this point in the history
Update money request options for different room types
  • Loading branch information
pecanoro authored Nov 8, 2023
2 parents fac5b04 + 2300d06 commit fae9178
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 49 deletions.
58 changes: 30 additions & 28 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ function isChatThread(report) {
* @returns {Boolean}
*/
function isDM(report) {
return !getChatType(report);
return isChatReport(report) && !getChatType(report);
}

/**
Expand Down Expand Up @@ -3617,18 +3617,23 @@ function hasIOUWaitingOnCurrentUserBankAccount(chatReport) {
* - 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
* - in a 1:1 DM chat
*
* @param {Object} report
* @param {Array<Number>} participants
* @param {Array<Number>} otherParticipants
* @returns {Boolean}
*/
function canRequestMoney(report, participants) {
// User cannot request money in chat thread or in task report
if (isChatThread(report) || isTaskReport(report)) {
function canRequestMoney(report, otherParticipants) {
// User cannot request money in chat thread or in task report or in chat room
if (isChatThread(report) || isTaskReport(report) || isChatRoom(report)) {
return false;
}

// Users can only request money in DMs if they are a 1:1 DM
if (isDM(report)) {
return otherParticipants.length === 1;
}

// Prevent requesting money if pending IOU report waiting for their bank account already exists
if (hasIOUWaitingOnCurrentUserBankAccount(report)) {
return false;
Expand All @@ -3641,7 +3646,7 @@ function canRequestMoney(report, participants) {
}

// 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) {
if (otherParticipants.length === 0 && !isOwnPolicyExpenseChat) {
return false;
}

Expand Down Expand Up @@ -3683,39 +3688,36 @@ function getMoneyRequestOptions(report, reportParticipants) {
return [];
}

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

// We don't allow IOU actions if an Expensify account is a participant of the report, unless the policy that the report is on is owned by an Expensify account
const doParticipantsIncludeExpensifyAccounts = lodashIntersection(reportParticipants, CONST.EXPENSIFY_ACCOUNT_IDS).length > 0;
const isPolicyOwnedByExpensifyAccounts = report.policyID ? CONST.EXPENSIFY_ACCOUNT_IDS.includes(getPolicy(report.policyID).ownerAccountID || 0) : false;
if (doParticipantsIncludeExpensifyAccounts && !isPolicyOwnedByExpensifyAccounts) {
return [];
}

const hasSingleParticipantInReport = participants.length === 1;
const hasMultipleParticipants = participants.length > 1;
const otherParticipants = _.filter(reportParticipants, (accountID) => currentUserPersonalDetails.accountID !== accountID);
const hasSingleOtherParticipantInReport = otherParticipants.length === 1;
const hasMultipleOtherParticipants = otherParticipants.length > 1;
let options = [];

// 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 IOU or Expense reports which are threads
if (
(isChatRoom(report) && participants.length > 0) ||
(hasMultipleParticipants && !isPolicyExpenseChat(report) && !isMoneyRequestReport(report)) ||
(isControlPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat)
) {
return [CONST.IOU.TYPE.SPLIT];
// unless there are no other 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 2 other people in the chat.
// Your own workspace chats will have the split bill option.
if ((isChatRoom(report) && otherParticipants.length > 0) || (isDM(report) && hasMultipleOtherParticipants) || (isPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat)) {
options = [CONST.IOU.TYPE.SPLIT];
}

// DM chats that only have 2 people will see the Send / Request money options.
// 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, participants) ? [CONST.IOU.TYPE.REQUEST] : []),
if (canRequestMoney(report, otherParticipants)) {
options = [...options, CONST.IOU.TYPE.REQUEST];
}

// Send money option should be visible only in DMs
...(isChatReport(report) && !isPolicyExpenseChat(report) && hasSingleParticipantInReport ? [CONST.IOU.TYPE.SEND] : []),
];
// Send money option should be visible only in 1:1 DMs
if (isDM(report) && hasSingleOtherParticipantInReport) {
options = [...options, CONST.IOU.TYPE.SEND];
}

return options;
}

/**
Expand Down
45 changes: 24 additions & 21 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ describe('ReportUtils', () => {
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.SPLIT)).toBe(true);
});

it("it's a group chat report", () => {
it("it's a group DM report", () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.CHAT,
Expand All @@ -465,17 +465,6 @@ describe('ReportUtils', () => {
});

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,
isOwnPolicyExpenseChat: true,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, ...participantsAccountIDs], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.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',
Expand Down Expand Up @@ -520,15 +509,29 @@ describe('ReportUtils', () => {
});
});

it('return both iou send and request money in DM', () => {
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.TYPE.REQUEST)).toBe(true);
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.SEND)).toBe(true);
describe('return multiple 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,
isOwnPolicyExpenseChat: true,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, [currentUserAccountID, ...participantsAccountIDs], [CONST.BETAS.IOU_SEND]);
expect(moneyRequestOptions.length).toBe(2);
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.REQUEST)).toBe(true);
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.SPLIT)).toBe(true);
});

it('it is a 1:1 DM', () => {
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.TYPE.REQUEST)).toBe(true);
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.SEND)).toBe(true);
});
});
});

Expand Down

0 comments on commit fae9178

Please sign in to comment.