Skip to content

Commit e895e58

Browse files
shubham1206agrashubham1206agra
committed
Refactored localeCompare in WorkflowUtils
1 parent 8b4ae17 commit e895e58

7 files changed

Lines changed: 42 additions & 38 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: [],
@@ -69,46 +69,35 @@ function calculateApprovers({employees, firstEmail, personalDetailsByEmail}: Get
6969
}
7070

7171
type PolicyConversionParams = {
72-
/**
73-
* List of employees in the policy
74-
*/
72+
/** List of employees in the policy */
7573
employees: PolicyEmployeeList;
7674

77-
/**
78-
* Personal details of the employees
79-
*/
75+
/** Personal details of the employees */
8076
personalDetails: PersonalDetailsList;
8177

82-
/**
83-
* Email of the default approver for the policy
84-
*/
78+
/** Email of the default approver for the policy */
8579
defaultApprover: string;
8680

87-
/**
88-
* Email of the first approver in current edited workflow
89-
*/
81+
/** Email of the first approver in current edited workflow */
9082
firstApprover?: string;
83+
84+
/** Locale comparison function */
85+
localeCompare: LocaleContextProps['localeCompare'];
9186
};
9287

9388
type PolicyConversionResult = {
94-
/**
95-
* List of approval workflows
96-
*/
89+
/** List of approval workflows */
9790
approvalWorkflows: ApprovalWorkflow[];
9891

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

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

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

114103
// 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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type WorkspaceWorkflowsApprovalsEditPageProps = WithPolicyAndFullscreenLoadingPr
3333

3434
function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true, route}: WorkspaceWorkflowsApprovalsEditPageProps) {
3535
const styles = useThemeStyles();
36-
const {translate} = useLocalize();
36+
const {translate, localeCompare} = useLocalize();
3737
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
3838
const [approvalWorkflow] = useOnyx(ONYXKEYS.APPROVAL_WORKFLOW);
3939
const [initialApprovalWorkflow, setInitialApprovalWorkflow] = useState<ApprovalWorkflow | undefined>();
@@ -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]);

‎tests/utils/TestHelper.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,16 @@ async function navigateToSidebarOption(index: number): Promise<void> {
346346
await waitForBatchedUpdatesWithAct();
347347
}
348348

349+
/**
350+
* @private
351+
* This is a custom collator only for testing purposes.
352+
*/
353+
const customCollator = new Intl.Collator('en', {usage: 'sort', sensitivity: 'variant', numeric: true, caseFirst: 'upper'});
354+
355+
function localeCompare(a: string, b: string): number {
356+
return customCollator.compare(a, b);
357+
}
358+
349359
export type {MockFetch, FormData};
350360
export {
351361
assertFormDataMatchesObject,
@@ -363,4 +373,5 @@ export {
363373
navigateToSidebarOption,
364374
getOnyxData,
365375
getNavigateToChatHintRegex,
376+
localeCompare,
366377
};

0 commit comments

Comments
 (0)