From 672c4084d3c43a5523f991ff0da37f5489c58fcc Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 6 Mar 2023 10:20:20 -0800 Subject: [PATCH 01/12] disable renaming public rooms if the current user is not a member of the workspace --- src/CONST.js | 2 ++ src/libs/ReportUtils.js | 11 +++++++++++ src/pages/ReportSettingsPage.js | 5 ++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/CONST.js b/src/CONST.js index 203ab6a09255..3b1be1042e60 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -354,6 +354,8 @@ const CONST = { VISIBILITY: { RESTRICTED: 'restricted', PRIVATE: 'private', + PUBLIC: 'public', + PUBLIC_ANNOUNCE: 'public_announce', }, RESERVED_ROOM_NAMES: ['#admins', '#announce'], MAX_PREVIEW_AVATARS: 4, diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 83a84f7be041..826ee261a003 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -220,6 +220,17 @@ function isChatRoom(report) { return isUserCreatedPolicyRoom(report) || isDefaultRoom(report); } +/** + * Whether the provided report is a public room + * @param {Object} report + * @param {String} report.visibility + * @returns {Boolean} + */ +function isPublicRoom(report) { + const visibility = lodashGet(report, 'visibility', ''); + return visibility === CONST.REPORT.VISIBILITY.PUBLIC || visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE, +} + /** * Get the policy type from a given report * @param {Object} report diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index fca21166d0f6..4a4b168bc484 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -110,9 +110,12 @@ class ReportSettingsPage extends Component { render() { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); - const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); + const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) + || ReportUtils.isArchivedRoom(this.props.report) + || (ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace); + return ( From f4efc774aabb7fb5b7c8e5b1371477eeb33076f8 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 6 Mar 2023 10:30:22 -0800 Subject: [PATCH 02/12] disable renaming public rooms if the current user is not a member of the workspace --- src/libs/ReportUtils.js | 16 +++++++++++++++- src/libs/actions/Policy.js | 12 ++++++++++++ src/pages/ReportSettingsPage.js | 8 +++++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 826ee261a003..ccb600acee0d 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -228,7 +228,20 @@ function isChatRoom(report) { */ function isPublicRoom(report) { const visibility = lodashGet(report, 'visibility', ''); - return visibility === CONST.REPORT.VISIBILITY.PUBLIC || visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE, + return visibility === CONST.REPORT.VISIBILITY.PUBLIC || visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE; +} + +/** + * Whether the current user is able to edit the name of a public room + * @param {Object} report + * @param {Object} policy + */ +function canEditPublicRoomName(report, policy) { + let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; + if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { + shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) || linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; + } + return !shouldDisablePublicRoomRename; } /** @@ -1614,6 +1627,7 @@ export { getPolicyName, getPolicyType, isArchivedRoom, + isPublicRoom, isConciergeChatReport, hasAutomatedExpensifyEmails, hasExpensifyGuidesEmails, diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index c269accf6ae1..631c95b7db59 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -14,6 +14,7 @@ import * as OptionsListUtils from '../OptionsListUtils'; import DateUtils from '../DateUtils'; import * as ReportUtils from '../ReportUtils'; import Log from '../Log'; +import { session } from 'electron'; const allPolicies = {}; Onyx.connect({ @@ -150,6 +151,16 @@ function isAdminOfFreePolicy(policies) { && policy.role === CONST.POLICY.ROLE.ADMIN); } +/** + * Is the user the owner of the given policy? + * + * @param {Object} policy + * @returns {Boolean} + */ +function isPolicyOwner(policy) { + return policy.owner === sessionEmail; +} + /** * Check if the user has any active free policies (aka workspaces) * @@ -1027,4 +1038,5 @@ export { openWorkspaceMembersPage, openWorkspaceInvitePage, removeWorkspace, + isPolicyOwner, }; diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index 4a4b168bc484..efbb486971f1 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -10,6 +10,7 @@ import styles from '../styles/styles'; import compose from '../libs/compose'; import Navigation from '../libs/Navigation/Navigation'; import * as Report from '../libs/actions/Report'; +import * as Policy from '../libs/actions/Policy'; import * as ReportUtils from '../libs/ReportUtils'; import HeaderWithCloseButton from '../components/HeaderWithCloseButton'; import ScreenWrapper from '../components/ScreenWrapper'; @@ -112,9 +113,14 @@ class ReportSettingsPage extends Component { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); + let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; + if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { + shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) && linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; + } + const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) - || (ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace); + || shouldDisablePublicRoomRename; return ( From 1bacb54cea121f13f2367345a5c1c8ba6cddd755 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 6 Mar 2023 10:32:49 -0800 Subject: [PATCH 03/12] remove unused --- src/libs/ReportUtils.js | 13 ------------- src/pages/ReportSettingsPage.js | 5 ----- 2 files changed, 18 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index ccb600acee0d..3089fda9ba52 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -231,19 +231,6 @@ function isPublicRoom(report) { return visibility === CONST.REPORT.VISIBILITY.PUBLIC || visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE; } -/** - * Whether the current user is able to edit the name of a public room - * @param {Object} report - * @param {Object} policy - */ -function canEditPublicRoomName(report, policy) { - let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; - if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { - shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) || linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; - } - return !shouldDisablePublicRoomRename; -} - /** * Get the policy type from a given report * @param {Object} report diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index efbb486971f1..fe1d4283beb8 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -113,11 +113,6 @@ class ReportSettingsPage extends Component { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); - let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; - if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { - shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) && linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; - } - const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) || shouldDisablePublicRoomRename; From 4c0d1681a4f4b2265da05380a2d8f8f9d92abc40 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 6 Mar 2023 10:33:41 -0800 Subject: [PATCH 04/12] remove unused --- src/pages/ReportSettingsPage.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index fe1d4283beb8..efbb486971f1 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -113,6 +113,11 @@ class ReportSettingsPage extends Component { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); + let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; + if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { + shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) && linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; + } + const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) || shouldDisablePublicRoomRename; From bac9f026de585f08bb84c570220a28d55dc45d60 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 6 Mar 2023 11:04:17 -0800 Subject: [PATCH 05/12] lint --- src/libs/actions/Policy.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 631c95b7db59..84bcda7c7d98 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -14,7 +14,6 @@ import * as OptionsListUtils from '../OptionsListUtils'; import DateUtils from '../DateUtils'; import * as ReportUtils from '../ReportUtils'; import Log from '../Log'; -import { session } from 'electron'; const allPolicies = {}; Onyx.connect({ From 681a49f15ac9d1fa3b1405978a60b76f789cb9e2 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 7 Mar 2023 11:42:14 -0800 Subject: [PATCH 06/12] remove unused, style --- src/libs/actions/Report.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 3b503734ec4c..237dc58e511f 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -990,7 +990,9 @@ function addPolicyReport(policy, reportName, visibility) { onyxMethod: CONST.ONYX.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT}${policyReport.reportID}`, value: { - + pendingFields: { + addWorkspaceRoom: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }, ...policyReport, }, }, From 9b1bed7ebc34a049efd8c86a76d372d8374fe0f8 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 7 Mar 2023 11:42:36 -0800 Subject: [PATCH 07/12] remove unused --- src/libs/actions/Report.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 237dc58e511f..8b25233342d4 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -346,7 +346,6 @@ function openReport(reportID, participantList = [], newReportObject = {}) { isLoadingReportActions: false, pendingFields: { createChat: null, - openReport: null, }, errorFields: { createChat: null, From ac3744aa21246722f972d7b20a1ecaa4b66e770d Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Wed, 8 Mar 2023 20:18:55 -0800 Subject: [PATCH 08/12] use early returns --- src/pages/ReportSettingsPage.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index efbb486971f1..37e5b579edc0 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -66,6 +66,21 @@ class ReportSettingsPage extends Component { ]; } + /** + * @param {Object} report - the given report we're viewing settings for + * @param {Object|null} policy - null if the user isn't in the workspace the report is on + * @returns {Boolean} + */ + getShouldDisablePublicRoomRename(report, policy) { + if (!ReportUtils.isPublicRoom(report)) { + return false; + } + if (!policy) { + return true; + } + return !Policy.isPolicyOwner(policy) && policy.role !== CONST.POLICY.ROLE.ADMIN; + } + /** * @param {Object} values - form input values passed by the Form component */ @@ -112,15 +127,9 @@ class ReportSettingsPage extends Component { render() { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); - - let shouldDisablePublicRoomRename = ReportUtils.isPublicRoom(this.props.report) && !linkedWorkspace; - if (ReportUtils.isPublicRoom(this.props.report) && linkedWorkspace) { - shouldDisablePublicRoomRename = !Policy.isPolicyOwner(linkedWorkspace) && linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; - } - const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) - || shouldDisablePublicRoomRename; + || this.getShouldDisablePublicRoomRename(this.props.report, linkedWorkspace); return ( From efd3e2b771ab04173153dd561113ba4724c0950f Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Wed, 8 Mar 2023 20:19:31 -0800 Subject: [PATCH 09/12] clarify comment --- src/pages/ReportSettingsPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index 37e5b579edc0..50ff7d69ef9b 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -68,7 +68,7 @@ class ReportSettingsPage extends Component { /** * @param {Object} report - the given report we're viewing settings for - * @param {Object|null} policy - null if the user isn't in the workspace the report is on + * @param {Object|null} policy - the workspace the report is on, null if the user isn't a member of the workspace * @returns {Boolean} */ getShouldDisablePublicRoomRename(report, policy) { From b657fa096ea2bafb4f198d2ea74ddc09b96d7034 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Fri, 10 Mar 2023 15:17:43 +0000 Subject: [PATCH 10/12] review comments --- src/pages/ReportSettingsPage.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index 50ff7d69ef9b..1b6dfd071577 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -71,13 +71,11 @@ class ReportSettingsPage extends Component { * @param {Object|null} policy - the workspace the report is on, null if the user isn't a member of the workspace * @returns {Boolean} */ - getShouldDisablePublicRoomRename(report, policy) { - if (!ReportUtils.isPublicRoom(report)) { + shouldDisablePublicRoomRename(report, policy) { + if (!policy || !ReportUtils.isPublicRoom(report)) { return false; } - if (!policy) { - return true; - } + return !Policy.isPolicyOwner(policy) && policy.role !== CONST.POLICY.ROLE.ADMIN; } @@ -129,7 +127,7 @@ class ReportSettingsPage extends Component { const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report) - || this.getShouldDisablePublicRoomRename(this.props.report, linkedWorkspace); + || this.shouldDisablePublicRoomRename(this.props.report, linkedWorkspace); return ( From dffb136e9c9f5062f6748c2077083a7e6a66585d Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Fri, 10 Mar 2023 15:23:44 +0000 Subject: [PATCH 11/12] Update ReportSettingsPage.js --- src/pages/ReportSettingsPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index 1b6dfd071577..90034062715d 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -73,7 +73,7 @@ class ReportSettingsPage extends Component { */ shouldDisablePublicRoomRename(report, policy) { if (!policy || !ReportUtils.isPublicRoom(report)) { - return false; + return true; } return !Policy.isPolicyOwner(policy) && policy.role !== CONST.POLICY.ROLE.ADMIN; From 24335d4a381559c9fc2513a47c818c1c0c6c3a4b Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Sat, 11 Mar 2023 00:30:01 -0800 Subject: [PATCH 12/12] fix condition, move all condition checks for disabling renaming into single function --- src/pages/ReportSettingsPage.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/pages/ReportSettingsPage.js b/src/pages/ReportSettingsPage.js index 90034062715d..73469a441c1e 100644 --- a/src/pages/ReportSettingsPage.js +++ b/src/pages/ReportSettingsPage.js @@ -67,16 +67,28 @@ class ReportSettingsPage extends Component { } /** - * @param {Object} report - the given report we're viewing settings for - * @param {Object|null} policy - the workspace the report is on, null if the user isn't a member of the workspace + * @param {Object|null} linkedWorkspace - the workspace the report is on, null if the user isn't a member of the workspace * @returns {Boolean} */ - shouldDisablePublicRoomRename(report, policy) { - if (!policy || !ReportUtils.isPublicRoom(report)) { + shouldDisableRename(linkedWorkspace) { + if (ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report)) { return true; } - return !Policy.isPolicyOwner(policy) && policy.role !== CONST.POLICY.ROLE.ADMIN; + // The remaining checks only apply to public rooms + if (!ReportUtils.isPublicRoom(this.props.report)) { + return false; + } + + // if the linked workspace is null, that means the person isn't a member of the workspace the report is in + // which means this has to be a public room we want to disable renaming for + if (!linkedWorkspace) { + return true; + } + + // If there is a linked workspace, that means the user is a member of the workspace the report is in. + // Still, we only want policy owners and admins to be able to modify the name. + return !Policy.isPolicyOwner(linkedWorkspace) && linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN; } /** @@ -125,9 +137,7 @@ class ReportSettingsPage extends Component { render() { const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(this.props.report); const linkedWorkspace = _.find(this.props.policies, policy => policy && policy.id === this.props.report.policyID); - const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) - || ReportUtils.isArchivedRoom(this.props.report) - || this.shouldDisablePublicRoomRename(this.props.report, linkedWorkspace); + const shouldDisableRename = this.shouldDisableRename(linkedWorkspace); return (