Paths

Table of Contentst

Differential D13052

[lib] Split add members spec
ClosedPublic
Actions

Authored by tomek on Aug 12 2024, 7:21 AM.

Tags

None

Referenced Files

	F2763864: D13052.id43518.diff
	Thu, Sep 19, 11:47 AM

	F2763513: D13052.id43312.diff
	Thu, Sep 19, 11:31 AM

	F2763504: D13052.id43455.diff
	Thu, Sep 19, 11:28 AM

	F2756701: D13052.diff
	Thu, Sep 19, 1:02 AM

	Unknown Object (File)
	Tue, Sep 17, 1:52 AM

	Unknown Object (File)
	Mon, Sep 16, 3:18 AM

	Unknown Object (File)
	Sun, Sep 15, 9:07 AM

	Unknown Object (File)
	Sun, Sep 15, 9:06 AM

View All 40 Files

Subscribers

Details

Reviewers

kamil
inka

Commits

rCOMMd2fee63e6327: [lib] Split add members spec

Summary

Split the spec so that we don't send extraneous data to all the existing members.

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

Test Plan

Checked two scenarios:

Viewer creates a thread and adds a new user to it using add_members operation
Viewer is added to a thread using add_viewer_to_thread_members

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

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Aug 12 2024, 7:21 AM

Herald added a subscriber: ashoat. · View Herald TranscriptAug 12 2024, 7:21 AM

tomek added a child revision: D13053: [lib] Split change thread settings spec.Aug 12 2024, 7:27 AM

Harbormaster completed remote builds in B31048: Diff 43312.Aug 12 2024, 8:40 AM

tomek requested review of this revision.Aug 12 2024, 8:40 AM

inka added inline comments.Aug 19 2024, 5:41 AM

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
93	I don't think that's a correct reason?

Fix returned validation result

tomek marked an inline comment as done.Aug 19 2024, 7:09 AM

tomek added inline comments.

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
93	Yes, you're right. I added a comment // The operation is invalid when this condition is false where I tried to explain this, but a better solution is to add another `reason` to the response. In the future, this should be handled by our validation.

inka accepted this revision.Aug 19 2024, 7:11 AM

This revision is now accepted and ready to land.Aug 19 2024, 7:11 AM

Harbormaster completed remote builds in B31142: Diff 43455.Aug 19 2024, 7:26 AM

marcin added a parent revision: D13050: Extract logic of pushInfo -> usersToCollapsableNotifInfo to lib and use for client-based notifs.Aug 20 2024, 7:42 AM

tomek removed a parent revision: D13050: Extract logic of pushInfo -> usersToCollapsableNotifInfo to lib and use for client-based notifs.Aug 20 2024, 9:28 AM

Closed by commit rCOMMd2fee63e6327: [lib] Split add members spec (authored by tomek). · Explain WhyAug 20 2024, 9:33 AM

This revision was automatically updated to reflect the committed changes.

tomek marked an inline comment as done.

tomek added a commit: rCOMMd2fee63e6327: [lib] Split add members spec.

marcin added a parent revision: D13050: Extract logic of pushInfo -> usersToCollapsableNotifInfo to lib and use for client-based notifs.Tue, Aug 20, 1:16 PM

marcin removed a parent revision: D13050: Extract logic of pushInfo -> usersToCollapsableNotifInfo to lib and use for client-based notifs.Wed, Aug 21, 4:19 AM

Revision Contents
Changeset List

Path

Size

lib/

shared/

dm-ops/

add-members-spec.js

149 lines

add-viewer-to-thread-members-spec.js

100 lines

change-thread-settings-spec.js

62 lines

2 lines

types/

36 lines

Diff 43312

lib/shared/dm-ops/add-members-spec.js

Loading...

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js

Loading...

lib/shared/dm-ops/change-thread-settings-spec.js

Loading...

lib/shared/dm-ops/dm-op-specs.js

Loading...

lib/types/dm-ops.js

Loading...