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 56b242bd3210f..cbec1e28f8b6a 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]); @@ -10421,13 +10422,15 @@ 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)) { + if (!isExpenseReportType || !isPaidGroupPolicyType || !isSubmitterMember || (!isReportOpenOrSubmitted && !isAdminOfCurrentPolicy)) { return false; } const isCurrentUserReportSubmitter = session.accountID === report?.ownerAccountID; - if (isCurrentUserReportSubmitter && isReportOpenOrSubmitted) { + if (isCurrentUserReportSubmitter) { return true; } 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)) { diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index f0c51d3e0e9c7..bcb60c0f15ac5 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; @@ -831,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 = { @@ -839,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); }); }); diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index cf946c16af949..b8769c119edae 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; @@ -575,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, @@ -582,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 = { @@ -589,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});