Paths

Table of Contentst

Differential D13287

Update input type of useChangeThreadSettings
ClosedPublic
Actions

Authored by marcin on Wed, Sep 11, 2:42 AM.

Tags

None

Referenced Files

	F2761171: D13287.id44032.diff
	Thu, Sep 19, 7:31 AM

	F2755566: D13287.id44100.diff
	Wed, Sep 18, 9:58 PM

	F2748442: D13287.id44105.diff
	Wed, Sep 18, 10:23 AM

	Unknown Object (File)
	Wed, Sep 18, 4:47 AM

	Unknown Object (File)
	Wed, Sep 18, 3:31 AM

	Unknown Object (File)
	Tue, Sep 17, 8:12 PM

	Unknown Object (File)
	Tue, Sep 17, 8:10 PM

	Unknown Object (File)
	Tue, Sep 17, 12:18 PM

View All 13 Files

Subscribers

Details

Reviewers

tomek
kamil

Commits

rCOMM3909f7edd16f: Update input type of useChangeThreadSettings

Summary

This differential updates input type of useChangeThreadSettings to accept union of thin and thick thread specific data.

Test Plan

test that changing various thread settings works for both thin and thick threads and results in the right notif.

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Wed, Sep 11, 2:42 AM

Herald added a subscriber: ashoat. · View Herald TranscriptWed, Sep 11, 2:42 AM

marcin added a parent revision: D13260: Implement joining DM thread.Wed, Sep 11, 2:44 AM

Harbormaster completed remote builds in B31526: Diff 44031.Wed, Sep 11, 2:59 AM

marcin requested review of this revision.Wed, Sep 11, 2:59 AM

marcin added a child revision: D13289: Update input type of useUpdateSubscription.Wed, Sep 11, 4:10 AM

tomek accepted this revision.Thu, Sep 12, 12:34 AM

tomek added inline comments.

native/chat/settings/thread-settings-description.react.js
267 ↗	(On Diff #44031)	Why is it always false?

This revision is now accepted and ready to land.Thu, Sep 12, 12:34 AM

kamil accepted this revision.Thu, Sep 12, 1:53 AM

kamil added inline comments.

lib/actions/thread-actions.js
124–128 ↗	(On Diff #44031)	wondering, if this could be improved even more - for thick threads we're not able to modify thread members here (it's done via different spec), so we could remove the `newMemberIDs` field from `UpdateThreadRequest`
native/chat/settings/thread-settings-description.react.js
267 ↗	(On Diff #44031)	it shouldn't - it's possbile for DMs to modify description, but it's probably bug from the previous code
native/chat/settings/thread-settings-name.react.js
12–16 ↗	(On Diff #44031)	can be merged (applies to other files too)

Address review

Made significant changes to address feedback regarding adding members to thick threads.

Harbormaster completed remote builds in B31555: Diff 44072.Thu, Sep 12, 4:41 AM

ashoat added inline comments.Thu, Sep 12, 7:54 AM

lib/actions/thread-actions.js
126	Typo here

tomek accepted this revision.Thu, Sep 12, 8:04 AM

tomek added inline comments.

web/modals/threads/settings/thread-settings-general-tab.react.js
23–24	Why are we replacing this type by `ThickThreadChanges`? Are we still supporting thin threads? The new type is narrower, so it looks safe, but is confusing.

This revision is now accepted and ready to land.Thu, Sep 12, 8:04 AM

Rebase before landing

This revision was landed with ongoing or failed builds.Thu, Sep 12, 10:23 AM

Closed by commit rCOMM3909f7edd16f: Update input type of useChangeThreadSettings (authored by marcin). · Explain Why

This revision was automatically updated to reflect the committed changes.

marcin added a commit: rCOMM3909f7edd16f: Update input type of useChangeThreadSettings.

Harbormaster completed remote builds in B31571: Diff 44099.Thu, Sep 12, 10:24 AM

ashoat added inline comments.Thu, Sep 12, 12:33 PM

web/modals/threads/settings/thread-settings-general-tab.react.js
23–24	This comment was not responded to

marcin added inline comments.Thu, Sep 12, 12:35 PM

web/modals/threads/settings/thread-settings-general-tab.react.js
23–24	I am sorry for not documenting my answer here. However necessary changes were applied. `ThickThreadChanges` was renamed to `ThreadChanges` and `ThreadChanges` was renamed to `ThinThreadChanges` and used only in places where new members are added to thin thread.

ashoat added inline comments.Thu, Sep 12, 12:37 PM

web/modals/threads/settings/thread-settings-general-tab.react.js
23–24	Thanks for explaining!

Revision Contents
Changeset List

Path

Size

lib/

actions/

thread-actions.js

35 lines

components/

base-edit-thread-avatar-provider.react.js

13 lines

hooks/

promote-sidebar.react.js

1 line

shared/

messages/

text-message-spec.js

25 lines

types/

thread-types.js

15 lines

native/

chat/

settings/

add-users-modal.react.js

1 line

color-selector-modal.react.js

35 lines

thread-settings-description.react.js

27 lines

thread-settings-name.react.js

27 lines

web/

modals/

threads/

members/

add-members-modal.react.js

1 line

settings/

thread-settings-general-tab.react.js

6 lines

thread-settings-modal.react.js

7 lines

thread-settings-privacy-tab.react.js

6 lines

thread-settings-save-button.react.js

6 lines

thread-settings-utils.js

30 lines

Diff 44072

lib/actions/thread-actions.js

Loading...

lib/components/base-edit-thread-avatar-provider.react.js

Loading...

lib/hooks/promote-sidebar.react.js

Loading...

lib/shared/messages/text-message-spec.js

Loading...

lib/types/thread-types.js

Loading...

native/chat/settings/add-users-modal.react.js

Loading...

native/chat/settings/color-selector-modal.react.js

Loading...

native/chat/settings/thread-settings-description.react.js

Loading...

native/chat/settings/thread-settings-name.react.js

Loading...

web/modals/threads/members/add-members-modal.react.js

Loading...

web/modals/threads/settings/thread-settings-general-tab.react.js

Loading...

web/modals/threads/settings/thread-settings-modal.react.js

Loading...

web/modals/threads/settings/thread-settings-privacy-tab.react.js

Loading...

web/modals/threads/settings/thread-settings-save-button.react.js

Loading...

web/modals/threads/settings/thread-settings-utils.js

Loading...