From a306b5f7338dbf8bd1d94682be07aef862534359 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 10:22:53 +0200 Subject: [PATCH 1/9] prevent changing workspace if report is not open/submitted --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 56b242bd3210f..be9f6520f9823 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10422,7 +10422,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report // From this point on, reports must be of type Expense, the policy must be a paid type. // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. const isExpenseReportType = isExpenseReport(report); - if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin)) { + if (!isExpenseReportType || !isReportOpenOrSubmitted || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin)) { return false; } From e3ce74387381f89521d6264d96e6552729875d00 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 10:23:16 +0200 Subject: [PATCH 2/9] update comment --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index be9f6520f9823..a433abd62f6a6 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10419,7 +10419,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report return true; } - // From this point on, reports must be of type Expense, the policy must be a paid type. + // From this point on, reports must be of type Expense, open or submitted, the policy must be a paid type. // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. const isExpenseReportType = isExpenseReport(report); if (!isExpenseReportType || !isReportOpenOrSubmitted || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin)) { From ad8914689877fd84535724eddb9c77ae7a680bc2 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 10:25:28 +0200 Subject: [PATCH 3/9] rm redudant check --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index a433abd62f6a6..3debaf02ed327 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10427,7 +10427,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report } const isCurrentUserReportSubmitter = session.accountID === report?.ownerAccountID; - if (isCurrentUserReportSubmitter && isReportOpenOrSubmitted) { + if (isCurrentUserReportSubmitter) { return true; } From 6630b94712456e2fae31c7f674456ca2dc30cc07 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 17:32:12 +0200 Subject: [PATCH 4/9] update condition --- src/libs/ReportUtils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 3debaf02ed327..8dfe692508c7e 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10419,10 +10419,11 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report return true; } - // From this point on, reports must be of type Expense, open or submitted, the policy must be a paid type. + // From this point on, reports must be of type Expense, the policy must be a paid type. // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. + // Additionally, if the report is not open or submitted, the current user must be an admin. const isExpenseReportType = isExpenseReport(report); - if (!isExpenseReportType || !isReportOpenOrSubmitted || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin)) { + if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin || (!isReportOpenOrSubmitted && !isCurrentUserAdmin))) { return false; } From daf69f404463217783225352f9a312abb5360226 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 17:39:42 +0200 Subject: [PATCH 5/9] update logic and add test --- src/libs/ReportUtils.ts | 2 +- tests/unit/PolicyUtilsTest.ts | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8dfe692508c7e..c3bdaf6ee313c 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10423,7 +10423,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. // Additionally, if the report is not open or submitted, the current user must be an admin. const isExpenseReportType = isExpenseReport(report); - if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin || (!isReportOpenOrSubmitted && !isCurrentUserAdmin))) { + if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin) || (!isReportOpenOrSubmitted && !isCurrentUserAdmin)) { return false; } diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index f0c51d3e0e9c7..8b23ffa68c5a3 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -751,6 +751,31 @@ describe('PolicyUtils', () => { expect(result).toBe(true); }); + it('returns false if current user is not a policy admin and the report is approved', () => { + const currentUserLogin = approverEmail; + const currentUserAccountID = approverAccountID; + const session = {email: currentUserLogin, accountID: currentUserAccountID}; + + const newPolicy = { + ...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM), + role: CONST.POLICY.ROLE.USER, + approver: approverEmail, + employeeList: { + [employeeEmail]: {email: employeeEmail, role: CONST.POLICY.ROLE.USER}, + [currentUserLogin]: {email: currentUserLogin, role: CONST.POLICY.ROLE.USER}, + }, + }; + const report = { + ...createRandomReport(0), + type: CONST.REPORT.TYPE.EXPENSE, + stateNum: CONST.REPORT.STATE_NUM.APPROVED, + ownerAccountID: currentUserAccountID, + }; + + const result = isWorkspaceEligibleForReportChange(newPolicy, report, session); + expect(result).toBe(false); + }); + it('returns true if current user is the approver and submitter is a member', () => { const currentUserLogin = approverEmail; const currentUserAccountID = approverAccountID; From db6a38a040a8d3fa2ea8bc096f3e83e75f927b8b Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 20:20:51 +0200 Subject: [PATCH 6/9] update logic --- src/libs/ReportSecondaryActionUtils.ts | 6 +++--- src/libs/ReportUtils.ts | 5 +++-- src/pages/ReportChangeWorkspacePage.tsx | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 2322efdabeb81..a83d356e95e54 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -313,10 +313,10 @@ function isHoldActionForTransation(report: Report, reportTransaction: Transactio return isProcessingReport; } -function isChangeWorkspaceAction(report: Report): boolean { +function isChangeWorkspaceAction(report: Report, policy?: Policy): boolean { const policies = getAllPolicies(); const session = getSession(); - return policies.filter((newPolicy) => isWorkspaceEligibleForReportChange(newPolicy, report, session)).length > 0; + return policies.filter((newPolicy) => isWorkspaceEligibleForReportChange(newPolicy, report, session, policy)).length > 0; } function isDeleteAction(report: Report, reportTransactions: Transaction[]): boolean { @@ -391,7 +391,7 @@ function getSecondaryReportActions( options.push(CONST.REPORT.SECONDARY_ACTIONS.DOWNLOAD); - if (isChangeWorkspaceAction(report)) { + if (isChangeWorkspaceAction(report, policy)) { options.push(CONST.REPORT.SECONDARY_ACTIONS.CHANGE_WORKSPACE); } diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index c3bdaf6ee313c..da0476fcd7ad7 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10398,7 +10398,7 @@ function verifyStatus(report: OnyxEntry, validStatuses: Array, report: OnyxEntry, session: OnyxEntry): boolean { +function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report: OnyxEntry, session: OnyxEntry, currentPolicy?: OnyxEntry): boolean { if (!session?.accountID) { return false; } @@ -10409,6 +10409,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report const managerLogin = report?.managerID && getLoginByAccountID(report?.managerID); const isManagerMember = !!managerLogin && !!newPolicy?.employeeList?.[managerLogin]; const isCurrentUserAdmin = isPolicyAdminPolicyUtils(newPolicy, session?.email); + const isAdminOfCurrentPolicy = isPolicyAdminPolicyUtils(currentPolicy, session?.email); const isPaidGroupPolicyType = isPaidGroupPolicyPolicyUtils(newPolicy); const isReportOpenOrSubmitted = verifyState(report, [CONST.REPORT.STATE_NUM.OPEN, CONST.REPORT.STATE_NUM.SUBMITTED]); @@ -10423,7 +10424,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. // Additionally, if the report is not open or submitted, the current user must be an admin. const isExpenseReportType = isExpenseReport(report); - if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin) || (!isReportOpenOrSubmitted && !isCurrentUserAdmin)) { + if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin) || (!isReportOpenOrSubmitted && !isAdminOfCurrentPolicy)) { return false; } diff --git a/src/pages/ReportChangeWorkspacePage.tsx b/src/pages/ReportChangeWorkspacePage.tsx index 5b09e6ad58a5a..2325bd95538bb 100644 --- a/src/pages/ReportChangeWorkspacePage.tsx +++ b/src/pages/ReportChangeWorkspacePage.tsx @@ -56,13 +56,14 @@ function ReportChangeWorkspacePage({report}: ReportChangeWorkspacePageProps) { [session?.email, report, reportID], ); + const reportPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`]; const {sections, shouldShowNoResultsFoundMessage, shouldShowSearchInput} = useWorkspaceList({ policies, currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, selectedPolicyID: report.policyID, searchTerm: debouncedSearchTerm, - additionalFilter: (newPolicy) => isWorkspaceEligibleForReportChange(newPolicy, report, session), + additionalFilter: (newPolicy) => isWorkspaceEligibleForReportChange(newPolicy, report, session, reportPolicy), }); if (!isMoneyRequestReport(report) || isMoneyRequestReportPendingDeletion(report) || (!report.total && !report.unheldTotal)) { From d1fb007e170e952ed950369badaaad01cd630c17 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 21:05:34 +0200 Subject: [PATCH 7/9] disable workflow if user is not member --- src/libs/ReportUtils.ts | 3 ++- tests/unit/PolicyUtilsTest.ts | 12 +++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index da0476fcd7ad7..cbec1e28f8b6a 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -10423,8 +10423,9 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report // From this point on, reports must be of type Expense, the policy must be a paid type. // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. // Additionally, if the report is not open or submitted, the current user must be an admin. + // We're temporarily disabling moving reports to a workspace if the submitter is not a member of the new policy because this flow requires additional API changes. const isExpenseReportType = isExpenseReport(report); - if (!isExpenseReportType || !isPaidGroupPolicyType || !((isSubmitterMember && isManagerMember) || isCurrentUserAdmin) || (!isReportOpenOrSubmitted && !isAdminOfCurrentPolicy)) { + if (!isExpenseReportType || !isPaidGroupPolicyType || !isSubmitterMember || (!isReportOpenOrSubmitted && !isAdminOfCurrentPolicy)) { return false; } diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 8b23ffa68c5a3..bcb60c0f15ac5 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -856,6 +856,15 @@ describe('PolicyUtils', () => { role: CONST.POLICY.ROLE.ADMIN, employeeList: { [currentUserLogin]: {email: currentUserLogin, role: CONST.POLICY.ROLE.ADMIN}, + [employeeEmail]: {email: employeeEmail, role: CONST.POLICY.ROLE.USER}, + }, + }; + const currentPolicy = { + ...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM), + role: CONST.POLICY.ROLE.ADMIN, + employeeList: { + [currentUserLogin]: {email: currentUserLogin, role: CONST.POLICY.ROLE.ADMIN}, + [employeeEmail]: {email: employeeEmail, role: CONST.POLICY.ROLE.USER}, }, }; const report = { @@ -864,9 +873,10 @@ describe('PolicyUtils', () => { stateNum: CONST.REPORT.STATE_NUM.APPROVED, type: CONST.REPORT.TYPE.EXPENSE, managerID: approverAccountID, + policyID: currentPolicy.id, }; - expect(isWorkspaceEligibleForReportChange(newPolicy, report, session)).toBe(true); + expect(isWorkspaceEligibleForReportChange(newPolicy, report, session, currentPolicy)).toBe(true); }); }); From c779d9f8cdfbce6b36eff44e2689baeae12ddae3 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 21:09:43 +0200 Subject: [PATCH 8/9] fix test --- tests/unit/ReportSecondaryActionUtilsTest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index cf946c16af949..5f6db08166d4b 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -434,7 +434,7 @@ describe('getSecondaryAction', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.CHANGE_WORKSPACE)).toBe(true); }); - it('includes CHANGE_WORKSPACE option for opened expense report submitter', async () => { + it('includes CHANGE_WORKSPACE option for open expense report submitter', async () => { const report = { reportID: REPORT_ID, type: CONST.REPORT.TYPE.EXPENSE, @@ -454,6 +454,7 @@ describe('getSecondaryAction', () => { role: CONST.POLICY.ROLE.ADMIN, employeeList: { [ADMIN_EMAIL]: {email: ADMIN_EMAIL, role: CONST.POLICY.ROLE.ADMIN}, + [EMPLOYEE_EMAIL]: {email: EMPLOYEE_EMAIL, role: CONST.POLICY.ROLE.USER}, }, } as unknown as Policy; From e6222a2f7cc2318ce1d1dfe24f803af5bc7c6052 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 8 May 2025 21:15:51 +0200 Subject: [PATCH 9/9] fix another test --- tests/unit/ReportSecondaryActionUtilsTest.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 5f6db08166d4b..b8769c119edae 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -576,6 +576,16 @@ describe('getSecondaryAction', () => { }); it('includes CHANGE_WORKSPACE option for admin', async () => { + const policy = { + id: POLICY_ID, + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + employeeList: { + [ADMIN_EMAIL]: {email: ADMIN_EMAIL, role: CONST.POLICY.ROLE.ADMIN}, + [EMPLOYEE_EMAIL]: {login: EMPLOYEE_EMAIL, role: CONST.POLICY.ROLE.USER}, + }, + } as unknown as Policy; + const report = { reportID: REPORT_ID, type: CONST.REPORT.TYPE.EXPENSE, @@ -583,6 +593,7 @@ describe('getSecondaryAction', () => { managerID: MANAGER_ACCOUNT_ID, statusNum: CONST.REPORT.STATUS_NUM.REIMBURSED, stateNum: CONST.REPORT.STATE_NUM.APPROVED, + policyID: policy.id, } as unknown as Report; const personalDetails = { @@ -590,15 +601,6 @@ describe('getSecondaryAction', () => { [ADMIN_ACCOUNT_ID]: {login: ADMIN_EMAIL}, }; - const policy = { - id: POLICY_ID, - type: CONST.POLICY.TYPE.TEAM, - role: CONST.POLICY.ROLE.ADMIN, - employeeList: { - [ADMIN_EMAIL]: {email: ADMIN_EMAIL, role: CONST.POLICY.ROLE.ADMIN}, - }, - } as unknown as Policy; - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy); await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); await Onyx.merge(ONYXKEYS.SESSION, {email: ADMIN_EMAIL, accountID: ADMIN_ACCOUNT_ID});