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 Sep 4 2024, 5:56 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sun, Nov 10, 2:03 AM

	Unknown Object (File)
	Sat, Nov 9, 8:50 PM

	Unknown Object (File)
	Sat, Nov 9, 8:48 PM

	Unknown Object (File)
	Sat, Nov 9, 12:56 PM

	Unknown Object (File)
	Sun, Nov 3, 9:37 PM

	Unknown Object (File)
	Oct 23 2024, 11:57 AM

	Unknown Object (File)
	Oct 23 2024, 6:38 AM

	Unknown Object (File)
	Oct 23 2024, 6:11 AM

View All 35 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.Sep 4 2024, 5:56 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 4 2024, 5:56 AM

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

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

This fixes my issue, thank you!

This revision is now accepted and ready to land.Sep 4 2024, 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 WhySep 4 2024, 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.Sep 9 2024, 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.Sep 9 2024, 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.Sep 10 2024, 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...