Page MenuHomePhabricator
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)
Thu, Apr 17, 12:43 PM
Unknown Object (File)
Thu, Apr 17, 10:23 AM
Unknown Object (File)
Thu, Apr 17, 5:31 AM
Unknown Object (File)
Wed, Apr 16, 2:23 PM
Unknown Object (File)
Mar 15 2025, 4:06 AM
Unknown Object (File)
Mar 15 2025, 4:06 AM
Unknown Object (File)
Mar 15 2025, 4:06 AM
Unknown Object (File)
Mar 15 2025, 4:06 AM
View All 57 Files
Subscribers
ashoat
marcin

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
rCOMM Comm
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
inka accepted this revision.Sep 4 2024, 8:31 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

PathSize
lib/
reducers/
33 lines

Diff 43882

View Options

lib/reducers/message-reducer.js

Loading...