From ba7bd1714b873326c7bda6a75e29c33473100e40 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Thu, 19 Oct 2023 15:45:01 -0400 Subject: [PATCH 1/7] Allow requesting money when expensify owns workspace --- src/libs/ReportUtils.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 1b03c3a1bdb2..3e0b8962bf37 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3515,15 +3515,18 @@ function getMoneyRequestOptions(report, reportParticipants) { 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) { + // 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 policyID = lodashGet(report, 'policyID', ''); + const policyOwnerAccountID = lodashGet(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${policyID}.ownerAccountID`, 0); + const doExpensifyAccountsOwnPolicy = CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); + if (doParticipantsIncludeExpensifyAccounts && !doExpensifyAccountsOwnPolicy) { return []; } + const hasSingleParticipantInReport = participants.length === 1; + const hasMultipleParticipants = participants.length > 1; + // 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. From bd92d7e6503bdf413bedfc8e216d74a814e92b68 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Fri, 20 Oct 2023 12:12:55 -0400 Subject: [PATCH 2/7] Allow assigning task in workspace chat where expensify is owner --- .../report/ReportActionCompose/AttachmentPickerWithMenuItems.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js index 6522bedc825a..d3be3f47bf2f 100644 --- a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js +++ b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js @@ -152,7 +152,7 @@ function AttachmentPickerWithMenuItems({ */ const taskOption = useMemo(() => { // We only prevent the task option from showing if it's a DM and the other user is an Expensify default email - if (!Permissions.canUseTasks(betas) || ReportUtils.isExpensifyOnlyParticipantInReport(report)) { + if (!Permissions.canUseTasks(betas) || (!ReportUtils.isPolicyExpenseChat(report) && ReportUtils.isExpensifyOnlyParticipantInReport(report))) { return []; } From 481c5df19e6e36a7d7996bc65bd826d3260ed9d1 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Mon, 30 Oct 2023 17:40:32 -0400 Subject: [PATCH 3/7] abstract canCreateTaskInReport logic into function --- src/libs/ReportUtils.js | 34 ++++++++++++++++--- .../AttachmentPickerWithMenuItems.js | 2 +- .../TaskShareDestinationSelectorModal.js | 2 +- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index dbfa70943633..9be5ca65f5dc 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -514,6 +514,33 @@ function isExpensifyOnlyParticipantInReport(report) { return reportParticipants.length === 1 && _.some(reportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID)); } +/** + * Checks if the policy that the report is on is owned by one of the special Expensify accounts + * + * @param {Object} report + * @returns {Boolean} + */ +function doExpensifyAccountsOwnPolicy(report) { + const policyID = lodashGet(report, 'policyID', ''); + const policyOwnerAccountID = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, 'ownerAccountID'], 0); + return CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); +} + +/** + * Returns whether a given report can have tasks created in it. + * @param {Object} report + * @returns {Boolean} + */ +function canCreateTaskInReport(report) { + // Tasks cannot be created in DMs with special Expensify accounts but they can be created in policy rooms that are owned by them. + // So we check if Expensify accounts are the only other participant and also whether or not we are in a room owned by them. + if (isExpensifyOnlyParticipantInReport(report) && !doExpensifyAccountsOwnPolicy(report)) { + return false; + } + + return true; +} + /** * Returns true if there are any Expensify accounts (i.e. with domain 'expensify.com') in the set of accountIDs * by cross-referencing the accountIDs with personalDetails. @@ -3652,10 +3679,7 @@ function getMoneyRequestOptions(report, reportParticipants) { // 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 policyID = lodashGet(report, 'policyID', ''); - const policyOwnerAccountID = lodashGet(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${policyID}.ownerAccountID`, 0); - const doExpensifyAccountsOwnPolicy = CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); - if (doParticipantsIncludeExpensifyAccounts && !doExpensifyAccountsOwnPolicy) { + if (doParticipantsIncludeExpensifyAccounts && !doExpensifyAccountsOwnPolicy(report)) { return []; } @@ -4150,6 +4174,8 @@ export { getPolicyType, isArchivedRoom, isExpensifyOnlyParticipantInReport, + doExpensifyAccountsOwnPolicy, + canCreateTaskInReport, isPolicyExpenseChatAdmin, isPolicyAdmin, isPublicRoom, diff --git a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js index f29b727f0008..33316e657f02 100644 --- a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js +++ b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js @@ -152,7 +152,7 @@ function AttachmentPickerWithMenuItems({ */ const taskOption = useMemo(() => { // We only prevent the task option from showing if it's a DM and the other user is an Expensify default email - if (!Permissions.canUseTasks(betas) || (!ReportUtils.isPolicyExpenseChat(report) && ReportUtils.isExpensifyOnlyParticipantInReport(report))) { + if (!Permissions.canUseTasks(betas) || (!ReportUtils.canCreateTaskInReport(report))) { return []; } diff --git a/src/pages/tasks/TaskShareDestinationSelectorModal.js b/src/pages/tasks/TaskShareDestinationSelectorModal.js index fde02c2a4108..0765f435e3ca 100644 --- a/src/pages/tasks/TaskShareDestinationSelectorModal.js +++ b/src/pages/tasks/TaskShareDestinationSelectorModal.js @@ -53,7 +53,7 @@ function TaskShareDestinationSelectorModal(props) { _.keys(props.reports).forEach((reportKey) => { if ( ReportUtils.shouldDisableWriteActions(props.reports[reportKey]) || - ReportUtils.isExpensifyOnlyParticipantInReport(props.reports[reportKey]) || + !ReportUtils.canCreateTaskInReport(props.reports[reportKey]) || ReportUtils.isCanceledTaskReport(props.reports[reportKey]) ) { return; From baea0d26ab2d5fcce3b1caa43ca4c12878c8b665 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Mon, 30 Oct 2023 17:45:10 -0400 Subject: [PATCH 4/7] prettier --- .../report/ReportActionCompose/AttachmentPickerWithMenuItems.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js index 33316e657f02..e2266de95880 100644 --- a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js +++ b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js @@ -152,7 +152,7 @@ function AttachmentPickerWithMenuItems({ */ const taskOption = useMemo(() => { // We only prevent the task option from showing if it's a DM and the other user is an Expensify default email - if (!Permissions.canUseTasks(betas) || (!ReportUtils.canCreateTaskInReport(report))) { + if (!Permissions.canUseTasks(betas) || !ReportUtils.canCreateTaskInReport(report)) { return []; } From 8bc920faa72cb7cc0ceb8bb974331fdaee97dc37 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 14:07:44 -0400 Subject: [PATCH 5/7] Use isDM check instead of policy owner --- src/libs/ReportUtils.js | 57 +++++++------------ .../AttachmentPickerWithMenuItems.js | 1 - 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 65fe0b0846c2..27200ff4347c 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -475,6 +475,16 @@ function isChatThread(report) { return isThread(report) && report.type === CONST.REPORT.TYPE.CHAT; } +/** + * Returns true if report is a DM/Group DM chat. + * + * @param {Object} report + * @returns {Boolean} + */ +function isDM(report) { + return !getChatType(report); +} + /** * Only returns true if this is our main 1:1 DM report with Concierge * @@ -504,37 +514,17 @@ function shouldDisableDetailPage(report) { return false; } -/** - * Returns true if this report has only one participant and it's an Expensify account. - * @param {Object} report - * @returns {Boolean} - */ -function isExpensifyOnlyParticipantInReport(report) { - const reportParticipants = _.without(lodashGet(report, 'participantAccountIDs', []), currentUserAccountID); - return reportParticipants.length === 1 && _.some(reportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID)); -} - -/** - * Checks if the policy that the report is on is owned by one of the special Expensify accounts - * - * @param {Object} report - * @returns {Boolean} - */ -function doExpensifyAccountsOwnPolicy(report) { - const policyID = lodashGet(report, 'policyID', ''); - const policyOwnerAccountID = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, 'ownerAccountID'], 0); - return CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); -} - /** * Returns whether a given report can have tasks created in it. + * We only prevent the task option if it's a DM/group-DM and the other users are all special Expensify accounts + * * @param {Object} report * @returns {Boolean} */ function canCreateTaskInReport(report) { - // Tasks cannot be created in DMs with special Expensify accounts but they can be created in policy rooms that are owned by them. - // So we check if Expensify accounts are the only other participant and also whether or not we are in a room owned by them. - if (isExpensifyOnlyParticipantInReport(report) && !doExpensifyAccountsOwnPolicy(report)) { + const otherReportParticipants = _.without(lodashGet(report, 'participantAccountIDs', []), currentUserAccountID); + const areExpensifyAccountsOnlyOtherParticipants = _.every(otherReportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID)); + if (areExpensifyAccountsOnlyOtherParticipants && isDM(report)) { return false; } @@ -673,16 +663,6 @@ function isPolicyAdmin(policyID, policies) { return policyRole === CONST.POLICY.ROLE.ADMIN; } -/** - * Returns true if report is a DM/Group DM chat. - * - * @param {Object} report - * @returns {Boolean} - */ -function isDM(report) { - return !getChatType(report); -} - /** * Returns true if report has a single participant. * @@ -3679,7 +3659,10 @@ function getMoneyRequestOptions(report, reportParticipants) { // 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; - if (doParticipantsIncludeExpensifyAccounts && !doExpensifyAccountsOwnPolicy(report)) { + const policyID = lodashGet(report, 'policyID', ''); + const policyOwnerAccountID = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, 'ownerAccountID'], 0); + const isPolicyOwnedByExpensifyAccounts = CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); + if (doParticipantsIncludeExpensifyAccounts && !isPolicyOwnedByExpensifyAccounts) { return []; } @@ -4173,8 +4156,6 @@ export { getPolicyName, getPolicyType, isArchivedRoom, - isExpensifyOnlyParticipantInReport, - doExpensifyAccountsOwnPolicy, canCreateTaskInReport, isPolicyExpenseChatAdmin, isPolicyAdmin, diff --git a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js index e2266de95880..a31e718933ea 100644 --- a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js +++ b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.js @@ -151,7 +151,6 @@ function AttachmentPickerWithMenuItems({ * @returns {Boolean} */ const taskOption = useMemo(() => { - // We only prevent the task option from showing if it's a DM and the other user is an Expensify default email if (!Permissions.canUseTasks(betas) || !ReportUtils.canCreateTaskInReport(report)) { return []; } From f35acb9200c7212770b56ebb5bb7a35850eec2ff Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 14:22:43 -0400 Subject: [PATCH 6/7] Add isExpensifyOnlyParticipantInReport function back since its used by tests --- src/libs/ReportUtils.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 27200ff4347c..65b7a50d6ed8 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -514,6 +514,16 @@ function shouldDisableDetailPage(report) { return false; } +/** + * Returns true if this report has only one participant and it's an Expensify account. + * @param {Object} report + * @returns {Boolean} + */ +function isExpensifyOnlyParticipantInReport(report) { + const reportParticipants = _.without(lodashGet(report, 'participantAccountIDs', []), currentUserAccountID); + return reportParticipants.length === 1 && _.some(reportParticipants, (accountID) => _.contains(CONST.EXPENSIFY_ACCOUNT_IDS, accountID)); +} + /** * Returns whether a given report can have tasks created in it. * We only prevent the task option if it's a DM/group-DM and the other users are all special Expensify accounts @@ -4156,6 +4166,7 @@ export { getPolicyName, getPolicyType, isArchivedRoom, + isExpensifyOnlyParticipantInReport, canCreateTaskInReport, isPolicyExpenseChatAdmin, isPolicyAdmin, From 70a10f7ee915cca812775a173a00d6782d26d839 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 14:53:21 -0400 Subject: [PATCH 7/7] Reduce isPolicyOwnedByExpensifyAccounts to one line --- src/libs/ReportUtils.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 65b7a50d6ed8..6feab9796d10 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -3669,9 +3669,7 @@ function getMoneyRequestOptions(report, reportParticipants) { // 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 policyID = lodashGet(report, 'policyID', ''); - const policyOwnerAccountID = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, 'ownerAccountID'], 0); - const isPolicyOwnedByExpensifyAccounts = CONST.EXPENSIFY_ACCOUNT_IDS.includes(policyOwnerAccountID); + const isPolicyOwnedByExpensifyAccounts = report.policyID ? CONST.EXPENSIFY_ACCOUNT_IDS.includes(getPolicy(report.policyID).ownerAccountID || 0) : false; if (doParticipantsIncludeExpensifyAccounts && !isPolicyOwnedByExpensifyAccounts) { return []; }