We need this later in the stack to extract updated thread info from the result of operations processing.
https://linear.app/comm/issue/ENG-8930/unify-the-approach-to-replies-count-and-unread-status-updates
Depends on D13055
Differential D13057
[lib] Add function allowing extracting ThreadInfo from updates tomek on Aug 13 2024, 5:41 AM. Authored by Tags None Referenced Files
Subscribers
Details We need this later in the stack to extract updated thread info from the result of operations processing. https://linear.app/comm/issue/ENG-8930/unify-the-approach-to-replies-count-and-unread-status-updates Depends on D13055 Flow, tested with the rest of the stack.
Diff Detail
Event TimelineComment Actions One thing we could do here is add a separate update type for unread status. This way the logic from D13059 that needs getUpdatedThreadInfo would not be needed, and so getUpdatedThreadInfo would not be needed. But this might be too much work. Comment Actions Yeah, I'm still not sure if it is a good idea. This change would greatly simplify this logic, but has some downsides: introducing a whole new update type would require implementing its spec, which contains e.g. keyserver db layer - and we don't need that, but skipping it would also be weird. If we introduce this new update type, we will need to answer a question as to why we don't introduce similar updates for other props of the thread info... and it seems like we should do that - introduce a new update info that allows updating any property of thread info in isolation. But that might take some time. I don't think it is worth doing now.
Comment Actions Having threadInfo as a parameter doesn't make sense because in order to call this function we would need to know which part of the update represents the thread ID, which defeats the purpose of encapsulating this logic inside the spec. Comment Actions I'm confused about this discussion... don't we already have updateTypes.UPDATE_THREAD_READ_STATUS? Comment Actions Yes, we have. But this discussion was about introducing a new update for the replies count - which was the goal of this whole diff stack. I think this comes from the comment
Which contains a mistake. The new update, we were considering, should be for the replies count. |