Skip to content

Commit 7745673

Browse files
committed
user autocomplete: Use uniq to deduplicate items that match multiple ways
See zulip#5508 (comment)
1 parent befa9cf commit 7745673

File tree

4 files changed

+18
-31
lines changed

4 files changed

+18
-31
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
"lodash.isequal": "^4.4.0",
5252
"lodash.omit": "^4.5.0",
5353
"lodash.union": "^4.6.0",
54-
"lodash.uniqby": "^4.4.0",
54+
"lodash.uniq": "^4.5.0",
5555
"react": "17.0.2",
5656
"react-intl": "5.24.6",
5757
"react-native": "0.67.4",

src/users/__tests__/userHelpers-test.js

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
filterUserStartWith,
1111
filterUserThatContains,
1212
filterUserMatchesEmail,
13-
getUniqueUsers,
1413
groupUsersByStatus,
1514
} from '../userHelpers';
1615
import * as eg from '../../__tests__/lib/exampleData';
@@ -99,7 +98,7 @@ describe('getAutocompleteSuggestion', () => {
9998
const user5 = eg.makeUser({ full_name: 'match', email: '[email protected]' }); // satisfy full_name starts with condition
10099
const user6 = eg.makeUser({ full_name: 'match', email: '[email protected]' }); // satisfy starts with and email condition
101100
const user7 = eg.makeUser({ full_name: 'Match App Normal', email: '[email protected]' }); // satisfy all conditions
102-
const user8 = eg.makeUser({ full_name: 'match', email: '[email protected]' }); // duplicate
101+
const user8 = user5; // duplicate
103102
const user9 = eg.makeUser({ full_name: 'Laptop', email: '[email protected]' }); // random entry
104103
const user10 = eg.makeUser({ full_name: 'Mobile App', email: '[email protected]' }); // satisfy email condition
105104
const user11 = eg.makeUser({ full_name: 'Normal', email: '[email protected]' }); // satisfy contains in name and matches in email condition
@@ -342,21 +341,3 @@ describe('filterUserMatchesEmail', () => {
342341
expect(filterUserMatchesEmail(users, 'example', selfUser.user_id)).toEqual(expectedUsers);
343342
});
344343
});
345-
346-
describe('getUniqueUsers', () => {
347-
test('returns unique users check by email', () => {
348-
const user1 = eg.makeUser({ email: '[email protected]' });
349-
const user2 = eg.makeUser({ email: '[email protected]' });
350-
const user3 = eg.makeUser({ email: '[email protected]' });
351-
const user4 = eg.makeUser({ email: '[email protected]' });
352-
const user5 = eg.makeUser({ email: '[email protected]' });
353-
const user6 = eg.makeUser({ email: '[email protected]' });
354-
const user7 = eg.makeUser({ email: '[email protected]' });
355-
const user8 = eg.makeUser({ email: '[email protected]' });
356-
357-
const users = deepFreeze([user1, user2, user3, user4, user5, user6, user7, user8]);
358-
359-
const expectedUsers = [user1, user3, user5, user8];
360-
expect(getUniqueUsers(users)).toEqual(expectedUsers);
361-
});
362-
});

src/users/userHelpers.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* @flow strict-local */
22
// $FlowFixMe[untyped-import]
3-
import uniqby from 'lodash.uniqby';
3+
import uniq from 'lodash.uniq';
44
import * as typeahead from '@zulip/shared/js/typeahead';
55

66
import type {
@@ -104,9 +104,11 @@ export const filterUserMatchesEmail = (
104104
user => user.user_id !== ownUserId && user.email.toLowerCase().includes(filter.toLowerCase()),
105105
);
106106

107-
export const getUniqueUsers = (users: $ReadOnlyArray<UserOrBot>): $ReadOnlyArray<UserOrBot> =>
108-
uniqby(users, 'email');
109-
107+
/**
108+
* Get the list of users for a user-autocomplete query.
109+
*
110+
* Callers must ensure that `users` has just one unique object per user ID.
111+
*/
110112
export const getAutocompleteSuggestion = (
111113
users: $ReadOnlyArray<UserOrBot>,
112114
filter: string,
@@ -119,7 +121,16 @@ export const getAutocompleteSuggestion = (
119121
const startWith = filterUserStartWith(users, filter, ownUserId);
120122
const contains = filterUserThatContains(users, filter, ownUserId);
121123
const matchesEmail = filterUserMatchesEmail(users, filter, ownUserId);
122-
const candidates = getUniqueUsers([...startWith, ...contains, ...matchesEmail]);
124+
125+
// Dedupe users that match in multiple ways.
126+
const candidates: $ReadOnlyArray<UserOrBot> =
127+
// Lodash's "unique" is just as good as "unique by user ID" as long as
128+
// there aren't multiple unique objects per user ID. See doc:
129+
// https://lodash.com/docs/4.17.15#uniq
130+
// So we ask the caller to satisfy that for `users`; then when we
131+
// deduplicate, here, we only pass objects that appear in `users`.
132+
uniq([...startWith, ...contains, ...matchesEmail]);
133+
123134
return candidates.filter(user => !mutedUsers.has(user.user_id));
124135
};
125136

yarn.lock

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10541,11 +10541,6 @@ lodash.uniq@^4.5.0:
1054110541
resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773"
1054210542
integrity sha512-xfBaXQd9ryd9dlSDvnvI0lvxfLJlYAZzXomUYzLKtUeOQvOP5piqAWuGtrhWeqaXK9hhoM/iyJc5AV+XfsX3HQ==
1054310543

10544-
lodash.uniqby@^4.4.0:
10545-
version "4.7.0"
10546-
resolved "https://registry.yarnpkg.com/lodash.uniqby/-/lodash.uniqby-4.7.0.tgz#d99c07a669e9e6d24e1362dfe266c67616af1302"
10547-
integrity sha512-e/zcLx6CSbmaEgFHCA7BnoQKyCtKMxnuWrJygbwPs/AIn+IMKl66L8/s+wBUn5LRw2pZx3bUHibiV1b6aTWIww==
10548-
1054910544
"lodash@>=3.5 <5", lodash@^4.17.10, lodash@^4.17.11, lodash@^4.17.12, lodash@^4.17.13, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.17.5, lodash@^4.7.0:
1055010545
version "4.17.21"
1055110546
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"

0 commit comments

Comments
 (0)