diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index dffe44d6fe..cf3b484564 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -449,7 +449,7 @@ class MentionAutocompleteView extends AutocompleteView= 247; // TODO(server-9) + final isChannelWildcardAvailable = store.zulipFeatureLevel >= 247; // TODO(server-9) if (isChannelWildcardAvailable && tryOption(WildcardMentionOption.channel)) break all; if (tryOption(WildcardMentionOption.stream)) break all; } } - final isTopicWildcardAvailable = store.account.zulipFeatureLevel >= 224; // TODO(server-8) + final isTopicWildcardAvailable = store.zulipFeatureLevel >= 224; // TODO(server-8) if (isComposingChannelMessage && isTopicWildcardAvailable) { tryOption(WildcardMentionOption.topic); } diff --git a/lib/model/compose.dart b/lib/model/compose.dart index 54c7f6ce00..2aa2ed9f44 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -5,6 +5,7 @@ import '../generated/l10n/zulip_localizations.dart'; import 'internal_link.dart'; import 'narrow.dart'; import 'store.dart'; +import 'user.dart'; /// The available user wildcard mention options, /// known to the server as [canonicalString]. @@ -127,11 +128,12 @@ String wrapWithBacktickFence({required String content, String? infoString}) { /// An @-mention of an individual user, like @**Chris Bobbe|13313**. /// /// To omit the user ID part ("|13313") whenever the name part is unambiguous, -/// pass a Map of all users we know about. This means accepting a linear scan +/// pass the full UserStore. This means accepting a linear scan /// through all users; avoid it in performance-sensitive codepaths. -String userMention(User user, {bool silent = false, Map? users}) { +String userMention(User user, {bool silent = false, UserStore? users}) { bool includeUserId = users == null - || users.values.where((u) => u.fullName == user.fullName).take(2).length == 2; + || users.allUsers.where((u) => u.fullName == user.fullName) + .take(2).length == 2; return '@${silent ? '_' : ''}**${user.fullName}${includeUserId ? '|${user.userId}' : ''}**'; } @@ -140,8 +142,8 @@ String userMention(User user, {bool silent = false, Map? users}) { String wildcardMention(WildcardMentionOption wildcardOption, { required PerAccountStore store, }) { - final isChannelWildcardAvailable = store.account.zulipFeatureLevel >= 247; // TODO(server-9) - final isTopicWildcardAvailable = store.account.zulipFeatureLevel >= 224; // TODO(server-8) + final isChannelWildcardAvailable = store.zulipFeatureLevel >= 247; // TODO(server-9) + final isTopicWildcardAvailable = store.zulipFeatureLevel >= 224; // TODO(server-8) String name = wildcardOption.canonicalString; switch (wildcardOption) { @@ -188,8 +190,8 @@ String quoteAndReplyPlaceholder( PerAccountStore store, { required Message message, }) { - final sender = store.users[message.senderId]; - assert(sender != null); + final sender = store.getUser(message.senderId); + assert(sender != null); // TODO(#716): should use `store.senderDisplayName` final url = narrowLink(store, SendableNarrow.ofMessage(message, selfUserId: store.selfUserId), nearMessageId: message.id); @@ -210,8 +212,8 @@ String quoteAndReply(PerAccountStore store, { required Message message, required String rawContent, }) { - final sender = store.users[message.senderId]; - assert(sender != null); + final sender = store.getUser(message.senderId); + assert(sender != null); // TODO(#716): should use `store.senderDisplayName` final url = narrowLink(store, SendableNarrow.ofMessage(message, selfUserId: store.selfUserId), nearMessageId: message.id); diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart index 1290259033..749f60698c 100644 --- a/lib/model/internal_link.dart +++ b/lib/model/internal_link.dart @@ -60,7 +60,7 @@ String? decodeHashComponent(String str) { Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { // TODO(server-7) final apiNarrow = resolveApiNarrowForServer( - narrow.apiEncode(), store.connection.zulipFeatureLevel!); + narrow.apiEncode(), store.zulipFeatureLevel); final fragment = StringBuffer('narrow'); for (ApiNarrowElement element in apiNarrow) { fragment.write('/'); diff --git a/lib/model/store.dart b/lib/model/store.dart index 611494cc8f..94f6c56084 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -31,6 +31,7 @@ import 'recent_senders.dart'; import 'channel.dart'; import 'typing_status.dart'; import 'unreads.dart'; +import 'user.dart'; export 'package:drift/drift.dart' show Value; export 'database.dart' show Account, AccountsCompanion, AccountAlreadyExistsException; @@ -266,7 +267,7 @@ class AccountNotFoundException implements Exception {} /// This class does not attempt to poll an event queue /// to keep the data up to date. For that behavior, see /// [UpdateMachine]. -class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, MessageStore { +class PerAccountStore extends ChangeNotifier with EmojiStore, UserStore, ChannelStore, MessageStore { /// Construct a store for the user's data, starting from the given snapshot. /// /// The global store must already have been updated with @@ -307,7 +308,6 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess emoji: EmojiStoreImpl( realmUrl: realmUrl, allRealmEmoji: initialSnapshot.realmEmoji), accountId: accountId, - selfUserId: account.userId, userSettings: initialSnapshot.userSettings, typingNotifier: TypingNotifier( connection: connection, @@ -316,11 +316,9 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess typingStartedWaitPeriod: Duration( milliseconds: initialSnapshot.serverTypingStartedWaitPeriodMilliseconds), ), - users: Map.fromEntries( - initialSnapshot.realmUsers - .followedBy(initialSnapshot.realmNonActiveUsers) - .followedBy(initialSnapshot.crossRealmBots) - .map((user) => MapEntry(user.userId, user))), + users: UserStoreImpl( + selfUserId: account.userId, + initialSnapshot: initialSnapshot), typingStatus: TypingStatus( selfUserId: account.userId, typingStartedExpiryPeriod: Duration(milliseconds: initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds), @@ -351,22 +349,21 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess required this.emailAddressVisibility, required EmojiStoreImpl emoji, required this.accountId, - required this.selfUserId, required this.userSettings, required this.typingNotifier, - required this.users, + required UserStoreImpl users, required this.typingStatus, required ChannelStoreImpl channels, required MessageStoreImpl messages, required this.unreads, required this.recentDmConversationsView, required this.recentSenders, - }) : assert(selfUserId == globalStore.getAccount(accountId)!.userId), - assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl), + }) : assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl), assert(realmUrl == connection.realmUrl), assert(emoji.realmUrl == realmUrl), _globalStore = globalStore, _emoji = emoji, + _users = users, _channels = channels, _messages = messages; @@ -407,6 +404,10 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess /// This returns null if [reference] fails to parse as a URL. Uri? tryResolveUrl(String reference) => _tryResolveUrl(realmUrl, reference); + /// Always equal to `connection.zulipFeatureLevel` + /// and `account.zulipFeatureLevel`. + int get zulipFeatureLevel => connection.zulipFeatureLevel!; + String get zulipVersion => account.zulipVersion; final RealmWildcardMentionPolicy realmWildcardMentionPolicy; // TODO(#668): update this realm setting final bool realmMandatoryTopics; // TODO(#668): update this realm setting @@ -455,9 +456,6 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess /// Will throw if called after [dispose] has been called. Account get account => _globalStore.getAccount(accountId)!; - /// Always equal to `account.userId`. - final int selfUserId; - final UserSettings? userSettings; // TODO(server-5) final TypingNotifier typingNotifier; @@ -465,7 +463,16 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess //////////////////////////////// // Users and data about them. - final Map users; + @override + int get selfUserId => _users.selfUserId; + + @override + User? getUser(int userId) => _users.getUser(userId); + + @override + Iterable get allUsers => _users.allUsers; + + final UserStoreImpl _users; final TypingStatus typingStatus; @@ -634,44 +641,18 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess case RealmUserAddEvent(): assert(debugLog("server event: realm_user/add")); - users[event.person.userId] = event.person; + _users.handleRealmUserEvent(event); notifyListeners(); case RealmUserRemoveEvent(): assert(debugLog("server event: realm_user/remove")); - users.remove(event.userId); + _users.handleRealmUserEvent(event); autocompleteViewManager.handleRealmUserRemoveEvent(event); notifyListeners(); case RealmUserUpdateEvent(): assert(debugLog("server event: realm_user/update")); - final user = users[event.userId]; - if (user == null) { - return; // TODO log - } - if (event.fullName != null) user.fullName = event.fullName!; - if (event.avatarUrl != null) user.avatarUrl = event.avatarUrl!; - if (event.avatarVersion != null) user.avatarVersion = event.avatarVersion!; - if (event.timezone != null) user.timezone = event.timezone!; - if (event.botOwnerId != null) user.botOwnerId = event.botOwnerId!; - if (event.role != null) user.role = event.role!; - if (event.isBillingAdmin != null) user.isBillingAdmin = event.isBillingAdmin!; - if (event.deliveryEmail != null) user.deliveryEmail = event.deliveryEmail!.value; - if (event.newEmail != null) user.email = event.newEmail!; - if (event.isActive != null) user.isActive = event.isActive!; - if (event.customProfileField != null) { - final profileData = (user.profileData ??= {}); - final update = event.customProfileField!; - if (update.value != null) { - profileData[update.id] = ProfileFieldUserData(value: update.value!, renderedValue: update.renderedValue); - } else { - profileData.remove(update.id); - } - if (profileData.isEmpty) { - // null is equivalent to `{}` for efficiency; see [User._readProfileData]. - user.profileData = null; - } - } + _users.handleRealmUserEvent(event); autocompleteViewManager.handleRealmUserUpdateEvent(event); notifyListeners(); diff --git a/lib/model/user.dart b/lib/model/user.dart new file mode 100644 index 0000000000..1577e21048 --- /dev/null +++ b/lib/model/user.dart @@ -0,0 +1,142 @@ +import '../api/model/events.dart'; +import '../api/model/initial_snapshot.dart'; +import '../api/model/model.dart'; +import 'localizations.dart'; + +/// The portion of [PerAccountStore] describing the users in the realm. +mixin UserStore { + /// The user ID of the "self-user", + /// i.e. the account the person using this app is logged into. + /// + /// This always equals the [Account.userId] on [PerAccountStore.account]. + /// + /// For the corresponding [User] object, see [selfUser]. + int get selfUserId; + + /// The user with the given ID, if that user is known. + /// + /// There may be other users that are perfectly real but are + /// not known to the app, for multiple reasons: + /// + /// * The self-user may not have permission to see all the users in the + /// realm, for example because the self-user is a guest. + /// + /// * Because of the fetch/event race, any data that the client fetched + /// outside of the event system may reflect an earlier or later time + /// than this data, which is maintained by the event system. + /// This includes messages fetched for a message list, and notifications. + /// Those may therefore refer to users for which we have yet to see the + /// [RealmUserAddEvent], or have already handled a [RealmUserRemoveEvent]. + /// + /// Code that looks up a user here should therefore always handle + /// the possibility that the user is not found (except + /// where there is a specific reason to know the user should be found). + /// Consider using [userDisplayName]. + User? getUser(int userId); + + /// All known users in the realm. + /// + /// This may have a large number of elements, like tens of thousands. + /// Consider [getUser] or other alternatives to iterating through this. + /// + /// There may be perfectly real users which are not known + /// and so are not found here. For details, see [getUser]. + Iterable get allUsers; + + /// The [User] object for the "self-user", + /// i.e. the account the person using this app is logged into. + /// + /// When only the user ID is needed, see [selfUserId]. + User get selfUser => getUser(selfUserId)!; + + /// The name to show the given user as in the UI, even for unknown users. + /// + /// This is the user's [User.fullName] if the user is known, + /// and otherwise a translation of "(unknown user)". + /// + /// When a [Message] is available which the user sent, + /// use [senderDisplayName] instead for a better-informed fallback. + String userDisplayName(int userId) { + return getUser(userId)?.fullName + ?? GlobalLocalizations.zulipLocalizations.unknownUserName; + } + + /// The name to show for the given message's sender in the UI. + /// + /// If the user is known (see [getUser]), this is their current [User.fullName]. + /// If unknown, this uses the fallback value conveniently provided on the + /// [Message] object itself, namely [Message.senderFullName]. + /// + /// For a user who isn't the sender of some known message, + /// see [userDisplayName]. + String senderDisplayName(Message message) { + return getUser(message.senderId)?.fullName + ?? message.senderFullName; + } +} + +/// The implementation of [UserStore] that does the work. +/// +/// Generally the only code that should need this class is [PerAccountStore] +/// itself. Other code accesses this functionality through [PerAccountStore], +/// or through the mixin [UserStore] which describes its interface. +class UserStoreImpl with UserStore { + UserStoreImpl({ + required this.selfUserId, + required InitialSnapshot initialSnapshot, + }) : _users = Map.fromEntries( + initialSnapshot.realmUsers + .followedBy(initialSnapshot.realmNonActiveUsers) + .followedBy(initialSnapshot.crossRealmBots) + .map((user) => MapEntry(user.userId, user))); + + @override + final int selfUserId; + + final Map _users; + + @override + User? getUser(int userId) => _users[userId]; + + @override + Iterable get allUsers => _users.values; + + void handleRealmUserEvent(RealmUserEvent event) { + switch (event) { + case RealmUserAddEvent(): + _users[event.person.userId] = event.person; + + case RealmUserRemoveEvent(): + _users.remove(event.userId); + + case RealmUserUpdateEvent(): + final user = _users[event.userId]; + if (user == null) { + return; // TODO log + } + if (event.fullName != null) user.fullName = event.fullName!; + if (event.avatarUrl != null) user.avatarUrl = event.avatarUrl!; + if (event.avatarVersion != null) user.avatarVersion = event.avatarVersion!; + if (event.timezone != null) user.timezone = event.timezone!; + if (event.botOwnerId != null) user.botOwnerId = event.botOwnerId!; + if (event.role != null) user.role = event.role!; + if (event.isBillingAdmin != null) user.isBillingAdmin = event.isBillingAdmin!; + if (event.deliveryEmail != null) user.deliveryEmail = event.deliveryEmail!.value; + if (event.newEmail != null) user.email = event.newEmail!; + if (event.isActive != null) user.isActive = event.isActive!; + if (event.customProfileField != null) { + final profileData = (user.profileData ??= {}); + final update = event.customProfileField!; + if (update.value != null) { + profileData[update.id] = ProfileFieldUserData(value: update.value!, renderedValue: update.renderedValue); + } else { + profileData.remove(update.id); + } + if (profileData.isEmpty) { + // null is equivalent to `{}` for efficiency; see [User._readProfileData]. + user.profileData = null; + } + } + } + } +} diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index e943d7eb36..aa81461421 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -182,9 +182,9 @@ void showTopicActionSheet(BuildContext context, { final optionButtons = []; // TODO(server-7): simplify this condition away - final supportsUnmutingTopics = store.connection.zulipFeatureLevel! >= 170; + final supportsUnmutingTopics = store.zulipFeatureLevel >= 170; // TODO(server-8): simplify this condition away - final supportsFollowingTopics = store.connection.zulipFeatureLevel! >= 219; + final supportsFollowingTopics = store.zulipFeatureLevel >= 219; final visibilityOptions = []; final visibilityPolicy = store.topicVisibilityPolicy(channelId, topic); @@ -477,7 +477,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes final isComposeBoxOffered = messageListPage.composeBoxController != null; final isMessageRead = message.flags.contains(MessageFlag.read); - final markAsUnreadSupported = store.connection.zulipFeatureLevel! >= 155; // TODO(server-6) + final markAsUnreadSupported = store.zulipFeatureLevel >= 155; // TODO(server-6) final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead; final optionButtons = [ diff --git a/lib/widgets/actions.dart b/lib/widgets/actions.dart index 814b56d009..d49ce690d0 100644 --- a/lib/widgets/actions.dart +++ b/lib/widgets/actions.dart @@ -24,9 +24,8 @@ abstract final class ZulipAction { /// for details on the UI feedback, see there. static Future markNarrowAsRead(BuildContext context, Narrow narrow) async { final store = PerAccountStoreWidget.of(context); - final connection = store.connection; final zulipLocalizations = ZulipLocalizations.of(context); - final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6) + final useLegacy = store.zulipFeatureLevel < 155; // TODO(server-6) if (useLegacy) { try { await _legacyMarkNarrowAsRead(context, narrow); @@ -78,8 +77,7 @@ abstract final class ZulipAction { Message message, Narrow narrow, ) async { - final connection = PerAccountStoreWidget.of(context).connection; - assert(connection.zulipFeatureLevel! >= 155); // TODO(server-6) + assert(PerAccountStoreWidget.of(context).zulipFeatureLevel >= 155); // TODO(server-6) final zulipLocalizations = ZulipLocalizations.of(context); await updateMessageFlagsStartingFromAnchor( context: context, diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart index 40d1f2bf16..5da053c329 100644 --- a/lib/widgets/autocomplete.dart +++ b/lib/widgets/autocomplete.dart @@ -200,9 +200,10 @@ class ComposeAutocomplete extends AutocompleteField= 247; // TODO(server-9) + final isChannelWildcardAvailable = store.zulipFeatureLevel >= 247; // TODO(server-9) final localizations = ZulipLocalizations.of(context); final description = switch (wildcardOption) { WildcardMentionOption.all || WildcardMentionOption.everyone => isDmNarrow @@ -267,8 +268,9 @@ class _MentionAutocompleteItem extends StatelessWidget { Widget label; switch (option) { case UserMentionAutocompleteResult(:var userId): + final user = store.getUser(userId)!; // must exist because UserMentionAutocompleteResult avatar = Avatar(userId: userId, size: 32, borderRadius: 3); // web uses 21px - label = Text(store.users[userId]!.fullName); + label = Text(user.fullName); case WildcardMentionAutocompleteResult(:var wildcardOption): avatar = const Icon(ZulipIcons.three_person, size: 29); // web uses 19px label = wildcardLabel(wildcardOption, context: context, store: store); diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 0a060a6bcf..902d57892d 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -664,7 +664,7 @@ class _FixedDestinationContentInput extends StatelessWidget { case DmNarrow(otherRecipientIds: [final otherUserId]): final store = PerAccountStoreWidget.of(context); - final fullName = store.users[otherUserId]?.fullName; + final fullName = store.getUser(otherUserId)?.fullName; if (fullName == null) return zulipLocalizations.composeBoxGenericContentHint; return zulipLocalizations.composeBoxDmContentHint(fullName); @@ -1419,23 +1419,24 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM Widget? _errorBanner(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final selfUser = store.users[store.selfUserId]!; switch (widget.narrow) { case ChannelNarrow(:final streamId): case TopicNarrow(:final streamId): final channel = store.streams[streamId]; if (channel == null || !store.hasPostingPermission(inChannel: channel, - user: selfUser, byDate: DateTime.now())) { + user: store.selfUser, byDate: DateTime.now())) { return _ErrorBanner(label: ZulipLocalizations.of(context).errorBannerCannotPostInChannelLabel); } + case DmNarrow(:final otherRecipientIds): final hasDeactivatedUser = otherRecipientIds.any((id) => - !(store.users[id]?.isActive ?? true)); + !(store.getUser(id)?.isActive ?? true)); if (hasDeactivatedUser) { return _ErrorBanner(label: ZulipLocalizations.of(context).errorBannerDeactivatedDmLabel); } + case CombinedFeedNarrow(): case MentionsNarrow(): case StarredMessagesNarrow(): diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 8799e8d192..728ee880f6 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -1509,7 +1509,7 @@ class AvatarImage extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final user = store.users[userId]; + final user = store.getUser(userId); if (user == null) { // TODO(log) return const SizedBox.shrink(); diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index de3e2baa26..c418337e62 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -162,7 +162,7 @@ class ReactionChip extends StatelessWidget { ? userIds.map((id) { return id == store.selfUserId ? zulipLocalizations.reactedEmojiSelfUser - : store.users[id]?.fullName ?? zulipLocalizations.unknownUserName; + : store.userDisplayName(id); }).join(', ') : userIds.length.toString(); diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 799f763f1c..480437c912 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -382,22 +382,16 @@ class _DmItem extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final selfUser = store.users[store.selfUserId]!; - - final zulipLocalizations = ZulipLocalizations.of(context); final designVariables = DesignVariables.of(context); final title = switch (narrow.otherRecipientIds) { // TODO dedupe with [RecentDmConversationsItem] - [] => selfUser.fullName, - [var otherUserId] => - store.users[otherUserId]?.fullName ?? zulipLocalizations.unknownUserName, + [] => store.selfUser.fullName, + [var otherUserId] => store.userDisplayName(otherUserId), // TODO(i18n): List formatting, like you can do in JavaScript: // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya', 'Shu']) // // 'Chris、Greg、Alya、Shu' - _ => narrow.otherRecipientIds.map( - (id) => store.users[id]?.fullName ?? zulipLocalizations.unknownUserName - ).join(', '), + _ => narrow.otherRecipientIds.map(store.userDisplayName).join(', '), }; return Material( diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 65d013a4b6..dd2aca5b82 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -178,7 +178,7 @@ class _LightboxPageLayoutState extends State<_LightboxPageLayout> { child: RichText( text: TextSpan(children: [ TextSpan( - text: '${widget.message.senderFullName}\n', + text: '${widget.message.senderFullName}\n', // TODO(#716): use `store.senderDisplayName` // Restate default style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor)), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 127bd65c61..c4ca22bce3 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -431,8 +431,7 @@ class MessageListAppBarTitle extends StatelessWidget { if (otherRecipientIds.isEmpty) { return Text(zulipLocalizations.dmsWithYourselfPageTitle); } else { - final names = otherRecipientIds.map( - (id) => store.users[id]?.fullName ?? zulipLocalizations.unknownUserName); + final names = otherRecipientIds.map(store.userDisplayName); // TODO show avatars return Text( zulipLocalizations.dmsWithOthersPageTitle(names.join(', '))); @@ -774,10 +773,10 @@ class _TypingStatusWidgetState extends State with PerAccount if (typistIds.isEmpty) return const SizedBox(); final text = switch (typistIds.length) { 1 => localizations.onePersonTyping( - store.users[typistIds.first]?.fullName ?? localizations.unknownUserName), + store.userDisplayName(typistIds.first)), 2 => localizations.twoPeopleTyping( - store.users[typistIds.first]?.fullName ?? localizations.unknownUserName, - store.users[typistIds.last]?.fullName ?? localizations.unknownUserName), + store.userDisplayName(typistIds.first), + store.userDisplayName(typistIds.last)), _ => localizations.manyPeopleTyping, }; @@ -1179,7 +1178,7 @@ class DmRecipientHeader extends StatelessWidget { if (message.allRecipientIds.length > 1) { title = zulipLocalizations.messageListGroupYouAndOthers(message.allRecipientIds .where((id) => id != store.selfUserId) - .map((id) => store.users[id]?.fullName ?? zulipLocalizations.unknownUserName) + .map(store.userDisplayName) .sorted() .join(", ")); } else { @@ -1331,7 +1330,7 @@ class MessageWithPossibleSender extends StatelessWidget { final designVariables = DesignVariables.of(context); final message = item.message; - final sender = store.users[message.senderId]; + final sender = store.getUser(message.senderId); Widget? senderRow; if (item.showSender) { @@ -1353,7 +1352,7 @@ class MessageWithPossibleSender extends StatelessWidget { userId: message.senderId), const SizedBox(width: 8), Flexible( - child: Text(message.senderFullName, // TODO get from user data + child: Text(message.senderFullName, // TODO(#716): use `store.senderDisplayName` style: TextStyle( fontSize: 18, height: (22 / 18), diff --git a/lib/widgets/poll.dart b/lib/widgets/poll.dart index dc340d08b8..b851c55525 100644 --- a/lib/widgets/poll.dart +++ b/lib/widgets/poll.dart @@ -80,8 +80,7 @@ class _PollWidgetState extends State { // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya', 'Zixuan']) // // 'Chris、Greg、Alya、Zixuan' final voterNames = option.voters - .map((userId) => - store.users[userId]?.fullName ?? zulipLocalizations.unknownUserName) + .map(store.userDisplayName) .join(', '); return Row( diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index 327910f6c0..00620cb82d 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -40,7 +40,7 @@ class ProfilePage extends StatelessWidget { /// /// Returns null if self-user isn't able to see [user]'s real email address. String? _getDisplayEmailFor(User user, {required PerAccountStore store}) { - if (store.account.zulipFeatureLevel >= 163) { // TODO(server-7) + if (store.zulipFeatureLevel >= 163) { // TODO(server-7) // A non-null value means self-user has access to [user]'s real email, // while a null value means it doesn't have access to the email. // Search for "delivery_email" in https://zulip.com/api/register-queue. @@ -67,7 +67,7 @@ class ProfilePage extends StatelessWidget { Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); final store = PerAccountStoreWidget.of(context); - final user = store.users[userId]; + final user = store.getUser(userId); if (user == null) { return const _ProfileErrorPage(); } @@ -291,9 +291,6 @@ class _UserWidget extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final zulipLocalizations = ZulipLocalizations.of(context); - final user = store.users[userId]; - final fullName = user?.fullName ?? zulipLocalizations.unknownUserName; return InkWell( onTap: () => Navigator.push(context, ProfilePage.buildRoute(context: context, @@ -301,9 +298,12 @@ class _UserWidget extends StatelessWidget { child: Padding( padding: const EdgeInsets.all(8), child: Row(children: [ + // TODO(#196) render active status Avatar(userId: userId, size: 32, borderRadius: 32 / 8), const SizedBox(width: 8), - Expanded(child: Text(fullName, style: _TextStyles.customProfileFieldText)), // TODO(#196) render active status + Expanded( + child: Text(store.userDisplayName(userId), + style: _TextStyles.customProfileFieldText)), ]))); } } diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index ddc32861a3..982dde4f08 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -1,6 +1,5 @@ import 'package:flutter/material.dart'; -import '../generated/l10n/zulip_localizations.dart'; import '../model/narrow.dart'; import '../model/recent_dm_conversations.dart'; import '../model/unreads.dart'; @@ -80,31 +79,26 @@ class RecentDmConversationsItem extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final selfUser = store.users[store.selfUserId]!; - - final zulipLocalizations = ZulipLocalizations.of(context); final designVariables = DesignVariables.of(context); final String title; final Widget avatar; switch (narrow.otherRecipientIds) { // TODO dedupe with DM items in [InboxPage] case []: - title = selfUser.fullName; - avatar = AvatarImage(userId: selfUser.userId, size: _avatarSize); + title = store.selfUser.fullName; + avatar = AvatarImage(userId: store.selfUserId, size: _avatarSize); case [var otherUserId]: // TODO(#296) actually don't show this row if the user is muted? // (should we offer a "spam folder" style summary screen of recent // 1:1 DM conversations from muted users?) - final otherUser = store.users[otherUserId]; - title = otherUser?.fullName ?? zulipLocalizations.unknownUserName; + title = store.userDisplayName(otherUserId); avatar = AvatarImage(userId: otherUserId, size: _avatarSize); default: // TODO(i18n): List formatting, like you can do in JavaScript: // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya']) // // 'Chris、Greg、Alya' - title = narrow.otherRecipientIds.map( - (id) => store.users[id]?.fullName ?? zulipLocalizations.unknownUserName - ).join(', '); + title = narrow.otherRecipientIds.map(store.userDisplayName) + .join(', '); avatar = ColoredBox(color: designVariables.groupDmConversationIconBg, child: Center( child: Icon(color: designVariables.groupDmConversationIcon, diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index da05030493..e916151426 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -369,7 +369,7 @@ void main() { } }); - test('MentionAutocompleteView mutating store.users while in progress does not ' + test('MentionAutocompleteView mutating user store while in progress does not ' 'prevent query from finishing', () async { const narrow = ChannelNarrow(1); final store = eg.store(); diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index 73fa9452d7..bfbc170ca1 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -234,17 +234,17 @@ hello test('`users` passed; has two users with same fullName', () async { final store = eg.store(); await store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName)]); - check(userMention(user, silent: true, users: store.users)).equals('@_**Full Name|123**'); + check(userMention(user, silent: true, users: store)).equals('@_**Full Name|123**'); }); test('`users` passed; has two same-name users but one of them is deactivated', () async { final store = eg.store(); await store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName, isActive: false)]); - check(userMention(user, silent: true, users: store.users)).equals('@_**Full Name|123**'); + check(userMention(user, silent: true, users: store)).equals('@_**Full Name|123**'); }); test('`users` passed; user has unique fullName', () async { final store = eg.store(); await store.addUsers([user, eg.user(userId: 234, fullName: 'Another Name')]); - check(userMention(user, silent: true, users: store.users)).equals('@_**Full Name**'); + check(userMention(user, silent: true, users: store)).equals('@_**Full Name**'); }); }); diff --git a/test/model/emoji_test.dart b/test/model/emoji_test.dart index a55f2dc36b..7916af0849 100644 --- a/test/model/emoji_test.dart +++ b/test/model/emoji_test.dart @@ -262,14 +262,14 @@ void main() { ]); }); - test('updates on RealmEmojiUpdateEvent', () { + test('updates on RealmEmojiUpdateEvent', () async { final store = prepare(); check(store.allEmojiCandidates()).deepEquals([ ...arePopularCandidates, isZulipCandidate(), ]); - store.handleEvent(RealmEmojiUpdateEvent(id: 1, realmEmoji: { + await store.handleEvent(RealmEmojiUpdateEvent(id: 1, realmEmoji: { '1': eg.realmEmojiItem(emojiCode: '1', emojiName: 'happy'), })); check(store.allEmojiCandidates()).deepEquals([ diff --git a/test/model/store_checks.dart b/test/model/store_checks.dart index f2ef63cd4b..00ada1eea5 100644 --- a/test/model/store_checks.dart +++ b/test/model/store_checks.dart @@ -39,7 +39,6 @@ extension PerAccountStoreChecks on Subject { Subject get account => has((x) => x.account, 'account'); Subject get selfUserId => has((x) => x.selfUserId, 'selfUserId'); Subject get userSettings => has((x) => x.userSettings, 'userSettings'); - Subject> get users => has((x) => x.users, 'users'); Subject> get streams => has((x) => x.streams, 'streams'); Subject> get streamsByName => has((x) => x.streamsByName, 'streamsByName'); Subject> get subscriptions => has((x) => x.subscriptions, 'subscriptions'); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 99235dbe67..eb6444309d 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -369,36 +369,8 @@ void main() { group('PerAccountStore.handleEvent', () { // Mostly this method just dispatches to ChannelStore and MessageStore etc., - // and so most of the tests live in the test files for those + // and so its tests generally live in the test files for those // (but they call the handleEvent method because it's the entry point). - - group('RealmUserUpdateEvent', () { - // TODO write more tests for handling RealmUserUpdateEvent - - test('deliveryEmail', () { - final user = eg.user(deliveryEmail: 'a@mail.example'); - final store = eg.store(initialSnapshot: eg.initialSnapshot( - realmUsers: [eg.selfUser, user])); - - User getUser() => store.users[user.userId]!; - - store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, - deliveryEmail: null)); - check(getUser()).deliveryEmail.equals('a@mail.example'); - - store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, - deliveryEmail: const JsonNullable(null))); - check(getUser()).deliveryEmail.isNull(); - - store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, - deliveryEmail: const JsonNullable('b@mail.example'))); - check(getUser()).deliveryEmail.equals('b@mail.example'); - - store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, - deliveryEmail: const JsonNullable('c@mail.example'))); - check(getUser()).deliveryEmail.equals('c@mail.example'); - }); - }); }); group('PerAccountStore.sendMessage', () { @@ -461,7 +433,7 @@ void main() { // clobber the recorded registerQueue request so we can't check it. // checkLastRequest(); - check(updateMachine.store.users.values).unorderedMatches( + check(updateMachine.store.allUsers).unorderedMatches( users.map((expected) => (it) => it.fullName.equals(expected.fullName))); })); @@ -518,7 +490,7 @@ void main() { updateMachine.debugPauseLoop(); check(complete).isTrue(); // checkLastRequest(); TODO UpdateMachine.debugPauseLoop was too late; see comment above - check(updateMachine.store.users.values).unorderedMatches( + check(updateMachine.store.allUsers).unorderedMatches( users.map((expected) => (it) => it.fullName.equals(expected.fullName))); })); diff --git a/test/model/user_test.dart b/test/model/user_test.dart new file mode 100644 index 0000000000..63ac1589c7 --- /dev/null +++ b/test/model/user_test.dart @@ -0,0 +1,82 @@ +import 'package:checks/checks.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/events.dart'; +import 'package:zulip/api/model/model.dart'; + +import '../api/model/model_checks.dart'; +import '../example_data.dart' as eg; +import 'test_store.dart'; + +void main() { + group('userDisplayName', () { + test('on a known user', () async { + final user = eg.user(fullName: 'Some User'); + final store = eg.store(); + await store.addUser(user); + check(store.userDisplayName(user.userId)).equals('Some User'); + }); + + test('on an unknown user', () { + final store = eg.store(); + check(store.userDisplayName(eg.user().userId)).equals('(unknown user)'); + }); + }); + + group('senderDisplayName', () { + test('on a known user', () async { + final store = eg.store(); + final user = eg.user(fullName: 'Old Name'); + await store.addUser(user); + final message = eg.streamMessage(sender: user); + await store.addMessage(message); + check(store.senderDisplayName(message)).equals('Old Name'); + + // If the user's name changes, `store.senderDisplayName` should update... + await store.handleEvent(RealmUserUpdateEvent(id: 1, + userId: user.userId, fullName: 'New Name')); + check(store.senderDisplayName(message)).equals('New Name'); + // ... even though the Message object itself still has the old name. + check(store.messages[message.id]!).senderFullName.equals('Old Name'); + }); + + test('on an unknown user', () async { + final store = eg.store(); + final message = eg.streamMessage(sender: eg.user(fullName: 'Some User')); + await store.addMessage(message); + // If the user is unknown, `store.senderDisplayName` should fall back + // to the name in the message... + check(store.senderDisplayName(message)).equals('Some User'); + // ... even though `store.userDisplayName` (with no message available + // for fallback) only has a generic fallback name. + check(store.userDisplayName(message.senderId)).equals('(unknown user)'); + }); + }); + + group('RealmUserUpdateEvent', () { + // TODO write more tests for handling RealmUserUpdateEvent + + test('deliveryEmail', () async { + final user = eg.user(deliveryEmail: 'a@mail.example'); + final store = eg.store(initialSnapshot: eg.initialSnapshot( + realmUsers: [eg.selfUser, user])); + + User getUser() => store.getUser(user.userId)!; + + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, + deliveryEmail: null)); + check(getUser()).deliveryEmail.equals('a@mail.example'); + + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, + deliveryEmail: const JsonNullable(null))); + check(getUser()).deliveryEmail.isNull(); + + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, + deliveryEmail: const JsonNullable('b@mail.example'))); + check(getUser()).deliveryEmail.equals('b@mail.example'); + + await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: user.userId, + deliveryEmail: const JsonNullable('c@mail.example'))); + check(getUser()).deliveryEmail.equals('c@mail.example'); + }); + }); +} diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index 3f3c32bd59..8dc24d239e 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -174,7 +174,7 @@ void main() { await tester.tap(find.text('User Three')); await tester.pump(); check(tester.widget(composeInputFinder).controller!.text) - .contains(userMention(user3, users: store.users)); + .contains(userMention(user3, users: store)); checkUserShown(user1, store, expected: false); checkUserShown(user2, store, expected: false); checkUserShown(user3, store, expected: false); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index b7384824fd..b5d3844c1a 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -799,8 +799,8 @@ void main() { foundOldest: false, messages: messages).toJson()); } - void handleMessageMoveEvent(List messages, String newTopic, {int? newChannelId}) { - store.handleEvent(eg.updateMessageEventMoveFrom( + Future handleMessageMoveEvent(List messages, String newTopic, {int? newChannelId}) async { + await store.handleEvent(eg.updateMessageEventMoveFrom( origMessages: messages, newTopicStr: newTopic, newStreamId: newChannelId, @@ -821,7 +821,7 @@ void main() { ..controller.isNotNull().text.equals('Some text'); prepareGetMessageResponse([message]); - handleMessageMoveEvent([message], 'new topic', newChannelId: otherChannel.streamId); + await handleMessageMoveEvent([message], 'new topic', newChannelId: otherChannel.streamId); await tester.pump(const Duration(seconds: 1)); check(tester.widget(channelContentInputFinder)) ..decoration.isNotNull().hintText.equals('Message #${otherChannel.name} > new topic') @@ -851,7 +851,7 @@ void main() { final existingMessage = eg.streamMessage( stream: eg.stream(), topic: 'new topic', content: 'Existing message'); prepareGetMessageResponse([existingMessage, message]); - handleMessageMoveEvent([message], 'new topic'); + await handleMessageMoveEvent([message], 'new topic'); await tester.pump(const Duration(seconds: 1)); check(find.textContaining('Existing message').evaluate()).length.equals(1); @@ -863,7 +863,7 @@ void main() { await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]); prepareGetMessageResponse([message]); - handleMessageMoveEvent([message], 'new topic'); + await handleMessageMoveEvent([message], 'new topic'); await tester.pump(const Duration(seconds: 1)); check(find.descendant( diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 0976ca1bfd..0d440accec 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -215,7 +215,7 @@ void main() { checkTitle(tester, user.fullName); }); - testWidgets('no error when user somehow missing from store.users', (tester) async { + testWidgets('no error when user somehow missing from user store', (tester) async { final user = eg.user(userId: 1); final message = eg.dmMessage(from: eg.selfUser, to: [user]); await setupPage(tester, @@ -271,7 +271,7 @@ void main() { checkTitle(tester, '${user0.fullName}, ${user1.fullName}'); }); - testWidgets('no error when one user somehow missing from store.users', (tester) async { + testWidgets('no error when one user somehow missing from user store', (tester) async { final users = usersList(2); final user0 = users[0]; final user1 = users[1];