Handle unknown users everywhere

[gnprice]

Our local store has a map with all the users we know about, in PerAccountStore.users aka store.users. But this might not be all the users in the realm. In particular, if the self-user is a guest user, there may be users we don't have permission to know about.

For the most part the users we actually encounter will be users we do have data on: for example, a guest user still has permission to know about users that are subscribed to any of the same streams they are, which means that most of the messages they see there will be sent by users they know about. But there could be older messages by users who've since unsubscribed, and other scenarios. So we will sometimes encounter a user ID that doesn't exist in store.users.

We should therefore make sure we never crash when we encounter a user that's not in store.users. This means:

• When we look up a user with an expression like store.users[userId], we should explicitly handle the possibility that the result is null.

  This means no ! and no assert saying it's non-null. Depending on the context, the code might use ?. and ?? to fall back to a placeholder like ZulipLocalizations.unknownUserName, or might return without doing anything, or something else as appropriate.

  • The exception is that it's fine to assume the self-user is present. We have a few places that do this; they could benefit from being factored out into a small getter like store.selfUser.

• When we have a context like a Message that provides its own details on a user (like the message sender), we should fall back to those if the user is unknown. That way we not only don't crash, but provide helpful information, in cases like a guest user looking at older messages sent by someone no longer subscribed to their streams.

We already do these correctly in many places, but there are several places we don't. So this task is about sweeping through those, probably in one commit per call site, and fixing them.

A key part of doing this is to find an exhaustive list of places that are potentially affected. You can do this in an IDE by going to the definition of PerAccountStore.users and then using the "find references" feature.

[mofirojean]

Our local store has a map with all the users we know about, in PerAccountStore.users aka store.users. But this might not be all the users in the realm. In particular, if the self-user is a guest user, there may be users we don't have permission to know about.
...
...
We already do these correctly in many places, but there are several places we don't. So this task is about sweeping through those, probably in one commit per call site, and fixing them.

A key part of doing this is to find an exhaustive list of places that are potentially affected. You can do this in an IDE by going to the definition of PerAccountStore.users and then using the "find references" feature.

HI @gnprice
As a follow up to this issue, i wanted to get some corrections so i could go ahead making changes. I was able to identify various areas in the code base following the reference store.users[userId] and I spotted quiet a few of them that matched what was explained on this issue.

I also noticed some had the correct changes you explain such as the store.users[userId]?.fullName ?? zulipLocalizations.unknownUserName, which was found on the PollWidget file. I was able to sweep accross the code base and found several areas that will need these changes. Can I start working this issue ?

[gnprice]

Sure, go ahead.

I recommend sending a small PR first that updates a handful of these places in the code. Then after that's been reviewed, you can use the feedback when writing the remaining changes, in order to make something that's as easy as possible for us to review and merge.

[mofirojean]

Okay, i will do just that.