Skip to content

Commit 9cdb9ce

Browse files
tgolentgolen
authored
Merge pull request Expensify#67054 from shubham1206agra/refactor-onyx-6
Refactored localeCompare in WorkflowUtils
2 parents 78cf216 + 8ed477c commit 9cdb9ce

6 files changed

Lines changed: 32 additions & 39 deletions

File tree

‎src/libs/WorkflowUtils.ts‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import lodashMapKeys from 'lodash/mapKeys';
22
import type {ValueOf} from 'type-fest';
3+
import type {LocaleContextProps} from '@components/LocaleContextProvider';
34
import CONST from '@src/CONST';
45
import type {ApprovalWorkflowOnyx, Approver, Member} from '@src/types/onyx/ApprovalWorkflow';
56
import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow';
67
import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails';
78
import type PersonalDetails from '@src/types/onyx/PersonalDetails';
89
import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee';
9-
import localeCompare from './LocaleCompare';
1010

1111
const INITIAL_APPROVAL_WORKFLOW: ApprovalWorkflowOnyx = {
1212
members: [],
@@ -70,46 +70,35 @@ function calculateApprovers({employees, firstEmail, personalDetailsByEmail}: Get
7070
}
7171

7272
type PolicyConversionParams = {
73-
/**
74-
* List of employees in the policy
75-
*/
73+
/** List of employees in the policy */
7674
employees: PolicyEmployeeList;
7775

78-
/**
79-
* Personal details of the employees
80-
*/
76+
/** Personal details of the employees */
8177
personalDetails: PersonalDetailsList;
8278

83-
/**
84-
* Email of the default approver for the policy
85-
*/
79+
/** Email of the default approver for the policy */
8680
defaultApprover: string;
8781

88-
/**
89-
* Email of the first approver in current edited workflow
90-
*/
82+
/** Email of the first approver in current edited workflow */
9183
firstApprover?: string;
84+
85+
/** Locale comparison function */
86+
localeCompare: LocaleContextProps['localeCompare'];
9287
};
9388

9489
type PolicyConversionResult = {
95-
/**
96-
* List of approval workflows
97-
*/
90+
/** List of approval workflows */
9891
approvalWorkflows: ApprovalWorkflow[];
9992

100-
/**
101-
* List of available members that can be selected in the workflow
102-
*/
93+
/** List of available members that can be selected in the workflow */
10394
availableMembers: Member[];
10495

105-
/**
106-
* Emails that are used as approvers in currently configured workflows
107-
*/
96+
/** Emails that are used as approvers in currently configured workflows */
10897
usedApproverEmails: string[];
10998
};
11099

111100
/** Convert a list of policy employees to a list of approval workflows */
112-
function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, firstApprover}: PolicyConversionParams): PolicyConversionResult {
101+
function convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, firstApprover, localeCompare}: PolicyConversionParams): PolicyConversionResult {
113102
const approvalWorkflows: Record<string, ApprovalWorkflow> = {};
114103

115104
// Keep track of used approver emails to display hints in the UI

‎src/pages/workspace/WorkspaceMembersPage.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers
108108
const [isDownloadFailureModalVisible, setIsDownloadFailureModalVisible] = useState(false);
109109
const isOfflineAndNoMemberDataAvailable = isEmptyObject(policy?.employeeList) && isOffline;
110110
const prevPersonalDetails = usePrevious(personalDetails);
111-
const {translate, formatPhoneNumber} = useLocalize();
111+
const {translate, formatPhoneNumber, localeCompare} = useLocalize();
112112
const {isAccountLocked, showLockedAccountModal} = useContext(LockedAccountContext);
113113
const filterEmployees = useCallback(
114114
(employee?: PolicyEmployee) => {
@@ -152,8 +152,9 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers
152152
employees: policy?.employeeList ?? {},
153153
defaultApprover: policyApproverEmail ?? policy?.owner ?? '',
154154
personalDetails: personalDetails ?? {},
155+
localeCompare,
155156
}),
156-
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail],
157+
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail, localeCompare],
157158
);
158159

159160
const canSelectMultiple = isPolicyAdmin && (shouldUseNarrowLayout ? isMobileSelectionModeEnabled : true);

‎src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
7373
const workspaceAccountID = policy?.workspaceAccountID ?? CONST.DEFAULT_NUMBER_ID;
7474

7575
const styles = useThemeStyles();
76-
const {formatPhoneNumber, translate} = useLocalize();
76+
const {formatPhoneNumber, translate, localeCompare} = useLocalize();
7777
const StyleUtils = useStyleUtils();
7878
const illustrations = useThemeIllustrations();
7979
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
@@ -111,8 +111,9 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
111111
employees: policy?.employeeList ?? {},
112112
defaultApprover: policyApproverEmail ?? policy?.owner ?? '',
113113
personalDetails: personalDetails ?? {},
114+
localeCompare,
114115
}),
115-
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail],
116+
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail, localeCompare],
116117
);
117118

