Page MenuHomePhabricator

[lib] Split change thread settings spec
ClosedPublic

Authored by tomek on Aug 12 2024, 7:27 AM.
Tags
None
Referenced Files
F2763907: D13053.diff
Thu, Sep 19, 11:50 AM
Unknown Object (File)
Mon, Sep 16, 3:16 AM
Unknown Object (File)
Sun, Sep 15, 9:07 AM
Unknown Object (File)
Sun, Sep 15, 9:06 AM
Unknown Object (File)
Tue, Sep 10, 2:31 PM
Unknown Object (File)
Thu, Sep 5, 9:34 AM
Unknown Object (File)
Tue, Sep 3, 4:28 PM
Unknown Object (File)
Mon, Sep 2, 5:22 PM
Subscribers

Details

Summary

Avoid sending unnecessary data and simplify the spec logic.

https://linear.app/comm/issue/ENG-8929/split-add-members-operation

Depends on D13052

Test Plan

Checked two scenarios:

  1. Viewer creates a thread and adds a new user to it using change_thread_settings operation
  2. Viewer is added to a thread using change_thread_settings_and_add_viewer operation

In both cases the thread appeared in the store with correct membership array.

Diff Detail

Repository
rCOMM Comm
Branch
dmops
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested review of this revision.Aug 12 2024, 8:55 AM
lib/shared/dm-ops/add-members-spec.js
27–28

What's the point of making the field read-only if the value isn't?

inka added inline comments.
lib/shared/dm-ops/change-thread-settings-and-add-viewer-spec.js
14

Can we call it createAddViewerOperation? or createAddViewerToThreadOperation

33

Can we merge this with the function above?

This revision is now accepted and ready to land.Aug 19 2024, 5:56 AM
lib/shared/dm-ops/add-members-spec.js
27–28

Right, this isn't too useful.

lib/shared/dm-ops/change-thread-settings-and-add-viewer-spec.js
14

Renamed to createAddViewerAndMembersOperation because there might be more users that get added.

33

We can't because createAddMembersOperation is used in canBeProcessed.

This revision was automatically updated to reflect the committed changes.