From 4b059bfeb0c2635694a1dcb8bb834cafd182aaa8 Mon Sep 17 00:00:00 2001 From: truph01 Date: Sat, 14 Mar 2026 14:21:51 +0700 Subject: [PATCH 1/3] fix: Change workspace option is shown in paid expense in 1:1 chat --- src/libs/ReportSecondaryActionUtils.ts | 9 ++++++-- tests/unit/ReportSecondaryActionUtilsTest.ts | 22 ++++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 2cd9c0a30d399..a82532193e1e0 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -567,7 +567,7 @@ function isHoldActionForTransaction(report: Report, reportTransaction: Transacti return isProcessingReport; } -function isChangeWorkspaceAction(report: Report, policies: OnyxCollection, reportActions?: ReportAction[]): boolean { +function isChangeWorkspaceAction(report: Report, policies: OnyxCollection, currentUserLogin: string, reportActions?: ReportAction[]): boolean { // We can't move the iou report to the workspace if both users from the iou report create the expense if (isIOUReportUtils(report) && doesReportContainRequestsFromMultipleUsers(report)) { return false; @@ -581,6 +581,7 @@ function isChangeWorkspaceAction(report: Report, policies: OnyxCollection { const report = createReport({type: CONST.REPORT.TYPE.IOU}); const policies = createPolicies(POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); }); it('should return false when IOU report and user is neither submitter nor manager', () => { @@ -3506,7 +3506,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({type: CONST.REPORT.TYPE.IOU}); const policies = createPolicies(POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); }); it('should return false when there are no available policies', () => { @@ -3514,7 +3514,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport(); const policies = createPolicies(POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); }); it('should return false when only one available policy and it is the same as current report policy', () => { @@ -3522,7 +3522,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: POLICY_ID}); const policies = createPolicies(POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); }); it('should return true when only one available policy but report has no policy', () => { @@ -3530,7 +3530,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: undefined}); const policies = createPolicies(POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(true); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); it('should return true when only one available policy and it is different from current report policy', () => { @@ -3538,7 +3538,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(true); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); it('should return false when cannot edit report policy', () => { @@ -3546,7 +3546,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); }); it('should return false when report is exported', () => { @@ -3554,7 +3554,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); - expect(isChangeWorkspaceAction(report, policies, [])).toBe(false); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL, [])).toBe(false); }); it('should return true when multiple available policies exist', () => { @@ -3562,7 +3562,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID, 'another_policy'); - expect(isChangeWorkspaceAction(report, policies)).toBe(true); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); it('should return true when IOU report with single user and user is submitter', () => { @@ -3570,7 +3570,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({type: CONST.REPORT.TYPE.IOU, policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(true); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); it('should return true when IOU report with single user and user is manager', () => { @@ -3578,7 +3578,7 @@ describe('getSecondaryTransactionThreadActions', () => { const report = createReport({type: CONST.REPORT.TYPE.IOU, policyID: OLD_POLICY_ID}); const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); - expect(isChangeWorkspaceAction(report, policies)).toBe(true); + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); }); }); From f1d8a432defe619bef68c7a286b196e54ef9d1ad Mon Sep 17 00:00:00 2001 From: truph01 Date: Wed, 18 Mar 2026 16:01:36 +0700 Subject: [PATCH 2/3] fix: add test --- tests/unit/ReportSecondaryActionUtilsTest.ts | 70 ++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 75af20e2df334..c54b0a6c52b54 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -3592,6 +3592,7 @@ describe('getSecondaryTransactionThreadActions', () => { isWorkspaceEligibleForReportChange: MockFunction; canEditReportPolicy: boolean; isExported: boolean; + isSettled: boolean; }>; const setupMocks = (mocks: MockConfig = {}) => { @@ -3603,6 +3604,7 @@ describe('getSecondaryTransactionThreadActions', () => { isWorkspaceEligibleForReportChange: true, canEditReportPolicy: true, isExported: false, + isSettled: false, }; for (const [method, value] of Object.entries({...defaults, ...mocks})) { @@ -3706,5 +3708,73 @@ describe('getSecondaryTransactionThreadActions', () => { expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); }); + + it('should return true when report is settled and currentUserLogin is admin of available policies', () => { + setupMocks({isSettled: true}); + const mockedIsPolicyAdmin = jest.requireMock('@libs/PolicyUtils').isPolicyAdmin as jest.Mock; + mockedIsPolicyAdmin.mockReturnValue(true); + + const report = createReport({policyID: OLD_POLICY_ID}); + const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); + + expect(isChangeWorkspaceAction(report, policies, ADMIN_EMAIL)).toBe(true); + }); + + it('should return false when report is settled and currentUserLogin is not admin of any policy', () => { + setupMocks({isSettled: true}); + const mockedIsPolicyAdmin = jest.requireMock('@libs/PolicyUtils').isPolicyAdmin as jest.Mock; + mockedIsPolicyAdmin.mockReturnValue(false); + + const report = createReport({policyID: OLD_POLICY_ID}); + const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); + + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); + }); + + it('should filter policies by admin role using currentUserLogin when report is settled', () => { + setupMocks({isSettled: true}); + const mockedIsPolicyAdmin = jest.requireMock('@libs/PolicyUtils').isPolicyAdmin as jest.Mock; + mockedIsPolicyAdmin.mockImplementation((policy: Policy, login?: string) => { + return login === ADMIN_EMAIL && policy?.id === POLICY_ID; + }); + + const report = createReport({policyID: OLD_POLICY_ID}); + const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); + + // Admin user sees the one eligible policy (POLICY_ID) which differs from report's OLD_POLICY_ID + expect(isChangeWorkspaceAction(report, policies, ADMIN_EMAIL)).toBe(true); + // Non-admin user has all policies filtered out + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(false); + }); + + it('should not filter policies by admin role when report is not settled', () => { + setupMocks({isSettled: false}); + const mockedIsPolicyAdmin = jest.requireMock('@libs/PolicyUtils').isPolicyAdmin as jest.Mock; + mockedIsPolicyAdmin.mockReturnValue(false); + + const report = createReport({policyID: OLD_POLICY_ID}); + const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); + + // Even though isPolicyAdmin returns false, non-settled reports skip the admin check + expect(isChangeWorkspaceAction(report, policies, EMPLOYEE_EMAIL)).toBe(true); + }); + + it('should pass currentUserLogin to isPolicyAdmin for each candidate policy when settled', () => { + setupMocks({isSettled: true}); + const mockedIsPolicyAdmin = jest.requireMock('@libs/PolicyUtils').isPolicyAdmin as jest.Mock; + mockedIsPolicyAdmin.mockReturnValue(true); + + const report = createReport({policyID: OLD_POLICY_ID}); + const policies = createPolicies(POLICY_ID, OLD_POLICY_ID); + const testLogin = 'specific-user@mail.com'; + + isChangeWorkspaceAction(report, policies, testLogin); + + const callsWithLogin = mockedIsPolicyAdmin.mock.calls.filter( + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + (call: unknown[]) => call[1] === testLogin, + ); + expect(callsWithLogin.length).toBeGreaterThan(0); + }); }); }); From 82e578ecd3d0db0def210061e4a44c71543b2a16 Mon Sep 17 00:00:00 2001 From: truph01 Date: Wed, 18 Mar 2026 16:06:25 +0700 Subject: [PATCH 3/3] fix: lint --- tests/unit/ReportSecondaryActionUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index c54b0a6c52b54..e925eeba8840e 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -3772,7 +3772,7 @@ describe('getSecondaryTransactionThreadActions', () => { const callsWithLogin = mockedIsPolicyAdmin.mock.calls.filter( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - (call: unknown[]) => call[1] === testLogin, + (call: unknown[]) => call.at(1) === testLogin, ); expect(callsWithLogin.length).toBeGreaterThan(0); });