118119
useEffect(() => {

‎src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type WorkspaceWorkflowsPageProps = WithPolicyProps & PlatformStackScreenProps<Wo
6161
type CurrencyType = TupleToUnion<typeof CONST.DIRECT_REIMBURSEMENT_CURRENCIES>;
6262

6363
function WorkspaceWorkflowsPage({policy, route}: WorkspaceWorkflowsPageProps) {
64-
const {translate} = useLocalize();
64+
const {translate, localeCompare} = useLocalize();
6565
const theme = useTheme();
6666
const styles = useThemeStyles();
6767

@@ -82,8 +82,9 @@ function WorkspaceWorkflowsPage({policy, route}: WorkspaceWorkflowsPageProps) {
8282
employees: policy?.employeeList ?? {},
8383
defaultApprover: policyApproverEmail ?? policy?.owner ?? '',
8484
personalDetails: personalDetails ?? {},
85+
localeCompare,
8586
}),
86-
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail],
87+
[personalDetails, policy?.employeeList, policy?.owner, policyApproverEmail, localeCompare],
8788
);
8889
const {isBetaEnabled} = usePermissions();
8990

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ type WorkspaceWorkflowsApprovalsEditPageProps = WithPolicyAndFullscreenLoadingPr
3333

3434
function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true, route}: WorkspaceWorkflowsApprovalsEditPageProps) {
3535
const styles = useThemeStyles();
36-
const {translate} = useLocalize();
37-
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {canBeMissing: true});
36+
const {translate, localeCompare} = useLocalize();
37+
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {canBeMissing: false});
3838
const [approvalWorkflow] = useOnyx(ONYXKEYS.APPROVAL_WORKFLOW, {canBeMissing: true});
3939
const [initialApprovalWorkflow, setInitialApprovalWorkflow] = useState<ApprovalWorkflow | undefined>();
4040
const [isDeleteModalVisible, setIsDeleteModalVisible] = useState(false);
@@ -83,14 +83,15 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true
8383
defaultApprover,
8484
personalDetails,
8585
firstApprover,
86+
localeCompare,
8687
});
8788

8889
return {
8990
defaultWorkflowMembers: result.availableMembers,
9091
usedApproverEmails: result.usedApproverEmails,
9192
currentApprovalWorkflow: result.approvalWorkflows.find((workflow) => workflow.approvers.at(0)?.email === firstApprover),
9293
};
93-
}, [personalDetails, policy, route.params.firstApproverEmail]);
94+
}, [personalDetails, policy, route.params.firstApproverEmail, localeCompare]);
9495

9596
// eslint-disable-next-line rulesdir/no-negated-variables
9697
const shouldShowNotFoundView = (isEmptyObject(policy) && !isLoadingReportData) || !isPolicyAdmin(policy) || isPendingDeletePolicy(policy) || !currentApprovalWorkflow;

‎tests/unit/WorkflowUtilsTest.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ describe('WorkflowUtils', () => {
201201
const employees: PolicyEmployeeList = {};
202202
const defaultApprover = '1@example.com';
203203

204-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
204+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
205205

206206
expect(approvalWorkflows).toEqual([]);
207207
});
@@ -216,7 +216,7 @@ describe('WorkflowUtils', () => {
216216
};
217217
const defaultApprover = '1@example.com';
218218

219-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
219+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
220220

221221
expect(approvalWorkflows).toEqual([]);
222222
});
@@ -236,7 +236,7 @@ describe('WorkflowUtils', () => {
236236
};
237237
const defaultApprover = '1@example.com';
238238

239-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
239+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
240240

241241
expect(approvalWorkflows).toEqual([buildWorkflow([1, 2], [1], {isDefault: true})]);
242242
});
@@ -266,7 +266,7 @@ describe('WorkflowUtils', () => {
266266
};
267267
const defaultApprover = '1@example.com';
268268

269-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
269+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
270270

271271
expect(approvalWorkflows).toEqual([buildWorkflow([2, 3], [1], {isDefault: true}), buildWorkflow([1, 4], [4])]);
272272
});
@@ -301,7 +301,7 @@ describe('WorkflowUtils', () => {
301301
};
302302
const defaultApprover = '1@example.com';
303303

304-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
304+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
305305

306306
expect(approvalWorkflows).toEqual([buildWorkflow([3, 2], [1], {isDefault: true}), buildWorkflow([5], [3]), buildWorkflow([4, 1], [4])]);
307307
});
@@ -331,7 +331,7 @@ describe('WorkflowUtils', () => {
331331
};
332332
const defaultApprover = '1@example.com';
333333

334-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
334+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
335335

336336
const defaultWorkflow = buildWorkflow([2, 3, 4], [1, 3, 4], {isDefault: true});
337337
let firstApprover = defaultWorkflow.approvers.at(0);
@@ -386,7 +386,7 @@ describe('WorkflowUtils', () => {
386386
};
387387
const defaultApprover = '1@example.com';
388388

389-
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails});
389+
const {approvalWorkflows} = WorkflowUtils.convertPolicyEmployeesToApprovalWorkflows({employees, defaultApprover, personalDetails, localeCompare: TestHelper.localeCompare});
390390

391391
const defaultWorkflow = buildWorkflow([1, 4, 5, 6], [1], {isDefault: true});
392392
const secondWorkflow = buildWorkflow([2, 3], [4, 5, 6]);

0 commit comments

Comments
 (0)