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
F5901564: D13239.id43884.diff
Thu, Apr 17, 12:43 PM
F5894432: D13239.id43882.diff
Thu, Apr 17, 10:23 AM
F5889956: D13239.id.diff
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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

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

Actually, looks like @marcin already addressed this

tomek added inline comments.Sep 10 2024, 12:36 AM
lib/reducers/message-reducer.js
628

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 43884

View Options

lib/reducers/message-reducer.js

  • This file was added.
// @flow
import * as React from 'react';
import { DBOpsHandler as BaseHandler } from 'lib/handlers/db-ops-handler.react.js';
import type { StoreOperations } from 'lib/types/store-ops-types.js';
import { useSelector } from '../redux/redux-utils.js';
import { processDBStoreOperations } from '../shared-worker/utils/store.js';
function DBOpsHandler(): React.Node {
const currentUserID = useSelector(state => state.currentUserInfo?.id ?? null);
const processOperations = React.useCallback(
(ops: StoreOperations) => processDBStoreOperations(ops, currentUserID),
[currentUserID],
);
return <BaseHandler processDBStoreOperations={processOperations} />;
}
export { DBOpsHandler };