Page MenuHomePhabricator

[lib][native] Review usingOlmViaTunnelbrokerForDMs and pendingThreadType
ClosedPublic

Authored by inka on Sep 6 2024, 6:07 AM.
Tags
None
Referenced Files
F3177504: D13258.id43922.diff
Thu, Nov 7, 11:35 PM
F3176347: D13258.diff
Thu, Nov 7, 9:18 PM
Unknown Object (File)
Fri, Nov 1, 5:36 AM
Unknown Object (File)
Tue, Oct 22, 10:44 AM
Unknown Object (File)
Tue, Oct 22, 10:19 AM
Unknown Object (File)
Tue, Oct 22, 10:19 AM
Unknown Object (File)
Tue, Oct 22, 5:34 AM
Unknown Object (File)
Tue, Oct 22, 5:33 AM
Subscribers

Details

Summary

issue: ENG-9146
pendingThreadType should actually return thick or thin types depending on where we call it from.
We want to keep the usingOlmViaTunnelbrokerForDMs flag, so that for now thick threads are not created.

Test Plan

Tested that with the flag set to true thick threads can be created. Checked that with the flag set to false, thin threads are created instead.

Diff Detail

Repository
rCOMM Comm
Branch
inka/thread_ops
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/shared/thread-actions-utils.js
124

This is inside threadTypeIsThick(threadInfo.type), so we want the thread type to be thick

139

This in in an else to threadTypeIsThick(threadInfo.type), so we want this to be thin

lib/shared/thread-utils.js
1244

This needs to search for both, but that will be handled in ENG-8433. For now keeping the old behaviour

1262

If the thread doesn't exist, we will want to always create a thick thread. For now we want to keep creating a thin thread, but that is handled by the usingOlmViaTunnelbrokerForDMs flag

native/chat/message-list-container.react.js
162

Will remove this

173

This is responsible for displaying some text when creating a chat. Setting this to true breaks the current behaviour, because we need to update some other code as well for this to handle thick threads. Created ENG-9173 to track.

inka requested review of this revision.Sep 6 2024, 6:25 AM
This revision is now accepted and ready to land.Sep 6 2024, 7:33 AM
lib/shared/thread-utils.js
682 ↗(On Diff #43924)

Can we use an enum here instead of a boolean to make it more clear what's being specified at the callsite?

It could just be thickOrThin: 'thick' | 'thin', for instance

native/chat/message-list-container.react.js
170 ↗(On Diff #43924)

Is there equivalent code for web?

native/chat/message-list-container.react.js
170 ↗(On Diff #43924)

You are right. We don't display the thread type when creating a chat on native, but we do display breadcrumbs in the top bar. I created ENG-9260 to fix this