Stop using bogus data on unknown-stream ChatScreen
#5,212 opened on Jan 29, 2022
Repository metrics
- Stars
- (1,348 stars)
- PR merge metrics
- (PR metrics pending)
Description
As we saw in #5206, it's possible to have a message in the message list that's in a stream we don't have in our data structures. (For example: if you were in a stream, got some messages there, then left it, and either it's private (and you're not an admin) or you're a guest. Then find that message in all-messages, in search, or if applicable in @-mentions or starred messages.) If you tap that recipient header, we navigate to ChatScreen as usual, where we'd show the message list; but instead of an actual message list, we try to show a nice error screen.
This issue is about a pair of buggy aspects of that screen:
- There's a "Subscribe" button at the bottom, but you can't actually subscribe.
- The stream icon is "muted", which doesn't make a lot of sense.
The root cause of both of these is NULL_SUBSCRIPTION, and specifically its use by getStreamInNarrow. In general NULL_SUBSCRIPTION is always a sign of dubious code, because it consists entirely of bogus made-up data. (This is one of the few remaining uses of it; we've already eliminated all its former siblings.) See the comment by its definition, and the discussion at 25125db94 and the other commits mentioned in that comment.
Here, what's happening is:
-
NotSubscribedcallsgetStreamInNarrowto supply a localstream, and later consults bothstream.nameandstream.invite_only. The bogus value thatNULL_SUBSCRIPTIONarbitrarily chooses forinvite_onlyisfalse. SoNotSubscribedbehaves as if this stream is a public stream, and shows a "Subscribe" button.In reality, if the stream is unknown, then it's one our user cannot just subscribe to, and so
NotSubscribedshould not show that button. -
TitleStreamcallsgetStreamInNarrow, and later consultsstream.stream_id,stream.name,stream.in_home_view, andstream.invite_only. The bogus value inNULL_SUBSCRIPTIONforin_home_viewisfalse. This causesStreamIconto be passedisMuted: true, which causes it to show a "muted" icon.Instead, if the stream is unknown, we should show a more neutral "stream"/"hash" icon; or possibly the "private"/"lock" icon.
To fix this:
getStreamInNarrowon an unknown stream should returnundefined, notNULL_SUBSCRIPTION.- Each caller should then handle that possibility appropriately. In
NotSubscribedandTitleStream, use?.. For example,stream?.invite_only ?? true. - Where we use
stream.name, that can bestream?.name ?? '', preserving the current behavior. Fixing that will be a larger task, and a separate issue under the #5210 umbrella.
(This is part of the umbrella issue #5210.)