diff --git a/lib/model/user.dart b/lib/model/user.dart index aa4666c359..7d195e3197 100644 --- a/lib/model/user.dart +++ b/lib/model/user.dart @@ -35,7 +35,10 @@ mixin UserStore on PerAccountStoreBase, RealmStore { /// Consider using [userDisplayName]. User? getUser(int userId); - /// All known users in the realm. + /// All known users in the realm, including deactivated users. + /// + /// Before presenting these users in the UI, consider whether to exclude + /// users who are deactivated (see [User.isActive]) or muted ([isUserMuted]). /// /// This may have a large number of elements, like tens of thousands. /// Consider [getUser] or other alternatives to iterating through this. diff --git a/lib/widgets/new_dm_sheet.dart b/lib/widgets/new_dm_sheet.dart index 682adea91c..93fbf5c354 100644 --- a/lib/widgets/new_dm_sheet.dart +++ b/lib/widgets/new_dm_sheet.dart @@ -1,4 +1,3 @@ -import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import '../api/model/model.dart'; import '../generated/l10n/zulip_localizations.dart'; @@ -69,9 +68,9 @@ class _NewDmPickerState extends State with PerAccountStoreAwareStat } void _initSortedUsers(PerAccountStore store) { - final sansMuted = store.allUsers - .whereNot((user) => store.isUserMuted(user.userId)); - sortedUsers = List.from(sansMuted) + final users = store.allUsers + .where((user) => user.isActive && !store.isUserMuted(user.userId)); + sortedUsers = List.from(users) ..sort((a, b) => MentionAutocompleteView.compareByDms(a, b, store: store)); _updateFilteredUsers(store); } diff --git a/test/widgets/new_dm_sheet_test.dart b/test/widgets/new_dm_sheet_test.dart index ae77e9643f..a95d42dc1a 100644 --- a/test/widgets/new_dm_sheet_test.dart +++ b/test/widgets/new_dm_sheet_test.dart @@ -116,24 +116,19 @@ void main() { }); group('user filtering', () { - final mutedUser = eg.user(fullName: 'Someone Muted'); final testUsers = [ eg.user(fullName: 'Alice Anderson'), eg.user(fullName: 'Bob Brown'), eg.user(fullName: 'Charlie Carter'), - mutedUser, ]; - testWidgets('shows all non-muted users initially', (tester) async { - await setupSheet(tester, users: testUsers, mutedUserIds: [mutedUser.userId]); + testWidgets('shows full list initially', (tester) async { + await setupSheet(tester, users: testUsers); check(findText(includePlaceholders: false, 'Alice Anderson')).findsOne(); check(findText(includePlaceholders: false, 'Bob Brown')).findsOne(); check(findText(includePlaceholders: false, 'Charlie Carter')).findsOne(); - check(find.byIcon(ZulipIcons.check_circle_unchecked)).findsExactly(3); check(find.byIcon(ZulipIcons.check_circle_checked)).findsNothing(); - check(findText(includePlaceholders: false, 'Someone Muted')).findsNothing(); - check(findText(includePlaceholders: false, 'Muted user')).findsNothing(); }); testWidgets('shows filtered users based on search', (tester) async { @@ -145,6 +140,43 @@ void main() { check(findText(includePlaceholders: false, 'Bob Brown')).findsNothing(); }); + testWidgets('deactivated users excluded', (tester) async { + // Omit a deactivated user both before there's a query… + final deactivatedUser = eg.user(fullName: 'Impostor Charlie', isActive: false); + await setupSheet(tester, users: [...testUsers, deactivatedUser]); + check(findText(includePlaceholders: false, 'Impostor Charlie')).findsNothing(); + check(findText(includePlaceholders: false, 'Charlie Carter')).findsOne(); + check(find.byIcon(ZulipIcons.check_circle_unchecked)).findsExactly(3); + + // … and after a query that would match their name. + await tester.enterText(find.byType(TextField), 'Charlie'); + await tester.pump(); + check(findText(includePlaceholders: false, 'Impostor Charlie')).findsNothing(); + check(findText(includePlaceholders: false, 'Charlie Carter')).findsOne(); + check(find.byIcon(ZulipIcons.check_circle_unchecked)).findsExactly(1); + }); + + testWidgets('muted users excluded', (tester) async { + // Omit muted users both before there's a query… + final mutedUser = eg.user(fullName: 'Someone Muted'); + await setupSheet(tester, + users: [...testUsers, mutedUser], mutedUserIds: [mutedUser.userId]); + check(findText(includePlaceholders: false, 'Someone Muted')).findsNothing(); + check(findText(includePlaceholders: false, 'Muted user')).findsNothing(); + check(findText(includePlaceholders: false, 'Alice Anderson')).findsOne(); + check(find.byIcon(ZulipIcons.check_circle_unchecked)).findsExactly(3); + + // … and after a query. One which matches both the user's actual name and + // the replacement text "Muted user", for good measure. + await tester.enterText(find.byType(TextField), 'e'); + await tester.pump(); + check(findText(includePlaceholders: false, 'Someone Muted')).findsNothing(); + check(findText(includePlaceholders: false, 'Muted user')).findsNothing(); + check(findText(includePlaceholders: false, 'Alice Anderson')).findsOne(); + check(findText(includePlaceholders: false, 'Charlie Carter')).findsOne(); + check(find.byIcon(ZulipIcons.check_circle_unchecked)).findsExactly(2); + }); + // TODO test sorting by recent-DMs // TODO test that scroll position resets on query change