Page MenuHomePhabricator

[lib] Extract read status operations logic
ClosedPublic

Authored by tomek on Aug 5 2024, 6:58 AM.
Tags
None
Referenced Files
F3368424: D12987.id43270.diff
Mon, Nov 25, 7:48 PM
F3367914: D12987.diff
Mon, Nov 25, 5:09 PM
Unknown Object (File)
Fri, Nov 22, 2:32 PM
Unknown Object (File)
Fri, Nov 22, 2:03 PM
Unknown Object (File)
Fri, Nov 22, 1:48 PM
Unknown Object (File)
Wed, Nov 20, 9:35 PM
Unknown Object (File)
Wed, Nov 20, 9:35 PM
Unknown Object (File)
Fri, Nov 8, 4:06 PM
Subscribers

Details

Summary
Test Plan

Processed an operation representing creation of a new thread with a viewer and a new user, created by that user. Checked if the thread was created and was set to unread.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/shared/dm-ops/process-dm-ops.js
74–75 ↗(On Diff #43145)

I didn't realize that the client code is able to ignore updateTypes.UPDATE_THREAD_READ_STATUS updates when their timestamp is too old. How does this work?

tomek requested review of this revision.Aug 5 2024, 9:29 AM
lib/shared/dm-ops/process-dm-ops.js
74–75 ↗(On Diff #43145)

It isn't yet. It needs to be implemented as a part of https://linear.app/comm/project/handle-unordered-dm-operations-1bd291b0b628. We have to make sure that if user A opens a thread on device A1, then user B sends an update to device A2 about a new message, and then A1 sends an update to A2 about changing the thread to read, we don't accidentally mark the thread as read if the message from A1 is older than the message from B.

But there are still more edge cases that need to be handled.

lib/shared/dm-ops/process-dm-ops.js
74–75 ↗(On Diff #43145)

Makes sense, thanks for explaining. I wonder if ignoring outdated updateTypes.UPDATE_THREAD_READ_STATUS would be useful for thin threads too... I often discover weird behavior where one client shows a thread as unread, and another doesn't

This revision is now accepted and ready to land.Aug 7 2024, 11:59 PM
lib/shared/dm-ops/process-dm-ops.js
74–75 ↗(On Diff #43145)
This revision was automatically updated to reflect the committed changes.