fix(chat): render and route group pushes, and cap transcript sender lookups - #1474
Merged
Merged
Conversation
`applyChatStyle` resolved the sender with `getOtherMember`, which returns the first member that isn't you. That is the sender in a DM and an arbitrary participant anywhere else, so every group push carried the wrong name and photo. `PushChatMetadata.sendingUserId` was already decoded and never read. `planSenderLookup` now picks the identity: a linked device contact where one exists, otherwise the sending user's profile, and nobody for a group system message rather than a guessed participant. The profile is read from `user_profiles` first — `ChatMemberDataSource` already writes every synced member's profile there — and only falls through to `ProfileController` for someone the device has not seen. `MessagingStyle` also keyed every `Person` by `groupKey`, which is per-chat, so all senders in a group collapsed into one person and the first speaker's name and icon were stamped on everyone's messages. The key is now the sender's user id.
`request` released a user id on any failure, and the transcript re-asks on every emission of its `combine` — each page load, each pending mutation, each write to `user_profiles`. One `NOT_FOUND` sender in a group therefore cost a `GetProfile` per emission for the life of the process. `NOT_FOUND` is an answer, not a failure to get one, so those ids are now held permanently. Other failures are held for 30s rather than released immediately. The next emission of the transcript is not evidence the network came back, so retrying on it just fails again, and being offline while scrolling was a burst of attempts that could not succeed. Opening a group whose loaded pages carry senders past the roster page asked for all of them in one frame. The in-flight set collapses duplicates but says nothing about the fan-out across distinct ids, so a semaphore caps concurrent fetches at six, held for the call alone. `clear` now cancels outstanding work. A fetch in flight was authorized by the account that just went away, and a sleeping backoff would otherwise wake to release an id the next account may already be fetching.
`applyChatStyle` builds a `MessagingStyle` but never calls `setGroupConversation` or `setConversationTitle`, so every chat push posts as a one-to-one. The platform reads that as "the notification title already says who this is" and drops the sender's name from the message line, and there is no group name anywhere on the notification. Set both, and set them after `extractMessagingStyleFromNotification`: a re-post rebuilds the style from the notification already on screen, which carries the previous flag and title back with it. The title comes from `chat_metadata.title` — the same row the conversation list is named from — through a new single-column DAO read. It is applied only when non-null, so a push that lands before the group's row syncs leaves whatever an earlier push resolved rather than blanking it. Resolving the type moved into `planConversationStyling`, payload first and stored row second, because `chat_metadata` is optional on the wire (`ProtobufToLocal.asPayload`). A group push that omits it was passing for a DM, which also sent `planSenderLookup` down its `OtherMember` branch and put an arbitrary participant's name on the notification. Both the styling and the sender lookup now key off the one resolved value. A group this device has never synced still posts untitled — the push's own `title` field is the sender-name fallback on this path, so reading it as a conversation title would be a guess about what the server puts there.
`buildContentIntent` deep-linked `ChatType.TIP_DM` alone, so tapping a group
push launched the app on the camera and left the user to find the chat.
The `/tip/chat/{id}` link is named for where it was first used, but `AppRouter`
resolves it to the Chats tab and the conversation by id — which is the
destination for a group too, since that tab lists both types
(`TipFlowViewModel`: `feed(TIP_DM, GROUP)`). `planChatTapTarget` now decides
from the resolved chat type, and only a `CONTACT_DM` still falls back to the
launcher: the Send tab that opened one was removed and `AppRouter` routes
nothing for it.
`planConversationStyling` moved up into `postNotification` so the tap target
and the message style share one resolved type. The content intent was reading
`payload.chatMetadata.chatType` directly and had the same blind spot the style
did — `chat_metadata` is optional on the wire, and a group push that omits it
would still have landed on the camera.
Every content intent was also built under request code 99. `FLAG_UPDATE_CURRENT`
rewrites the extras of whichever PendingIntent is held under a code, and
`filterEquals` ignores extras, so with two chats notified both notifications
pointed at whichever was posted last. The code now travels with the target.
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
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.
Group chats deliver pushes fine — nothing in the Android handling path gates on
ChatType. What they get wrong is how the push renders: who it says it came from, whether it reads as a group at all, and how often the transcript asks the server to fill in a sender it will never get.Push attribution
NotificationService.applyChatStyleresolved the sender withgetOtherMember, which returns the first member of the chat that isn't you. That's the sender in a DM and an arbitrary participant anywhere else, so every group push carried the wrong name and photo.PushChatMetadata.sendingUserIdwas already decoded off the payload and never read.planSenderLookupnow picks the identity, with the three cases split out so the rule is testable without a database or a live service:The profile is read from
user_profilesfirst.ChatMemberDataSourcealready writes every synced member's profile there, so this is a local read for anyone the app has seen, and only falls through toProfileControllerfor a group participant past the roster page. It writes back, so the next push from them doesn't.MessagingStylealso keyed everyPersonbygroupKey. That's per-chat, so all senders in a group collapsed into one person and the first speaker's name and icon got stamped on everyone's messages. The key is now the sender's user id.Transcript sender resolution
GetProfiletakes one user id. There's no batch RPC, and no chat RPC that lists members past the pageGetChatreturns, so one call per unknown sender is the floor.SenderResolverwas not holding it as the ceiling.requestreleased a user id on any failure, and the transcript re-asks on every emission of itscombine— each page load, each pending mutation, each write touser_profiles. OneNOT_FOUNDsender in a group therefore cost aGetProfileper emission for the life of the process.NOT_FOUNDis an answer, not a failure to get one, so those ids are now held permanently.Other failures are held for 30s rather than released immediately. The next emission of the transcript isn't evidence the network came back, so retrying on it just fails again — scrolling while offline was a burst of attempts that couldn't succeed.
Opening a group whose loaded pages carry senders past the roster page asked for all of them in one frame. The in-flight set collapses duplicates but says nothing about the fan-out across distinct ids, so a semaphore caps concurrent fetches at six, held for the network call alone.
clearnow cancels outstanding work. A fetch in flight was authorized by the account that just went away, and a sleeping backoff would otherwise wake to release an id the next account may already be fetching.Group conversation styling
applyChatStylenever calledsetGroupConversationorsetConversationTitle, so every chat push posted as a one-to-one. The platform reads that as "the notification title already says who this is" and drops the sender's name from the message line — which is what kept the attribution above from being visible in the shade — and no group name appeared anywhere.Both are now set, after
extractMessagingStyleFromNotification: a re-post rebuilds the style from the notification already on screen and carries the previous flag and title back with it. The title comes fromchat_metadata.title, the same row the conversation list is named from, and is applied only when non-null so a push that lands before the group's row syncs leaves whatever an earlier push resolved.Resolving the type moved into
planConversationStyling, payload first and stored row second.chat_metadatais optional on the wire (ProtobufToLocal.asPayload), and a group push that omitted it was passing for a DM — which also sentplanSenderLookupdown itsOtherMemberbranch. Both the styling and the sender lookup now key off the one resolved value.A group this device has never synced still posts untitled. The push's own
titlefield is the sender-name fallback on this path, so reading it as a conversation title would be a guess about what the server puts there.Tap target
buildContentIntentdeep-linkedTIP_DMalone, so tapping a group push launched the app on the camera. The/tip/chat/{id}link is named for where it was first used, butAppRouterresolves it to the Chats tab and the conversation by id — the destination for a group too, since that tab lists both types (TipFlowViewModel:feed(TIP_DM, GROUP)).planChatTapTargetdecides from the resolved type now; only aCONTACT_DMstill falls back to the launcher, because the Send tab that opened one was removed andAppRouterroutes nothing for it. No manifest change —app.flipcash.com/tip/.*is already claimed.planConversationStylingmoved up intopostNotificationso the tap target and the message style share one resolved type. The content intent was readingpayload.chatMetadata.chatTypedirectly and had the same blind spot.Separately, every content intent was built under request code 99.
FLAG_UPDATE_CURRENTrewrites the extras of whichever PendingIntent is held under a code andfilterEqualsignores extras, so with two chats notified both notifications pointed at whichever was posted last. The code travels with the target now.