[lib][web][native] Update createRealThreadFromPendingThread to handle thick thread creation
ClosedPublic
Actions

Authored by inka on Thu, Sep 5, 1:22 AM.

Details

Reviewers

tomek
kamil

Commits

rCOMM9428742124b0: [lib][web][native] Update createRealThreadFromPendingThread to handle thick…

Summary

issues:
ENG-8442, ENG-9122

Test Plan

Tested by setting usingOlmViaTunnelbrokerForDMs to true. Not setting this flag yet, because it has other effects.
Tested that a thick thread gets created on both web and native.

Tested that if the flag is set to false, thin threads are created correctly

There is a problem with navigation - on both platfrms when the thick thread is created the user is not navigate to it. Instead they stay in the pending thread. I will address that in ENG-9147

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Thu, Sep 5, 1:22 AM

Herald added a subscriber: ashoat. · View Herald TranscriptThu, Sep 5, 1:22 AM

inka added a parent revision: D13243: [lib] Add useNewThickThread.Thu, Sep 5, 1:22 AM

Harbormaster completed remote builds in B31432: Diff 43890.Thu, Sep 5, 1:38 AM

inka requested review of this revision.Thu, Sep 5, 1:38 AM

inka edited the test plan for this revision. (Show Details)Thu, Sep 5, 1:52 AM

inka edited the test plan for this revision. (Show Details)

inka added a child revision: D13253: [lib][web][native] Update createPendingThread usages.Fri, Sep 6, 3:49 AM

tomek accepted this revision.Fri, Sep 6, 7:16 AM

tomek added inline comments.

native/input/input-state-container.react.js
470 ↗	(On Diff #43890)	Should we mention that this is an operation that sends a text message?

This revision is now accepted and ready to land.Fri, Sep 6, 7:16 AM

ashoat added inline comments.Mon, Sep 9, 5:39 AM

lib/shared/thread-actions-utils.js
105–108 ↗	(On Diff #43890)	Should we add this `invariant` for thin sidebars too?
native/input/input-state-container.react.js
512–525 ↗	(On Diff #43890)	What do you think of extracting these lines to a separate function? Lines 516-525 are already repeated on 585-594, and you're adding some more repeated code here
web/input/input-state-container.react.js
1323–1336 ↗	(On Diff #43890)	Same question here

kamil added inline comments.Mon, Sep 9, 8:58 AM

web/input/input-state-container.react.js
1340–1343 ↗	(On Diff #43890)	I guess this is causing this bug: ENG-9187. In the previous revision, it worked because my version was early exiting no matter if the thread was pending or not. This code is executed for thick threads also, so you create local entries with different IDs (`messageInfo` has `localID`, but for DMs, we're using a different `uuid.v4`, lines above). Later on, you early-exit before calling `dispatchActionPromise` so there is an entry in `local` which never gets removed. We should make sure that for thick threads we only use `processAndSendDMOperation` and not dispatch any other action (other actions are thin threads related). Worth checking other places that could possibly be affected.