Page MenuHomePhabricator
Differential D13788

[lib] reduce thread updates from createOrUpdateFarcasterChannelTag and deleteFarcasterChannelTag
ClosedPublic
Actions

Authored by varun on Thu, Oct 24, 12:37 PM.
Tags
None
Referenced Files
F3328768: D13788.diff
Wed, Nov 20, 3:09 PM
Unknown Object (File)
Fri, Nov 15, 12:01 AM
Unknown Object (File)
Thu, Nov 14, 3:01 PM
Unknown Object (File)
Thu, Nov 14, 2:54 PM
Unknown Object (File)
Thu, Nov 14, 2:48 PM
Unknown Object (File)
Mon, Nov 11, 4:22 AM
Unknown Object (File)
Mon, Nov 11, 12:31 AM
Unknown Object (File)
Sun, Nov 10, 1:46 PM
View All 42 Files
Subscribers
tomek

Details

Reviewers
ashoat
Commits
rCOMM7fa23109af41: [lib] reduce thread updates from createOrUpdateFarcasterChannelTag and…
Summary

Depends on D13779

Test Plan

tested setting and unsetting community tags from a client running an older code version and it worked as expected -- no crashes, communities and threads tables updated on keyserver. eventually the state check mechanism synced the client threads with the keyserver

tested the same thing on a new client that can receive the thread changes directly and confirmed that the actions were reduced correctly and the redux state thread info looked right

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Thu, Oct 24, 12:37 PM
Herald added a subscriber: tomek. · View Herald TranscriptThu, Oct 24, 12:37 PM
varun updated this revision to Diff 45382.Thu, Oct 24, 12:46 PM

remove unnecessary import

Harbormaster failed remote builds in B32370: Diff 45381!Thu, Oct 24, 12:51 PM
Harbormaster completed remote builds in B32371: Diff 45382.Thu, Oct 24, 1:07 PM
varun requested review of this revision.Thu, Oct 24, 1:07 PM
ashoat requested changes to this revision.Thu, Oct 24, 2:51 PM

I looked at the type of ChangeThreadSettingsResult. It has two properties, one of which is newMessageInfos.

It looks like some of the other action types (eg. deleteCommunityRoleActionTypes.success) aren't reduced in message-reducer.js either. Is there a specific reason we aren't reducing the messages here?

lib/reducers/thread-reducer.js
275–281 ↗(On Diff #45382)

Don't think we need this after the type updates I suggest in prior diffs

This revision now requires changes to proceed.Thu, Oct 24, 2:51 PM
varun updated this revision to Diff 45385.Fri, Oct 25, 9:25 AM

we do want to reduce the messages too

varun added inline comments.Fri, Oct 25, 9:28 AM
lib/reducers/thread-reducer.js
275–281 ↗(On Diff #45382)

we do need this. in my latest comments on D13778 i explain why

https://phab.comm.dev/D13778#inline-77986

Harbormaster completed remote builds in B32373: Diff 45385.Fri, Oct 25, 9:44 AM
ashoat accepted this revision.Fri, Oct 25, 9:49 AM
This revision is now accepted and ready to land.Fri, Oct 25, 9:49 AM
Closed by commit rCOMM7fa23109af41: [lib] reduce thread updates from createOrUpdateFarcasterChannelTag and… (authored by varun). · Explain WhyFri, Oct 25, 11:48 AM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM7fa23109af41: [lib] reduce thread updates from createOrUpdateFarcasterChannelTag and….

Revision Contents
Changeset List

PathSize
lib/
reducers/
19 lines
15 lines

Diff 45388

View Options

lib/reducers/message-reducer.js

Loading...
View Options

lib/reducers/thread-reducer.js

Loading...