From 07f8faca338643a312ce4a24a9a443f02a58e3b3 Mon Sep 17 00:00:00 2001 From: ShridharGoel <35566748+ShridharGoel@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:05:29 +0530 Subject: [PATCH 1/2] Fix guest leaving and joining workspace room --- src/libs/ReportUtils.ts | 9 +++++---- .../ReportDetailsMenuLeaveItem.tsx | 5 +++-- tests/unit/ReportUtilsTest.ts | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 45400a160ceb..cd0275525c48 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -12991,8 +12991,9 @@ function canJoinChat( return false; } - // For restricted visibility rooms, the user must be a workspace member to join - if (isUserCreatedPolicyRoom(report) && report?.visibility === CONST.REPORT.VISIBILITY.RESTRICTED && !getPolicyRole(policy, currentUserEmail ?? deprecatedCurrentUserEmail)) { + // For restricted visibility rooms, the user must be a non-guest workspace member to join + const policyRole = getPolicyRole(policy, currentUserEmail ?? deprecatedCurrentUserEmail); + if (isUserCreatedPolicyRoom(report) && report?.visibility === CONST.REPORT.VISIBILITY.RESTRICTED && (!policyRole || policyRole === CONST.POLICY.ROLE.GUEST)) { return false; } @@ -14972,13 +14973,13 @@ function getStatusBadgeBackgroundColor(theme: ThemeColors, stateNum?: number, st * Checks if a workspace member is leaving a workspace room * This is used to determine if we need to show special handling when a workspace member leaves a room */ -function isWorkspaceMemberLeavingWorkspaceRoom(report: OnyxEntry, isPolicyEmployee: boolean, isPolicyAdminParam: boolean): boolean { +function isWorkspaceMemberLeavingWorkspaceRoom(report: OnyxEntry, isPolicyEmployee: boolean, isPolicyAdminParam: boolean, isPolicyGuestParam = false): boolean { if (!report) { return false; } // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const hasAccessPolicyExpenseChat = isPolicyExpenseChat(report) && (report.isOwnPolicyExpenseChat || isPolicyAdminParam); - return (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || hasAccessPolicyExpenseChat) && isPolicyEmployee; + return (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || hasAccessPolicyExpenseChat) && isPolicyEmployee && !isPolicyGuestParam; } /** diff --git a/src/pages/DynamicReportDetailsPage/ReportDetailsMenuLeaveItem.tsx b/src/pages/DynamicReportDetailsPage/ReportDetailsMenuLeaveItem.tsx index b96aced4aae5..beee59581027 100644 --- a/src/pages/DynamicReportDetailsPage/ReportDetailsMenuLeaveItem.tsx +++ b/src/pages/DynamicReportDetailsPage/ReportDetailsMenuLeaveItem.tsx @@ -10,7 +10,7 @@ import useOnyx from '@hooks/useOnyx'; import {useAllPersonalDetails} from '@hooks/usePersonalDetails'; import useReportIsArchived from '@hooks/useReportIsArchived'; -import {isPolicyAdmin as isPolicyAdminUtil, isPolicyEmployee as isPolicyEmployeeUtil} from '@libs/PolicyUtils'; +import {isPolicyAdmin as isPolicyAdminUtil, isPolicyEmployee as isPolicyEmployeeUtil, isPolicyGuest as isPolicyGuestUtil} from '@libs/PolicyUtils'; import { canLeaveChat, findLastAccessedReport, @@ -59,6 +59,7 @@ function ReportDetailsMenuLeaveItem({reportID}: ReportDetailsMenuLeaveItemProps) const isPolicyAdmin = isPolicyAdminUtil(policy); const isPolicyEmployee = isPolicyEmployeeUtil(report.policyID, policy); + const isPolicyGuest = isPolicyGuestUtil(policy); const isRootGroupChat = isRootGroupChatUtil(report, isReportArchived); const leaveChat = () => { @@ -78,7 +79,7 @@ function ReportDetailsMenuLeaveItem({reportID}: ReportDetailsMenuLeaveItemProps) return; } - const isWorkspaceMemberLeavingWorkspaceRoom = isWorkspaceMemberLeavingWorkspaceRoomUtil(report, isPolicyEmployee, isPolicyAdmin); + const isWorkspaceMemberLeavingWorkspaceRoom = isWorkspaceMemberLeavingWorkspaceRoomUtil(report, isPolicyEmployee, isPolicyAdmin, isPolicyGuest); leaveRoom(report, currentUserAccountID, conciergeReportID, introSelected, isSelfTourViewed, personalDetails, isWorkspaceMemberLeavingWorkspaceRoom, lastAccessedReportID); }; diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index e3f610e18328..28186424af55 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -15509,6 +15509,13 @@ describe('ReportUtils', () => { employeeList: {}, }; + const policyWithGuestUser: Policy = { + ...createRandomPolicy(3), + id: policyID, + role: CONST.POLICY.ROLE.GUEST, + employeeList: {}, + }; + const restrictedReport: Report = { ...createRandomReport(1, CONST.REPORT.CHAT_TYPE.POLICY_ROOM), type: CONST.REPORT.TYPE.CHAT, @@ -15530,7 +15537,9 @@ describe('ReportUtils', () => { expect(canJoinChat(restrictedReport, undefined, policyWithoutCurrentUser, undefined)).toBe(false); expect(canJoinChat(restrictedReport, undefined, policyWithCurrentUser, undefined)).toBe(true); + expect(canJoinChat(restrictedReport, undefined, policyWithGuestUser, undefined)).toBe(false); expect(canJoinChat(publicReport, undefined, policyWithoutCurrentUser, undefined)).toBe(true); + expect(canJoinChat(publicReport, undefined, policyWithGuestUser, undefined)).toBe(true); }); }); @@ -17317,6 +17326,14 @@ describe('ReportUtils', () => { }; expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false)).toBe(false); }); + + test('should return false for restricted room when user is a policy guest', () => { + const report = { + ...createRandomReport(7, undefined), + visibility: CONST.REPORT.VISIBILITY.RESTRICTED, + }; + expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false, true)).toBe(false); + }); }); describe('shouldExcludeAncestorReportAction', () => { From 70c40115c4ad7f2f495b14d45fcb04f8e6ca5438 Mon Sep 17 00:00:00 2001 From: ShridharGoel <35566748+ShridharGoel@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:55:15 +0530 Subject: [PATCH 2/2] Add Given-When-Then structure to ReportUtils tests --- tests/unit/ReportUtilsTest.ts | 62 +++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 28186424af55..98625f1f63cb 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -15493,6 +15493,7 @@ describe('ReportUtils', () => { }); it('should respect workspace membership for restricted visibility rooms', async () => { + // Given a workspace and reports with restricted and public visibility const policyID = '123456'; await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID}); @@ -15535,9 +15536,14 @@ describe('ReportUtils', () => { visibility: CONST.REPORT.VISIBILITY.PUBLIC, }; + // When checking whether a user can join restricted rooms + // Then regular workspace members can join, but non-members and guests cannot expect(canJoinChat(restrictedReport, undefined, policyWithoutCurrentUser, undefined)).toBe(false); expect(canJoinChat(restrictedReport, undefined, policyWithCurrentUser, undefined)).toBe(true); expect(canJoinChat(restrictedReport, undefined, policyWithGuestUser, undefined)).toBe(false); + + // When checking whether a user can join public rooms + // Then non-members and guests can both join expect(canJoinChat(publicReport, undefined, policyWithoutCurrentUser, undefined)).toBe(true); expect(canJoinChat(publicReport, undefined, policyWithGuestUser, undefined)).toBe(true); }); @@ -17275,64 +17281,106 @@ describe('ReportUtils', () => { describe('isWorkspaceMemberLeavingWorkspaceRoom', () => { test('should return false when not a policy employee', () => { + // Given a restricted policy expense chat and a user who is not a policy employee const report = { ...createRandomReport(1, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), visibility: CONST.REPORT.VISIBILITY.RESTRICTED, isOwnPolicyExpenseChat: true, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, false, true)).toBe(false); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, false, true); + + // Then it should return false + expect(result).toBe(false); }); test('should return true for restricted room when policy employee', () => { + // Given a restricted room and a user who is a policy employee const report = { ...createRandomReport(2, undefined), visibility: CONST.REPORT.VISIBILITY.RESTRICTED, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false)).toBe(true); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, false); + + // Then it should return true + expect(result).toBe(true); }); test('should return true for policy expense chat when own chat and policy employee', () => { + // Given a public policy expense chat owned by the user who is a policy employee const report = { ...createRandomReport(3, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), visibility: CONST.REPORT.VISIBILITY.PUBLIC, isOwnPolicyExpenseChat: true, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false)).toBe(true); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, false); + + // Then it should return true + expect(result).toBe(true); }); test('should return true for policy expense chat when policy admin and policy employee', () => { + // Given a public policy expense chat not owned by the user, but the user is a policy admin and employee const report = { ...createRandomReport(4, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), visibility: CONST.REPORT.VISIBILITY.PUBLIC, isOwnPolicyExpenseChat: false, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, true)).toBe(true); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, true); + + // Then it should return true + expect(result).toBe(true); }); test('should return false for non-restricted, non-policy expense chat', () => { + // Given a public group chat const report = { ...createRandomReport(5, CONST.REPORT.CHAT_TYPE.GROUP), visibility: CONST.REPORT.VISIBILITY.PUBLIC, isOwnPolicyExpenseChat: false, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false)).toBe(false); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, false); + + // Then it should return false + expect(result).toBe(false); }); test('should return false for non-restricted policy expense chat when not own chat and not admin', () => { + // Given a public policy expense chat not owned by the user and the user is not an admin const report = { ...createRandomReport(6, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), visibility: CONST.REPORT.VISIBILITY.PUBLIC, isOwnPolicyExpenseChat: false, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false)).toBe(false); + + // When checking if the member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, false); + + // Then it should return false + expect(result).toBe(false); }); test('should return false for restricted room when user is a policy guest', () => { + // Given a restricted room and a user who is a policy guest const report = { ...createRandomReport(7, undefined), visibility: CONST.REPORT.VISIBILITY.RESTRICTED, }; - expect(isWorkspaceMemberLeavingWorkspaceRoom(report, true, false, true)).toBe(false); + + // When checking if the guest member is leaving the workspace room + const result = isWorkspaceMemberLeavingWorkspaceRoom(report, true, false, true); + + // Then it should return false + expect(result).toBe(false); }); });