fix(chat): sync the group feed on every trigger that refreshes the list - #1475
Merged
Merged
Conversation
The conversation list is two feeds behind one surface, and only one of them was reachable after login. `refreshFeed` delegated to `FeedSyncDelegate`, which fetches `CONTACT_DM` and `TIP_DM`; a group's row comes from `GroupFeedDelegate`, whose `syncGroupFeed` had exactly one caller, `onUserLoggedIn`. A chat push, a foreground resume, a network reconnect, the heartbeat's stream recovery, and both payment delegates refreshed the DM half alone. The chat-id-targeted half of a push plan hid most of this. `ApplyMessage` and `LoadMessages` do not branch on chat type, so a group the device already has reorders and re-previews normally. Both only ever `UPDATE chat_metadata ... WHERE chat_id_hex = :hex`, though, which is a no-op with no row, so a group joined from another device or joined while this one was backgrounded stayed out of the list until the next login. `syncFeeds` pairs the two fetches and every trigger goes through it, including the event stream's signal that a message arrived for a chat with no local row. The group feed is requested descending with a limit of 100, so a group that just pushed you a message is at the head of the page that fetches. The routing tests now tear the coordinator down in a `finally`. The heartbeat the login hook starts is a `while (true)` on the test scheduler, so a test that fails before its teardown line hangs the run instead of reporting.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The conversation list is two feeds behind one surface, and only one of them was reachable after login.
refreshFeeddelegated toFeedSyncDelegate, which fetchesCONTACT_DMandTIP_DM. A group's row comes fromGroupFeedDelegate, and itssyncGroupFeedhad exactly one caller:onUserLoggedIn. A chat push, a foreground resume, a network reconnect, the heartbeat's stream recovery, and both payment delegates refreshed the DM half alone.Why it mostly did not show
A push at a named chat plans two actions, and only one of them is the feed refresh. The other is
ApplyMessageorLoadMessages, neither of which branches on chat type, so a group the device already has reorders and re-previews exactly like a DM.Both of those only ever issue
UPDATE chat_metadata SET … WHERE chat_id_hex = :hex, which is a silent no-op when there is no row, and nothing else on the push path inserts one. A group joined from another device, or joined while this one was backgrounded, stayed out of the list until the next login — with a notification posted for a chat the list did not contain.The event stream has the same hole from the other side: it emits
SyncFeedRequestedprecisely when a message arrives for a chatmetadataDataSourcehas no row for, which routed to the DM-only sync.The change
syncFeedspairs the two fetches, and all six trigger sites go through it rather than either delegate directly. The delegates keep separate jobs, so the fetches overlap rather than queue, and a group-feed failure is traced and dropped insideGroupFeedDelegate— it cannot take the DM list down the way a contact-feed failure does.Group and DM sync stay in separate delegates.
GroupFeedDelegatedocuments why: the group feed pages where the DM feed does not, and their failure modes differ. Pairing them at the coordinator is the orchestration this class already owns.getChat(chatId)would be a more targeted fix for the missing-row case, and is deliberately not used here. The group feed is requested descending with a limit of 100, so a group that just pushed you a message is at the head of the page this fetches; a targeted call would only add something for a group with 100 more-recently-active groups ahead of it.Tests
Three cases in
GroupChatRoutingTest:refreshFeedreaches the group delegate, a foreground resume reaches it, andrefreshFeedstill fetches both DM feeds. The last one exists becauserefreshFeednow overrides theFeedOperationsdelegation, so the half that used to be the only one running needs asserting too.The file's tests now tear the coordinator down in a
finally. The heartbeat the login hook starts is awhile (true)on the test scheduler, so a test that fails before reaching its teardown line hangs the run instead of reporting the failure — which is how the first run of these tests against the unfixed source behaved.Related
Separate from #1474, which fixes who a group push says it came from. This one fixes whether the chat it names is in the list at all.