Page MenuHomePhabricator

[lib] Check the timestamps when leaving a thread
ClosedPublic

Authored by tomek on Mon, Sep 2, 8:53 AM.
Tags
None
Referenced Files
F2743705: D13220.diff
Wed, Sep 18, 3:45 AM
Unknown Object (File)
Mon, Sep 16, 1:24 AM
Unknown Object (File)
Mon, Sep 16, 12:58 AM
Unknown Object (File)
Sun, Sep 15, 11:15 AM
Unknown Object (File)
Sat, Sep 14, 3:55 PM
Unknown Object (File)
Sat, Sep 14, 12:38 PM
Unknown Object (File)
Fri, Sep 13, 2:06 AM
Unknown Object (File)
Thu, Sep 12, 8:45 PM
Subscribers

Details

Summary

Only remove the editor when the operation is more recent than the timestamp.

https://linear.app/comm/issue/ENG-9116/update-leavethreadspec

Depends on D13219

Test Plan

Create a thread, add some members, and remove some. Check if the timestamps behave correctly, and the membership is correct.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested review of this revision.Mon, Sep 2, 9:09 AM
inka requested changes to this revision.Tue, Sep 3, 4:15 AM
inka added inline comments.
lib/shared/dm-ops/leave-thread-spec.js
73 ↗(On Diff #43834)

We seem to prioritise removing a user over adding a user. I would do it the other way around because it's easier to remove someone than it is to add them, but up to you.

74–77 ↗(On Diff #43834)

We are updating the timestamp even if isMember > time. Should we early return here like in previous diffs?

This revision now requires changes to proceed.Tue, Sep 3, 4:15 AM

Fix the timestamp condition

This revision is now accepted and ready to land.Fri, Sep 6, 5:05 AM
This revision was landed with ongoing or failed builds.Fri, Sep 6, 8:10 AM
This revision was automatically updated to reflect the committed changes.