notify: get notifs from activity, not hark #3609
Merged
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
native: fix issues with reactions in DMs
…ing-attachment native: blur message input when adding an attachment
…mages native: fix some issues with posting images to galleries
native: wire up chat sub
…d-focus-issues native: fix keyboard focus issues by relying on the OS
Fixes TLON-2068
Fixes TLON-2054
Fixes TLON-2038
Fixes TLON-2039
…op in ListItemTextIcon
…nnel root views Fixes TLON-2065
Fixes TLON-1966
Fixes TLON-2053
Fixes TLON-2066
Fixes TLON-2033
…istItemContent Fixes TLON-2058
native: low-priority omnibus
…inner-from-channel-view native: remove loading spinner from channel view
…s-in-note-content native: fix missing serifs in note content
…r-row-on-dms-and-multidms native: fix missing author row on DMs and multiDMS
…ked-images-in-gallery-posts native: better handle linked images in gallery posts
…t issue in threads
…oining-channels native: correctly handle channel join updates
ops: keep dev in sync with staging
…-in-galleries native: fix weird unread banner in gallery channels
… Avatar and GroupListItemContent
native: more low-priority fixes
…d the thread/postscreen view, add new shared keyboardavoidingview component
…sage-spacing-and-keyboard-offsets native: reduce space within/between chat messages, fix keyboard offset in thread view
The change as described above is simple enough. But there's a catch here that pulls in many lines of low-complexity high-nuance code: we want to continue supporting old clients. Notify passes notification uids through a pipeline where we can't trivially slap on additional markers/details. Current/old-style clients use these uids to look up notification data in the hark agent directly. Here, we propose a new scheme, of asking the notify agent itself about notification details, creating some separation of concerns, or at least separating us from hark. Clients may not update to use this scheme right away, so we include code that converts new-style activity events into old-style hark notifications, and injects them into hark. This way, we can support both old and new clients while emitting a single notification uid. Due to lack of context, this conversion cannot always achieve 100% parity with the old-style notifications. These cases have been marked for ::REVIEW explicitly. Smoke-tested, but not yet seen working end-to-end. Closes TLON-1517.
jamesacklin
reviewed
Jun 12, 2024
| == | ||
| :: | ||
| %dm-post | ||
| ::REVIEW should maybe not be rendered if you haven't accepted |
arthyn
approved these changes
Jun 12, 2024
| =; base=^path | ||
| ?~ id-reply base | ||
| (snoc base (rsh 4 (scot %ui u.id-reply))) | ||
| ?- kind.nest ::REVIEW technically want the -.kind-data of the post... |
There was a problem hiding this comment.
this is ok for now, but yes will have to change later
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.
The change as described above is simple enough. But there's a catch here that pulls in many lines of low-complexity high-nuance code: we want to continue supporting old clients.
Notify passes notification uids through a pipeline where we can't trivially slap on additional markers/details. Current/old-style clients use these uids to look up notification data in the hark agent directly.
Here, we propose a new scheme, of asking the notify agent itself about notification details, creating some separation of concerns, or at least separating us from hark.
Clients may not update to use this scheme right away, so we include code that converts new-style activity events into old-style hark notifications, and injects them into hark. This way, we can support both old and new clients while emitting a single notification uid.
Due to lack of context, this conversion cannot always achieve 100% parity with the old-style notifications. These cases have been marked for
::REVIEWexplicitly.Smoke-tested, but not yet seen working end-to-end.
Closes TLON-1517.