[lib] Check the timestamps when adding the viewer to thread members
ClosedPublic
Actions

Authored by tomek on Sep 2 2024, 7:15 AM.

Details

Reviewers

kamil
inka

Commits

rCOMM2a7c9bc65f1a: [lib] Check the timestamps when adding the viewer to thread members

Summary

Check the timestamp and update the membership only when necessary.

https://linear.app/comm/issue/ENG-9114/update-addviewertothreadmembersspec

Depends on D13217

Test Plan

Created two add_viewer_to_thread_members operations and checked if the resulting timestamps are correct (all the timestamps should equal the first operation time except the membership.role timestamp).

Diff Detail

Repository

rCOMM Comm

Branch

timestamps

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Sep 2 2024, 7:15 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 2 2024, 7:15 AM

Rebase

Harbormaster completed remote builds in B31381: Diff 43826.Sep 2 2024, 7:32 AM

tomek added a child revision: D13219: [lib] Include `timestamps` inside `CreateThickRawThreadInfoInput`.Sep 2 2024, 7:41 AM

Harbormaster completed remote builds in B31385: Diff 43830.Sep 2 2024, 7:41 AM

tomek requested review of this revision.Sep 2 2024, 7:41 AM

inka added inline comments.Sep 3 2024, 3:50 AM

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
76 ↗	(On Diff #43830)	Same as in previous diffs, we should probably handle timestamp collisions
90–117 ↗	(On Diff #43830)	Why don't we make `createThickRawThreadInfo` take `timestamps`?

tomek added inline comments.Sep 5 2024, 3:15 AM

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
76 ↗	(On Diff #43830)	https://linear.app/comm/issue/ENG-9159/thick-threads-timestamps-conflict-resolution
90–117 ↗	(On Diff #43830)	In the next diff D13219 I add this parameter. I'm going to update this spec in that diff.