unread [nfc]: Refactor computing contents of inbox screen #5690
Conversation
This file contains 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
The existence of this helper is an implementation detail of this function. (And probably not even a good choice.) It definitely should not have its own tests -- the tests should be of the behavior that forms a meaningful interface for the rest of the app to use.
There's further refactoring opportunities this opens up; we'll take care of those in upcoming commits.
These were always false in the data structure actually returned to the UI; their presence there was basically a leak of an implementation detail, as the purpose they served was in the communication between getUnreadStreamsAndTopics and its helper. Now that we've erased that boundary, we don't need them at all.
(Empty here means there were unreads in the stream, but only in muted topics.)
Use more locals rather than re-looking things up on an object; and give them better names.
…opics The Map nature here wasn't doing us any good; we only ever inserted at a given stream once, and then just made an array from `.values()`. (Long ago this code made use of this being a map; but that ended when we switched to nice efficient data structures for stream unreads, which among other things give us all the data for a given stream at once.)
This is NFC because when there's no subscription, the old code would get `in_home_view = false` and promptly ignore the stream. As a bonus, this is one fewer use of NULL_SUBSCRIPTION. Not many left!
gnprice
added
P1 high-priority
server release goal
|
Thanks, LGTM! Merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
For #5362, we'll be revising how we apply the concept of muting, which will require some changes to this
getUnreadStreamsAndTopicsselector that computes the unreads to show on the inbox screen. The existing structure of that selector is a bit of a mess and would make that change more complicated than it needs to be. This PR cleans it up.As a bonus, we get rid of one of the few remaining uses of the bogus-data object NULL_SUBSCRIPTION.