Skip to content

Commit 9ba3c5d

Browse files
mjasikowskimjasikowski
authored
Merge pull request Expensify#84682 from Expensify/claude-fixApproverInsightsVisibility
2 parents ec708e7 + 63df756 commit 9ba3c5d

6 files changed

Lines changed: 91 additions & 7 deletions

File tree

‎src/libs/SearchUIUtils.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,19 @@ function getSuggestedSearchesVisibility(
984984
const isPayer = isPolicyPayer(policy, currentUserEmail);
985985
const isAdmin = policy.role === CONST.POLICY.ROLE.ADMIN;
986986
const isExporter = policy.exporter === currentUserEmail;
987-
const isApprover = policy.approver === currentUserEmail;
987+
let isSubmittedTo = false;
988+
let isUserApprover = policy.approver === currentUserEmail;
989+
for (const employee of Object.values(policy.employeeList ?? {})) {
990+
if (employee?.submitsTo === currentUserEmail || employee?.forwardsTo === currentUserEmail) {
991+
isSubmittedTo = true;
992+
isUserApprover = true;
993+
} else if (employee?.overLimitForwardsTo === currentUserEmail) {
994+
isUserApprover = true;
995+
}
996+
if (isSubmittedTo && isUserApprover) {
997+
break;
998+
}
999+
}
9881000
const isApprovalEnabled = policy.approvalMode ? policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false;
9891001

9901002
const hasExportError = (Object.keys(policy.connections ?? {}) as ConnectionName[]).some((connection) => {
@@ -994,13 +1006,10 @@ function getSuggestedSearchesVisibility(
9941006
const hasVBBA = !!policy.achAccount?.bankAccountID && policy.achAccount.state === CONST.BANK_ACCOUNT.STATE.OPEN;
9951007
const hasReimburser = !!policy.achAccount?.reimburser;
9961008
const isECardEnabled = !!policy.areExpensifyCardsEnabled;
997-
const isSubmittedTo = Object.values(policy.employeeList ?? {}).some((employee) => {
998-
return employee.submitsTo === currentUserEmail || employee.forwardsTo === currentUserEmail;
999-
});
10001009

10011010
const isEligibleForSubmitSuggestion = isPaidPolicy;
10021011
const isEligibleForPaySuggestion = isPaidPolicy && isPayer;
1003-
const isPolicyEligibleForApproveSuggestion = isPaidPolicy && isEligibleForApproveSuggestion(policy.approvalMode, isApprover, isSubmittedTo);
1012+
const isPolicyEligibleForApproveSuggestion = isPaidPolicy && isEligibleForApproveSuggestion(policy.approvalMode, isUserApprover, isSubmittedTo);
10041013
const isEligibleForExportSuggestion = isExporter && !hasExportError;
10051014
const isEligibleForStatementsSuggestion = isPaidPolicy && !!policy.areCompanyCardsEnabled && hasCardFeed;
10061015
const isEligibleForUnapprovedCashSuggestion = isPaidPolicy && isAdmin && isApprovalEnabled && isPaymentEnabled;
@@ -1009,10 +1018,10 @@ function getSuggestedSearchesVisibility(
10091018
const isEligibleForReimbursementsSuggestion = isPaidPolicy && isAdmin && isPaymentEnabled && hasVBBA && hasReimburser;
10101019
const isAuditor = policy.role === CONST.POLICY.ROLE.AUDITOR;
10111020
const memberCount = Object.keys(policy.employeeList ?? {}).length;
1012-
const isEligibleForTopSpendersSuggestion = isPaidPolicy && (isAdmin || isAuditor || isApprover) && memberCount >= 2;
1021+
const isEligibleForTopSpendersSuggestion = isPaidPolicy && (isAdmin || isAuditor || isUserApprover) && memberCount >= 2;
10131022
const isEligibleForTopCategoriesSuggestion = isPaidPolicy && policy.areCategoriesEnabled === true;
10141023
const isEligibleForTopMerchantsSuggestion = isPaidPolicy;
1015-
const isEligibleForSpendOverTimeSuggestion = isPaidPolicy && (isAdmin || isAuditor || isApprover);
1024+
const isEligibleForSpendOverTimeSuggestion = isPaidPolicy && (isAdmin || isAuditor || isUserApprover);
10161025

10171026
shouldShowSubmitSuggestion ||= isEligibleForSubmitSuggestion;
10181027
shouldShowPaySuggestion ||= isEligibleForPaySuggestion;

‎src/pages/workspace/travel/WorkspaceTravelInvoicingExportPage.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ function WorkspaceTravelInvoicingExportPage({route}: WorkspaceTravelInvoicingExp
181181
setIsDownloading(false),
182182
);
183183
} else {
184+
// Intentional: this setState resets the loading indicator when generation completes but no file is available.
185+
// It runs in a cleanup path of the effect, not as a cascading re-render trigger.
186+
// eslint-disable-next-line react-hooks/set-state-in-effect
184187
setIsDownloading(false);
185188
}
186189
}, [prevIsGenerating, isGenerating, travelInvoiceStatement, policyID, getDateRange, translate, baseURL, currentUserPersonalDetails?.login, session?.encryptedAuthToken]);

‎src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
8787

8888
const defaultApprover = getDefaultApprover(policy);
8989

90+
// useCallback is needed here because goBack is passed as a prop to child components;
91+
// the rule flags it because the deps could be inlined, but removing useCallback would cause unnecessary re-renders.
92+
// eslint-disable-next-line react-hooks/preserve-manual-memoization
9093
const goBack = useCallback(() => {
9194
if ((!feature && featureNameAlias !== CONST.UPGRADE_FEATURE_INTRO_MAPPING.policyPreventMemberChangingTitle.alias) || !policyID) {
9295
Navigation.dismissModal();
@@ -131,6 +134,9 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
131134
upgradeToCorporate(policy.id, feature?.name);
132135
};
133136

137+
// useCallback is needed here because confirmUpgrade is passed as a prop to child components;
138+
// the rule flags it because the deps could be inlined, but removing useCallback would cause unnecessary re-renders.
139+
// eslint-disable-next-line react-hooks/preserve-manual-memoization
134140
const confirmUpgrade = useCallback(() => {
135141
if (!policyID) {
136142
return;

‎src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true
123123
errors: null,
124124
originalApprovers: currentApprovalWorkflow.approvers,
125125
});
126+
// Intentional: synchronizes the initial workflow snapshot when the current workflow changes.
127+
// This runs alongside setApprovalWorkflow above and is part of the same logical update.
128+
// eslint-disable-next-line react-hooks/set-state-in-effect
126129
setInitialApprovalWorkflow(currentApprovalWorkflow);
127130
}, [currentApprovalWorkflow, defaultWorkflowMembers, initialApprovalWorkflow, usedApproverEmails]);
128131

‎src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
4848
return;
4949
}
5050

51+
// Intentional: derives the selected-members list from the approval workflow data.
52+
// This effect synchronizes local component state with the Onyx-sourced workflow when it changes.
53+
// eslint-disable-next-line react-hooks/set-state-in-effect
5154
setSelectedMembers(
5255
approvalWorkflow.members.map((member) => {
5356
const policyMemberEmailsToAccountIDs = getMemberAccountIDsForWorkspace(policy?.employeeList);

‎tests/unit/Search/SearchUIUtilsTest.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6116,6 +6116,66 @@ describe('SearchUIUtils', () => {
61166116

61176117
expect(searchQuery).toContain(`view:${CONST.SEARCH.VIEW.PIE}`);
61186118
});
6119+
6120+
test('Should show Top Spenders for workflow approver (submitsTo) in paid policy', () => {
6121+
const workflowApproverEmail = 'workflow-approver@policy.com';
6122+
const policyKey = `policy_${policyID}`;
6123+
6124+
const policies: OnyxCollection<OnyxTypes.Policy> = {
6125+
[policyKey]: {
6126+
id: policyID,
6127+
type: CONST.POLICY.TYPE.TEAM,
6128+
approver: 'someone-else@policy.com',
6129+
employeeList: {
6130+
'employee1@policy.com': {submitsTo: workflowApproverEmail, forwardsTo: ''},
6131+
'employee2@policy.com': {submitsTo: '', forwardsTo: ''},
6132+
},
6133+
} as unknown as OnyxTypes.Policy,
6134+
};
6135+
6136+
const response = SearchUIUtils.getSuggestedSearchesVisibility(workflowApproverEmail, {}, policies, undefined);
6137+
expect(response.visibility.topSpenders).toBe(true);
6138+
});
6139+
6140+
test('Should show Spend Over Time for workflow approver (forwardsTo) in paid policy', () => {
6141+
const workflowApproverEmail = 'workflow-approver@policy.com';
6142+
const policyKey = `policy_${policyID}`;
6143+
6144+
const policies: OnyxCollection<OnyxTypes.Policy> = {
6145+
[policyKey]: {
6146+
id: policyID,
6147+
type: CONST.POLICY.TYPE.TEAM,
6148+
approver: 'someone-else@policy.com',
6149+
employeeList: {
6150+
'employee1@policy.com': {submitsTo: '', forwardsTo: workflowApproverEmail},
6151+
'employee2@policy.com': {submitsTo: '', forwardsTo: ''},
6152+
},
6153+
} as unknown as OnyxTypes.Policy,
6154+
};
6155+
6156+
const response = SearchUIUtils.getSuggestedSearchesVisibility(workflowApproverEmail, {}, policies, undefined);
6157+
expect(response.visibility.spendOverTime).toBe(true);
6158+
});
6159+
6160+
test('Should hide Top Spenders for regular member who is not a workflow approver', () => {
6161+
const regularEmail = 'regular@policy.com';
6162+
const policyKey = `policy_${policyID}`;
6163+
6164+
const policies: OnyxCollection<OnyxTypes.Policy> = {
6165+
[policyKey]: {
6166+
id: policyID,
6167+
type: CONST.POLICY.TYPE.TEAM,
6168+
approver: 'someone-else@policy.com',
6169+
employeeList: {
6170+
'employee1@policy.com': {submitsTo: 'someone-else@policy.com', forwardsTo: ''},
6171+
[regularEmail]: {submitsTo: '', forwardsTo: ''},
6172+
},
6173+
} as unknown as OnyxTypes.Policy,
6174+
};
6175+
6176+
const response = SearchUIUtils.getSuggestedSearchesVisibility(regularEmail, {}, policies, undefined);
6177+
expect(response.visibility.topSpenders).toBe(false);
6178+
});
61196179
});
61206180

61216181
describe('Test getSuggestedSearches sort defaults', () => {

0 commit comments

Comments
 (0)