Skip to content

Commit

Permalink
Merge pull request #38949 from wildan-m/wildan/fix/33650-implement-so…
Browse files Browse the repository at this point in the history 
…rtBy-tag

Make sort tag consistent with oldDot
  • Loading branch information
@madmax330
madmax330 authored Mar 28, 2024
2 parents 6143bd3 + c870552 commit 469b112
Show file tree
Hide file tree
Showing 3 changed files with 257 additions and 37 deletions.
14 changes: 5 additions & 9 deletions src/libs/OptionsListUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -900,15 +900,10 @@ function sortCategories(categories: Record<string, Category>): Category[] {
* Sorts tags alphabetically by name.
*/
function sortTags(tags: Record<string, PolicyTag | SelectedTagOption> | Array<PolicyTag | SelectedTagOption>) {
let sortedTags;
const sortedTags = Array.isArray(tags) ? tags : Object.values(tags);

if (Array.isArray(tags)) {
sortedTags = tags.sort((a, b) => localeCompare(a.name, b.name));
} else {
sortedTags = Object.values(tags).sort((a, b) => localeCompare(a.name, b.name));
}

return sortedTags;
// Use lodash's sortBy to ensure consistency with oldDot.
return lodashSortBy(sortedTags, 'name', localeCompare);
}

/**
Expand Down Expand Up @@ -1105,7 +1100,7 @@ function getTagListSections(
maxRecentReportsToShow: number,
) {
const tagSections = [];
const sortedTags = sortTags(tags);
const sortedTags = sortTags(tags) as PolicyTag[];
const selectedOptionNames = selectedOptions.map((selectedOption) => selectedOption.name);
const enabledTags = [...selectedOptions, ...sortedTags.filter((tag) => tag.enabled && !selectedOptionNames.includes(tag.name))];
const numberOfTags = enabledTags.length;
Expand Down Expand Up @@ -2089,6 +2084,7 @@ export {
getEnabledCategoriesCount,
hasEnabledOptions,
sortCategories,
sortTags,
getCategoryOptionTree,
hasEnabledTags,
formatMemberForList,
Expand Down
56 changes: 28 additions & 28 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {StackScreenProps} from '@react-navigation/stack';
import lodashSortBy from 'lodash/sortBy';
import React, {useEffect, useMemo, useRef, useState} from 'react';
import {ActivityIndicator, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand Down Expand Up @@ -35,6 +36,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import type DeepValueOf from '@src/types/utils/DeepValueOf';

type PolicyForList = {
Expand Down Expand Up @@ -77,40 +79,38 @@ function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) {
fetchTags();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]);
const tagList = useMemo<PolicyForList[]>(
() =>
policyTagLists
.map((policyTagList) =>
Object.values(policyTagList.tags || [])
.sort((a, b) => localeCompare(a.name, b.name))
.map((value) => {
const isDisabled = value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
return {
value: value.name,
text: value.name,
keyForList: value.name,
isSelected: !!selectedTags[value.name],
pendingAction: value.pendingAction,
errors: value.errors ?? undefined,
enabled: value.enabled,
isDisabled,
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.textSupporting, styles.alignSelfCenter, styles.pl2, styles.label]}>
{value.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
lodashSortBy(Object.values(policyTagList.tags || []), 'name', localeCompare).map((value) => {
const tag = value as OnyxCommon.OnyxValueWithOfflineFeedback<OnyxTypes.PolicyTag>;
const isDisabled = tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
return {
value: tag.name,
text: tag.name,
keyForList: tag.name,
isSelected: !!selectedTags[tag.name],
pendingAction: tag.pendingAction,
errors: tag.errors ?? undefined,
enabled: tag.enabled,
isDisabled,
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.textSupporting, styles.alignSelfCenter, styles.pl2, styles.label]}>
{tag.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
),
};
}),
</View>
),
};
}),
)
.flat(),
[policyTagLists, selectedTags, styles.alignSelfCenter, styles.flexRow, styles.label, styles.p1, styles.pl2, styles.textSupporting, theme.icon, translate],
Expand Down
224 changes: 224 additions & 0 deletions tests/unit/OptionsListUtilsTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,230 @@ describe('OptionsListUtils', () => {
expect(OptionsListUtils.sortCategories(categoriesIncorrectOrdering3)).toStrictEqual(result3);
});

it('sortTags', () => {
const createTagObjects = (names) => _.map(names, (name) => ({name, enabled: true}));

const unorderedTagNames = ['10bc', 'b', '0a', '1', '中国', 'b10', '!', '2', '0', '@', 'a1', 'a', '3', 'b1', '日本', '$', '20', '20a', '#', 'a20', 'c', '10'];
const expectedOrderNames = ['!', '#', '$', '0', '0a', '1', '10', '10bc', '2', '20', '20a', '3', '@', 'a', 'a1', 'a20', 'b', 'b1', 'b10', 'c', '中国', '日本'];
const unorderedTags = createTagObjects(unorderedTagNames);
const expectedOrder = createTagObjects(expectedOrderNames);
expect(OptionsListUtils.sortTags(unorderedTags)).toStrictEqual(expectedOrder);

const unorderedTagNames2 = ['0', 'a1', '1', 'b1', '3', '10', 'b10', 'a', '2', 'c', '20', 'a20', 'b'];
const expectedOrderNames2 = ['0', '1', '10', '2', '20', '3', 'a', 'a1', 'a20', 'b', 'b1', 'b10', 'c'];
const unorderedTags2 = createTagObjects(unorderedTagNames2);
const expectedOrder2 = createTagObjects(expectedOrderNames2);
expect(OptionsListUtils.sortTags(unorderedTags2)).toStrictEqual(expectedOrder2);

const unorderedTagNames3 = [
'61',
'39',
'97',
'93',
'77',
'71',
'22',
'27',
'30',
'64',
'91',
'24',
'33',
'60',
'21',
'85',
'59',
'76',
'42',
'67',
'13',
'96',
'84',
'44',
'68',
'31',
'62',
'87',
'50',
'4',
'100',
'12',
'28',
'49',
'53',
'5',
'45',
'14',
'55',
'78',
'11',
'35',
'75',
'18',
'9',
'80',
'54',
'2',
'34',
'48',
'81',
'6',
'73',
'15',
'98',
'25',
'8',
'99',
'17',
'90',
'47',
'1',
'10',
'38',
'66',
'57',
'23',
'86',
'29',
'3',
'65',
'74',
'19',
'56',
'63',
'20',
'7',
'32',
'46',
'70',
'26',
'16',
'83',
'37',
'58',
'43',
'36',
'69',
'79',
'72',
'41',
'94',
'95',
'82',
'51',
'52',
'89',
'88',
'40',
'92',
];
const expectedOrderNames3 = [
'1',
'10',
'100',
'11',
'12',
'13',
'14',
'15',
'16',
'17',
'18',
'19',
'2',
'20',
'21',
'22',
'23',
'24',
'25',
'26',
'27',
'28',
'29',
'3',
'30',
'31',
'32',
'33',
'34',
'35',
'36',
'37',
'38',
'39',
'4',
'40',
'41',
'42',
'43',
'44',
'45',
'46',
'47',
'48',
'49',
'5',
'50',
'51',
'52',
'53',
'54',
'55',
'56',
'57',
'58',
'59',
'6',
'60',
'61',
'62',
'63',
'64',
'65',
'66',
'67',
'68',
'69',
'7',
'70',
'71',
'72',
'73',
'74',
'75',
'76',
'77',
'78',
'79',
'8',
'80',
'81',
'82',
'83',
'84',
'85',
'86',
'87',
'88',
'89',
'9',
'90',
'91',
'92',
'93',
'94',
'95',
'96',
'97',
'98',
'99',
];
const unorderedTags3 = createTagObjects(unorderedTagNames3);
const expectedOrder3 = createTagObjects(expectedOrderNames3);
expect(OptionsListUtils.sortTags(unorderedTags3)).toStrictEqual(expectedOrder3);
});

it('getFilteredOptions() for taxRate', () => {
const search = 'rate';
const emptySearch = '';
Expand Down

0 comments on commit 469b112

Please sign in to comment.