Add UserStore; pull out getUser, userDisplayName, others #1327
+321
−156
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<User> 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<int, User> _users; | ||
|
|
||
| @override | ||
| User? getUser(int userId) => _users[userId]; | ||
|
|
||
| @override | ||
| Iterable<User> 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; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see that this assertion was already ineffective — data stores like
TypingStatus,Unreads, andRecentDmConversationsViewcould all take differentselfUserIds without detecting the discrepancy.In contrast, the three surviving assertions check that
realmUrlis consistent across the data stores. While we can do that forselfUserIdtoo, I'm not sure if that's useful. A bug can slip through if we add a newrealmUrlwithout adding a new assertion below, and the same issue applies toselfUserIdassertions if we are to maintain them.Perhaps a better way would be sharing the reference to
realmUrlandselfUserId, instead of passing them multiple times, but the wayPerAccountStore.fromInitialSnapshotis set up already makes it easy to check for mistakes, making this less of an issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this check is basically already internal to the implementation of the PerAccountStore constructors, so I think it's not essential to keep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also something I think we'll want to do — what I'm imagining is making a type named perhaps BasePerAccountStore, with those and perhaps
connectionand/oraccount(which subsumes them), and then these other sub-stores can sayon BasePerAccountStoreto get access to those getters.(Naturally that's for a future PR, though.)