From ab16481f986959ee92791dc4db2ef2531dec8497 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 7 Nov 2023 14:59:16 -0500 Subject: [PATCH 1/6] Update money request options for different room types --- src/libs/ReportUtils.js | 43 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 22118f992591..a52037419c7e 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3624,8 +3624,8 @@ function hasIOUWaitingOnCurrentUserBankAccount(chatReport) { * @returns {Boolean} */ function canRequestMoney(report, participants) { - // User cannot request money in chat thread or in task report - if (isChatThread(report) || isTaskReport(report)) { + // User cannot request money in chat thread or in task report or in chat room + if (isChatThread(report) || isTaskReport(report) || isChatRoom(report)) { return false; } @@ -3683,8 +3683,6 @@ 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; @@ -3692,30 +3690,33 @@ function getMoneyRequestOptions(report, reportParticipants) { 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 + // 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) && participants.length > 0) || - (hasMultipleParticipants && !isPolicyExpenseChat(report) && !isMoneyRequestReport(report)) || - (isControlPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat) + (isChatRoom(report) && otherParticipants.length > 0) || + (isDM(report) && hasMultipleOtherParticipants) || + (isPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat) ) { - return [CONST.IOU.TYPE.SPLIT]; + 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; } /** From 16da9a661ac2cd936524d82796f49bd018254096 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 7 Nov 2023 15:00:34 -0500 Subject: [PATCH 2/6] Rename participants to otherParticipants --- src/libs/ReportUtils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index a52037419c7e..aa5b25a8ad7d 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3620,10 +3620,10 @@ function hasIOUWaitingOnCurrentUserBankAccount(chatReport) { * - in DM chat * * @param {Object} report - * @param {Array} participants + * @param {Array} otherParticipants * @returns {Boolean} */ -function canRequestMoney(report, participants) { +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; @@ -3641,7 +3641,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; } From fce7b700d07b40e6a5e8a4706d69054ce2da1220 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 7 Nov 2023 15:12:00 -0500 Subject: [PATCH 3/6] prettier --- src/libs/ReportUtils.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index aa5b25a8ad7d..2224879b6026 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3699,11 +3699,7 @@ function getMoneyRequestOptions(report, reportParticipants) { // 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) - ) { + if ((isChatRoom(report) && otherParticipants.length > 0) || (isDM(report) && hasMultipleOtherParticipants) || (isPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat)) { options = [CONST.IOU.TYPE.SPLIT]; } From e8835296b2dec47eaffdb634b77245c36a7b0d47 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 7 Nov 2023 15:39:29 -0500 Subject: [PATCH 4/6] Fix isDM function to only be true for type chat --- src/libs/ReportUtils.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 2224879b6026..16ce16a77af2 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -482,7 +482,7 @@ function isChatThread(report) { * @returns {Boolean} */ function isDM(report) { - return !getChatType(report); + return isChatReport(report) && !getChatType(report); } /** @@ -3617,7 +3617,7 @@ 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} otherParticipants @@ -3629,6 +3629,11 @@ function canRequestMoney(report, otherParticipants) { 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; From f481ca9d1b07ed91067e537c6934d9ba6ee04191 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 7 Nov 2023 15:39:50 -0500 Subject: [PATCH 5/6] Update tests for updated scenarios --- tests/unit/ReportUtilsTest.js | 45 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index a29b2727c847..e0c98b1793f1 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -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, @@ -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', @@ -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); + }); }); }); From 2300d069ee0e5c9a4311db5e1a4cce7e30bddd91 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Wed, 8 Nov 2023 13:05:43 -0500 Subject: [PATCH 6/6] Add blank line before comment --- src/libs/ReportUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 16ce16a77af2..ba95961e983b 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3698,8 +3698,8 @@ function getMoneyRequestOptions(report, reportParticipants) { 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 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.