Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sort tag consistent with oldDot #38949

Merged
merged 14 commits into from
Mar 28, 2024
Merged

Make sort tag consistent with oldDot #38949

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cf7a891
Implement underscore's sort for tag, ensure consistent with oldDot
wildan-m Mar 23, 2024
3265380
Include localeCompare
wildan-m Mar 23, 2024
8a3ea16
fix sort tags, remove localCompate
wildan-m Mar 25, 2024
4f910e8
Add unit test of sortTags
wildan-m Mar 25, 2024
4578af6
adjust test case
wildan-m Mar 25, 2024
a9763a5
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Mar 25, 2024
a8af8e8
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Mar 25, 2024
448f6b4
run prettier
wildan-m Mar 25, 2024
8e017ba
fix policytag type guard
wildan-m Mar 25, 2024
872e99b
Handle weird ts error in github action
wildan-m Mar 25, 2024
12b1050
fix typecheck error
wildan-m Mar 25, 2024
8f82d7f
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Mar 26, 2024
c54dc0b
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Mar 27, 2024
c870552
change typescript type pattern
wildan-m Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,15 +917,10 @@
* Sorts tags alphabetically by name.
*/
function sortTags(tags: Record<string, Tag> | Tag[]) {
let sortedTags;
const tagsArray = 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(tagsArray, 'name', localeCompare);
}

/**
Expand Down Expand Up @@ -1118,7 +1113,7 @@
const tagSections = [];
const sortedTags = sortTags(tags);
const selectedOptionNames = selectedOptions.map((selectedOption) => selectedOption.name);
const enabledTags = [...selectedOptions, ...sortedTags.filter((tag) => tag.enabled && !selectedOptionNames.includes(tag.name))];

Check failure on line 1116 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'enabled' does not exist on type 'number | Tag | (() => IterableIterator<Tag>) | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 37 more ... | (() => string)'.

Check failure on line 1116 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | Tag | (() => IterableIterator<Tag>) | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 37 more ... | (() => string)'.
const numberOfTags = enabledTags.length;
let indexOffset = 0;

Expand All @@ -1141,14 +1136,14 @@
}

if (searchInputValue) {
const searchTags = enabledTags.filter((tag) => PolicyUtils.getCleanedTagName(tag.name.toLowerCase()).includes(searchInputValue.toLowerCase()));

Check failure on line 1139 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | Tag | Category | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 12 more ... | ((target: number, start: number, end?: number | undefined) => T...'.

tagSections.push({
// "Search" section
title: '',
shouldShow: true,
indexOffset,
data: getTagsOptions(searchTags),

Check failure on line 1146 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Argument of type '(number | Tag | Category | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 12 more ... | ((target: number, start: number, end?: number | undefined) => ...' is not assignable to parameter of type 'Category[]'.
});

return tagSections;
Expand All @@ -1160,7 +1155,7 @@
title: '',
shouldShow: false,
indexOffset,
data: getTagsOptions(enabledTags),

Check failure on line 1158 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Argument of type '(number | Tag | Category | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 12 more ... | ((target: number, start: number, end?: number | undefined) => ...' is not assignable to parameter of type 'Category[]'.
});

return tagSections;
Expand All @@ -1172,7 +1167,7 @@
return !!tagObject?.enabled && !selectedOptionNames.includes(recentlyUsedTag);
})
.map((tag) => ({name: tag, enabled: true}));
const filteredTags = enabledTags.filter((tag) => !selectedOptionNames.includes(tag.name));

Check failure on line 1170 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | Tag | Category | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 12 more ... | ((target: number, start: number, end?: number | undefined) => T...'.

if (selectedOptions.length) {
const selectedTagOptions = selectedOptions.map((option) => ({
Expand Down Expand Up @@ -1211,7 +1206,7 @@
title: Localize.translateLocal('common.all'),
shouldShow: true,
indexOffset,
data: getTagsOptions(filteredTags),

Check failure on line 1209 in src/libs/OptionsListUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Argument of type '(number | Tag | Category | { [x: number]: boolean | undefined; length?: boolean | undefined; toString?: boolean | undefined; toLocaleString?: boolean | undefined; ... 37 more ...; readonly [Symbol.unscopables]?: boolean | undefined; } | ... 12 more ... | ((target: number, start: number, end?: number | undefined) => ...' is not assignable to parameter of type 'Category[]'.
});

return tagSections;
Expand Down Expand Up @@ -2100,6 +2095,7 @@
getEnabledCategoriesCount,
hasEnabledOptions,
sortCategories,
sortTags,
getCategoryOptionTree,
hasEnabledTags,
formatMemberForList,
Expand Down
45 changes: 22 additions & 23 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
View file
Open in desktop
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 @@ -83,30 +84,28 @@
() =>
policyTagLists
.map((policyTagList) =>
Object.values(policyTagList.tags || [])
.sort((a, b) => localeCompare(a.name, b.name))
.map((value) => ({
value: value.name,
text: value.name,
keyForList: value.name,
isSelected: !!selectedTags[value.name],
pendingAction: value.pendingAction,
errors: value.errors ?? undefined,
enabled: value.enabled,
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) => ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not re-use OptionsListUtils.sortTags?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@situchan they have different types; one is Tag and the other one is PolicyTag

value: value.name,

Check failure on line 88 in src/pages/workspace/tags/WorkspaceTagsPage.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | (PolicyTag & OfflineFeedback<keyof PolicyTag>) | { <S extends PolicyTag & OfflineFeedback<keyof PolicyTag>>(predicate: (value: PolicyTag & OfflineFeedback<...>, index: number, array: (PolicyTag & OfflineFeedback<...>)[]) => value is S, thisArg?: any): S[]; (predicate: (value: PolicyTag & OfflineFeedback<......'.
text: value.name,

Check failure on line 89 in src/pages/workspace/tags/WorkspaceTagsPage.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | (PolicyTag & OfflineFeedback<keyof PolicyTag>) | { <S extends PolicyTag & OfflineFeedback<keyof PolicyTag>>(predicate: (value: PolicyTag & OfflineFeedback<...>, index: number, array: (PolicyTag & OfflineFeedback<...>)[]) => value is S, thisArg?: any): S[]; (predicate: (value: PolicyTag & OfflineFeedback<......'.
keyForList: value.name,

Check failure on line 90 in src/pages/workspace/tags/WorkspaceTagsPage.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

Property 'name' does not exist on type 'number | (PolicyTag & OfflineFeedback<keyof PolicyTag>) | { <S extends PolicyTag & OfflineFeedback<keyof PolicyTag>>(predicate: (value: PolicyTag & OfflineFeedback<...>, index: number, array: (PolicyTag & OfflineFeedback<...>)[]) => value is S, thisArg?: any): S[]; (predicate: (value: PolicyTag & OfflineFeedback<......'.
isSelected: !!selectedTags[value.name],
pendingAction: value.pendingAction,
errors: value.errors ?? undefined,
enabled: value.enabled,
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>
),
})),
</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
Open in desktop
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
Loading