Paths

Table of Contentst

Differential D13239

[lib] Don't assume that threads exist in thread store when merging new messages
ClosedPublic
Actions

Authored by tomek on Wed, Sep 4, 5:56 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Thu, Sep 26, 6:21 PM

	Unknown Object (File)
	Tue, Sep 17, 6:30 PM

	Unknown Object (File)
	Sun, Sep 15, 12:14 PM

	Unknown Object (File)
	Sat, Sep 14, 5:38 PM

	Unknown Object (File)
	Sat, Sep 14, 2:02 PM

	Unknown Object (File)
	Tue, Sep 10, 3:01 AM

	Unknown Object (File)
	Mon, Sep 9, 1:45 PM

	Unknown Object (File)
	Sun, Sep 8, 11:42 PM

View All 15 Files

Subscribers

Details

Reviewers

kamil
inka

Commits

rCOMM80eb2670dcc7: [lib] Don't assume that threads exist in thread store when merging new messages

Summary

We don't store pending threads in thread store.

https://linear.app/comm/issue/ENG-9140/incorrect-handling-of-startreached-in-message-reducer

Test Plan

Check if creating thin and thick threads is possible.

Diff Detail

Repository

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Wed, Sep 4, 5:56 AM

Herald added a subscriber: ashoat. · View Herald TranscriptWed, Sep 4, 5:56 AM

Harbormaster completed remote builds in B31427: Diff 43882.Wed, Sep 4, 6:30 AM

tomek requested review of this revision.Wed, Sep 4, 6:30 AM

This fixes my issue, thank you!

This revision is now accepted and ready to land.Wed, Sep 4, 8:31 AM

Closed by commit rCOMM80eb2670dcc7: [lib] Don't assume that threads exist in thread store when merging new messages (authored by tomek). · Explain WhyWed, Sep 4, 8:53 AM

This revision was automatically updated to reflect the committed changes.

tomek added a commit: rCOMM80eb2670dcc7: [lib] Don't assume that threads exist in thread store when merging new messages.

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

lib/reducers/message-reducer.js
628 ↗	(On Diff #43884)	This code might have a bug for thick sidebars. Looking at `parsePendingThreadID`, it seems like for pending sidebars, we assume `threadTypes.SIDEBAR`.

ashoat added a subscriber: marcin.Mon, Sep 9, 8:27 AM

ashoat added inline comments.

lib/reducers/message-reducer.js
628 ↗	(On Diff #43884)	Actually, looks like @marcin already addressed this

tomek added inline comments.Tue, Sep 10, 12:36 AM

lib/reducers/message-reducer.js
628 ↗	(On Diff #43884)	This was fixed in https://linear.app/comm/issue/ENG-9142/pending-thread-ids-dont-support-thick-sidebars.

Revision Contents
Changeset List

Path

Size

lib/

reducers/

message-reducer.js

33 lines

Diff 43882

lib/reducers/message-reducer.js

Loading...