Page MenuHomePhabricator

[lib] Update entry reducer so thick entry infos are properly stored
ClosedPublic

Authored by will on Mon, Sep 16, 1:15 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Sep 16, 2:00 PM
Unknown Object (File)
Mon, Sep 16, 1:59 PM
Unknown Object (File)
Mon, Sep 16, 1:56 PM
Subscribers
None

Details

Summary

Discovered that thick raw entry infos weren't being stored as thick and this was because the reducer wasn't accounting for their fields. This diff allows thick raw entry infos to be stringified properly

Depends on D13315

Test Plan

Tested later in stack by being able to access thick field from entry info

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

will requested review of this revision.Mon, Sep 16, 1:32 AM
tomek added inline comments.
lib/reducers/entry-reducer.js
117–137 ↗(On Diff #44192)

Can we merge these two branches? Or is Flow complaining about it?

Additionally, serverID is confusing for thick entries.

This revision is now accepted and ready to land.Mon, Sep 16, 3:31 AM
lib/reducers/entry-reducer.js
117–137 ↗(On Diff #44192)

I'm not sure I understand the feedback here... aren't the branches distinct? Curious for more details from Tomek on why he thinks the branches should be merged

lib/reducers/entry-reducer.js
117–137 ↗(On Diff #44192)

We didn't end up addressing this. Created a follow up for further discussion: https://linear.app/comm/issue/ENG-9305/consider-merging-branches-in-logic-for-adding-thick-thread-fields

lib/reducers/entry-reducer.js
117–137 ↗(On Diff #44192)

In the else branch, we're taking properties from rawEntryInfo. Then, in if (rawEntryInfo.thick) we're adding some more properties thick-entry specific. My guess was that we could merge the logic by simply spreading the rawEntryInfo, which should cover both thin and thick entry branches.