Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useSearchShouldCalculateTotals from '@hooks/useSearchShouldCalculateTotals';
import useShareActions from '@hooks/useShareActions';
import useShouldReturnTripRooms from '@hooks/useShouldReturnTripRooms';
import useStrictPolicyRules from '@hooks/useStrictPolicyRules';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViolationsForReport';
Expand Down Expand Up @@ -103,6 +104,7 @@ const MORE_MENU_SUBMIT_TO_POPOVER_ANCHOR_ALIGNMENT = {
};

function MoneyReportHeaderSecondaryActionsInner({reportID, primaryAction, isReportInSearch, backTo, dropdownMenuRef, isDisabled}: MoneyReportHeaderSecondaryActionsProps) {
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();
const {isPaidAnimationRunning, isApprovedAnimationRunning, startAnimation, startApprovedAnimation, startSubmittingAnimation} = usePaymentAnimationsContext();
const {openHoldMenu, openPDFDownload, openHoldEducational, openRejectModal} = useMoneyReportHeaderModals();

Expand Down Expand Up @@ -416,6 +418,7 @@ function MoneyReportHeaderSecondaryActionsInner({reportID, primaryAction, isRepo
// Compute list of applicable secondary action keys
const secondaryActions = moneyRequestReport
? getSecondaryReportActions({
areStrictPolicyRulesEnabled,
currentUserLogin: currentUserLogin ?? '',
currentUserAccountID: accountID,
submitterLogin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {usePersonalDetail} from '@hooks/usePersonalDetails';
import useReportIsArchived from '@hooks/useReportIsArchived';
import useSelectedTransactionsActions from '@hooks/useSelectedTransactionsActions';
import useSelectionModePayment from '@hooks/useSelectionModePayment';
import useStrictPolicyRules from '@hooks/useStrictPolicyRules';
import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViolationsForReport';
import useTransactionThreadReport from '@hooks/useTransactionThreadReport';

Expand Down Expand Up @@ -56,6 +57,7 @@ type MoneyReportHeaderSelectionDropdownProps = {
};

function MoneyReportHeaderSelectionDropdown({reportID, primaryAction, isReportInSearch, wrapperStyle}: MoneyReportHeaderSelectionDropdownProps) {
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();
const route = useRoute();
const {startAnimation, startApprovedAnimation, startSubmittingAnimation} = usePaymentAnimationsContext();
const {openHoldMenu: openHoldMenuAsync, openRejectModal} = useMoneyReportHeaderModals();
Expand Down Expand Up @@ -153,6 +155,7 @@ function MoneyReportHeaderSelectionDropdown({reportID, primaryAction, isReportIn

const computedSecondaryActions = moneyRequestReport
? getSecondaryReportActions({
areStrictPolicyRulesEnabled,
currentUserLogin: currentUserLogin ?? '',
currentUserAccountID: accountID,
submitterLogin,
Expand Down
23 changes: 22 additions & 1 deletion src/hooks/useSearchBulkActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ import {
isIndividualInvoiceRoom,
isInvoiceReport,
isIOUReport as isIOUReportUtil,
isMoneyRequestReportPendingDeletion,
isProcessingReport,
isSelfDM,
shouldShowMarkAsDone,
} from '@libs/ReportUtils';
Expand Down Expand Up @@ -190,6 +192,7 @@ import useSearchShouldCalculateTotals from './useSearchShouldCalculateTotals';
import useSelfDMReport from './useSelfDMReport';
import useShouldReturnTripRooms from './useShouldReturnTripRooms';
import useSplitEffectivePolicy from './useSplitEffectivePolicy';
import useStrictPolicyRules from './useStrictPolicyRules';
import useTheme from './useTheme';
import useThemeStyles from './useThemeStyles';
import useUndeleteTransactions from './useUndeleteTransactions';
Expand Down Expand Up @@ -691,6 +694,7 @@ function getChatReportForBulkPay(
type ExportMenuItem = DropdownOption<SearchHeaderOptionValue> & Pick<PopoverMenuItem, 'accessibilityLabel' | 'shouldCallAfterModalHide'>;

function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();
const {translate, localeCompare, dateFnsLocale} = useLocalize();
const styles = useThemeStyles();
const theme = useTheme();
Expand Down Expand Up @@ -3269,7 +3273,23 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
!isAnyTransactionOnHold &&
areSelectedTransactionsIncludedInReports &&
selectedReports.length &&
selectedReports.every((report) => report.canChangeApprover);
selectedReports.every((selectedReport) => {
if (selectedReport.canChangeApprover) {
return true;
}
const report = getReportFromSearchSnapshot(selectedReport.reportID, currentSearchResults?.data, allReports);
if (
areStrictPolicyRulesEnabled ||
report?.managerID !== accountID ||
Comment thread
hungvu193 marked this conversation as resolved.
Resolved
!isExpenseReportUtil(report) ||
!isProcessingReport(report) ||
isMoneyRequestReportPendingDeletion(report)
) {
return false;
}
const policy = getPolicyFromSearchSnapshot(selectedReport.policyID, currentSearchResults?.data, policies);
return !!policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL;
});

if (shouldShowChangeApproverOption) {
options.push({
Expand Down Expand Up @@ -3622,6 +3642,7 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
return buildResult(options);
}, [
selectedTransactionsKeys,
areStrictPolicyRulesEnabled,
showOfflineModal,
showDownloadErrorModal,
cardList,
Expand Down
3 changes: 3 additions & 0 deletions src/hooks/useSelectionModeReportActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import useOnyx from './useOnyx';
import {usePersonalDetail} from './usePersonalDetails';
import useReportIsArchived from './useReportIsArchived';
import useSelectionModePayment from './useSelectionModePayment';
import useStrictPolicyRules from './useStrictPolicyRules';

type UseSelectionModeReportActionsParams = {
report: OnyxEntry<OnyxTypes.Report>;
Expand All @@ -58,6 +59,7 @@ function useSelectionModeReportActions({
transactions,
selectedTransactionIDs,
}: UseSelectionModeReportActionsParams) {
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();
const {translate} = useLocalize();
const {isOffline} = useNetwork();
const {convertToDisplayString} = useCurrencyListActions();
Expand Down Expand Up @@ -180,6 +182,7 @@ function useSelectionModeReportActions({
return [];
}
return getSecondaryReportActions({
areStrictPolicyRulesEnabled,
currentUserLogin: currentUserEmail,
currentUserAccountID,
submitterLogin,
Expand Down
5 changes: 4 additions & 1 deletion src/libs/ReportSecondaryActionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,7 @@ function getSecondaryReportActions({
isOffline,
rules,
cardList,
areStrictPolicyRulesEnabled = true,
}: {
currentUserLogin: string;
currentUserAccountID: number;
Expand All @@ -1071,6 +1072,7 @@ function getSecondaryReportActions({
isOffline?: boolean;
rules: OnyxCollection<Rule>;
cardList: OnyxEntry<CardList>;
areStrictPolicyRulesEnabled?: boolean;
}): Array<ValueOf<typeof CONST.REPORT.SECONDARY_ACTIONS>> {
const options: Array<ValueOf<typeof CONST.REPORT.SECONDARY_ACTIONS>> = [];
const reportNameValuePairs = moveExpenseReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`];
Expand Down Expand Up @@ -1236,7 +1238,8 @@ function getSecondaryReportActions({
}

const isApprovalEnabled = policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL;
if (isExpenseReportUtils(report) && isProcessingReportUtils(report) && isPolicyAdmin(policy) && isApprovalEnabled) {
const canReassignAsApprover = report.managerID === currentUserAccountID && !areStrictPolicyRulesEnabled;
if (isExpenseReportUtils(report) && isProcessingReportUtils(report) && (isPolicyAdmin(policy) || canReassignAsApprover) && isApprovalEnabled) {
options.push(CONST.REPORT.SECONDARY_ACTIONS.CHANGE_APPROVER);
}

Expand Down
6 changes: 5 additions & 1 deletion src/pages/DynamicReportChangeApproverPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import useEnvironment from '@hooks/useEnvironment';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import usePermissions from '@hooks/usePermissions';
import useStrictPolicyRules from '@hooks/useStrictPolicyRules';
import useThemeStyles from '@hooks/useThemeStyles';

import {assignReportToMe} from '@libs/actions/IOU/ReportWorkflow';
Expand Down Expand Up @@ -54,6 +55,7 @@ function DynamicReportChangeApproverPage({report, policy, isLoadingReportData}:
const currentUserDetails = useCurrentUserPersonalDetails();
const [selectedApproverType, setSelectedApproverType] = useState<ApproverType>(APPROVER_TYPE.ADD_APPROVER);
const {isBetaEnabled} = usePermissions();
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();

const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT);
Expand All @@ -62,6 +64,8 @@ function DynamicReportChangeApproverPage({report, policy, isLoadingReportData}:
const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE);
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_CHANGE_APPROVER.path);

const canMemberReassignApprover = !areStrictPolicyRulesEnabled && currentUserDetails.accountID === report?.managerID;
Comment thread
hungvu193 marked this conversation as resolved.
Resolved

const goBack = () => {
Navigation.goBack(backPath);
};
Expand Down Expand Up @@ -106,7 +110,7 @@ function DynamicReportChangeApproverPage({report, policy, isLoadingReportData}:
isSelected: selectedApproverType === APPROVER_TYPE.BYPASS_APPROVER,
});
}
if (isPolicyAdmin(policy) && !isPendingDeletePolicy(policy)) {
if ((isPolicyAdmin(policy) || canMemberReassignApprover) && !isPendingDeletePolicy(policy)) {
Comment thread
hungvu193 marked this conversation as resolved.
Resolved
data.push({
text: translate('iou.changeApprover.actions.reassignApprover'),
keyForList: APPROVER_TYPE.REASSIGN_APPROVER,
Expand Down
11 changes: 9 additions & 2 deletions src/pages/Search/SearchChangeApproverPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useOnyx from '@hooks/useOnyx';
import usePermissions from '@hooks/usePermissions';
import useStrictPolicyRules from '@hooks/useStrictPolicyRules';
import useThemeStyles from '@hooks/useThemeStyles';

import {assignReportToMe} from '@libs/actions/IOU/ReportWorkflow';
Expand Down Expand Up @@ -78,22 +79,27 @@ function canReassignAllReports({
selectedReports,
onyxReports,
allPolicies,
currentUserAccountID,
areStrictPolicyRulesEnabled,
}: {
selectedReports: SelectedReportRef[];
onyxReports: Record<string, Report> | undefined;
allPolicies: OnyxCollection<Policy>;
currentUserAccountID: number;
areStrictPolicyRulesEnabled: boolean;
}): boolean {
return (
selectedReports.length > 0 &&
selectedReports.every((selectedReport) => {
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${selectedReport.policyID}`];
const report = selectedReport.reportID ? onyxReports?.[selectedReport.reportID] : undefined;
const isApprovalEnabled = !!policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL;
const canReassignAsApprover = report?.managerID === currentUserAccountID && !areStrictPolicyRulesEnabled;

return (
!!policy &&
!!report &&
isPolicyAdmin(policy) &&
(isPolicyAdmin(policy) || canReassignAsApprover) &&
Comment thread
hungvu193 marked this conversation as resolved.
Resolved
!isPendingDeletePolicy(policy) &&
isMoneyRequestReport(report) &&
!isMoneyRequestReportPendingDeletion(report) &&
Expand All @@ -113,6 +119,7 @@ function SearchChangeApproverPage() {
const currentUserDetails = useCurrentUserPersonalDetails();
const [selectedApproverType, setSelectedApproverType] = useState<ApproverType>(APPROVER_TYPE.ADD_APPROVER);
const {isBetaEnabled} = usePermissions();
const {areStrictPolicyRulesEnabled} = useStrictPolicyRules();
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT);
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
Expand Down Expand Up @@ -244,7 +251,7 @@ function SearchChangeApproverPage() {
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${selectedReport.policyID}`];
return !!policy && isPolicyAdmin(policy) && !isPendingDeletePolicy(policy);
});
const canReassign = canReassignAllReports({selectedReports, onyxReports, allPolicies});
const canReassign = canReassignAllReports({selectedReports, onyxReports, allPolicies, currentUserAccountID: currentUserDetails.accountID, areStrictPolicyRulesEnabled});
Comment thread
hungvu193 marked this conversation as resolved.
Resolved

const isAllowedToBypassApprovers =
hasAdminPermission &&
Expand Down
41 changes: 41 additions & 0 deletions tests/unit/ReportSecondaryActionUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,47 @@ describe('getSecondaryAction', () => {
Onyx.clear();
});

it.each([
[false, false, MANAGER_ACCOUNT_ID, true],
[false, true, MANAGER_ACCOUNT_ID, false],
[false, false, APPROVER_ACCOUNT_ID, false],
[true, true, APPROVER_ACCOUNT_ID, true],
])('change approver visibility: admin=%s, enforcement=%s, manager=%s', (isAdmin, areStrictPolicyRulesEnabled, managerID, expected) => {
// Given an outstanding report and the caller's workspace enforcement setting.
jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin).mockReturnValue(isAdmin);
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
managerID,
});
const policy = createMock<Policy>({
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
});

// When the caller opens the report menu.
const actions = getSecondaryReportActions({
currentUserLogin: MANAGER_EMAIL,
currentUserAccountID: MANAGER_ACCOUNT_ID,
submitterLogin: EMPLOYEE_EMAIL,
report,
chatReport,
reportTransactions: [],
originalTransaction: undefined,
violations: {},
bankAccountList: {},
policy,
rules: undefined,
cardList: undefined,
areStrictPolicyRulesEnabled,
});

// Then only admins and eligible current approvers can change the approver.
expect(actions.includes(CONST.REPORT.SECONDARY_ACTIONS.CHANGE_APPROVER)).toBe(expected);
jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin).mockReturnValue(true);
});

it('should always return default options', () => {
const report = createMock<Report>({});
const policy = createMock<Policy>({});
Expand Down
56 changes: 56 additions & 0 deletions tests/unit/hooks/useSearchBulkActionsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ jest.mock('@hooks/useTheme', () => ({
default: () => ({icon: ''}),
}));

let mockAreStrictPolicyRulesEnabled = false;
jest.mock('@hooks/useStrictPolicyRules', () => ({
__esModule: true,
default: () => ({areStrictPolicyRulesEnabled: mockAreStrictPolicyRulesEnabled}),
}));

let mockIsOffline = false;
jest.mock('@hooks/useNetwork', () => ({
__esModule: true,
Expand Down Expand Up @@ -330,6 +336,7 @@ describe('useSearchBulkActions - CSV export flow', () => {
beforeEach(async () => {
jest.clearAllMocks();
mockIsOffline = false;
mockAreStrictPolicyRulesEnabled = false;
mockAreAllMatchingItemsSelected = false;
await Onyx.clear();
mockSelectedTransactions = {};
Expand All @@ -345,6 +352,55 @@ describe('useSearchBulkActions - CSV export flow', () => {
await Onyx.clear();
});

it.each([
[false, CURRENT_USER_ACCOUNT_ID, false, CONST.REPORT.STATUS_NUM.SUBMITTED, true],
[true, CURRENT_USER_ACCOUNT_ID, false, CONST.REPORT.STATUS_NUM.SUBMITTED, false],
[false, 2, false, CONST.REPORT.STATUS_NUM.SUBMITTED, false],
[true, 2, true, CONST.REPORT.STATUS_NUM.SUBMITTED, true],
[false, CURRENT_USER_ACCOUNT_ID, false, CONST.REPORT.STATUS_NUM.APPROVED, false],
])('change approver visibility: enforcement=%s, manager=%s, admin capability=%s, status=%s', (enforcement, managerID, canChangeApprover, statusNum, expected) => {
// Given a selected report whose snapshot may not grant the admin-only capability.
mockAreStrictPolicyRulesEnabled = enforcement;
mockSelectedTransactions = {transaction1: makeSelectedTransaction()};
mockSelectedReports = [
{
reportID: 'report1',
policyID: 'policy1',
chatReportID: 'chat1',
action: CONST.SEARCH.ACTION_TYPES.VIEW,
canPay: false,
canApprove: false,
canSubmit: false,
canChangeApprover,
managerID,
total: 100,
},
];
mockCurrentSearchResults = {
search: {type: CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT},
data: {
[`${ONYXKEYS.COLLECTION.REPORT}report1`]: {
reportID: 'report1',
policyID: 'policy1',
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum,
managerID,
},
[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: {
id: 'policy1',
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
},
},
};

// When the bulk action menu is rendered from search snapshot data.
const {result} = renderHook(() => useSearchBulkActions({queryJSON: expenseReportQueryJSON}), {wrapper: OnyxListItemProvider});

// Then the menu respects assignment and enforcement while preserving admin capabilities.
expect(result.current.headerButtonsOptions.some((option) => option.value === CONST.SEARCH.BULK_ACTION_TYPES.CHANGE_APPROVER)).toBe(expected);
});

it('uses the unreported-expenses warning for a single expense report', async () => {
// Given a single selected expense report
mockSelectedTransactions = {report1: makeSelectedTransaction()};
Expand Down
Loading
Loading