Page MenuHomePhabricator

Update input type of useChangeThreadSettings
ClosedPublic

Authored by marcin on Sep 11 2024, 2:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 22, 5:34 AM
Unknown Object (File)
Tue, Oct 22, 5:33 AM
Unknown Object (File)
Tue, Oct 22, 5:33 AM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:28 PM
Unknown Object (File)
Sep 26 2024, 12:13 AM
Unknown Object (File)
Sep 22 2024, 9:05 PM
Subscribers

Details

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
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

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.Sep 12 2024, 12:34 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)

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

lib/actions/thread-actions.js
126

Typo here

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.Sep 12 2024, 8:04 AM
This revision was landed with ongoing or failed builds.Sep 12 2024, 10:23 AM
This revision was automatically updated to reflect the committed changes.
web/modals/threads/settings/thread-settings-general-tab.react.js
23–24

This comment was not responded to

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.

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

Thanks for explaining!