Page MenuHomePhabricator
Differential D9245

[lib] Create thread store ops from updates in a spec
ClosedPublic
Actions

Authored by tomek on Sep 20 2023, 5:23 AM.
Tags
None
Referenced Files
F6136925: D9245.diff
Tue, Apr 22, 5:50 AM
Unknown Object (File)
Thu, Apr 17, 12:42 PM
Unknown Object (File)
Thu, Apr 17, 11:07 AM
Unknown Object (File)
Thu, Apr 17, 9:04 AM
Unknown Object (File)
Thu, Apr 17, 2:47 AM
Unknown Object (File)
Thu, Apr 17, 12:01 AM
Unknown Object (File)
Wed, Apr 16, 5:59 AM
Unknown Object (File)
Mon, Apr 14, 11:18 AM
View All Files
Subscribers
ashoat
michal

Details

Reviewers
kamil
bartek
inka
Commits
rCOMMf165401467fe: [lib] Create thread store ops from updates in a spec
Summary

Move the logic from a reducer to specs.

Depends on D9244

https://linear.app/comm/issue/ENG-4590/describe-which-ops-are-generated-from-a-redux-action-in-a-spec

Test Plan

Open two clients and check if making operations (e.g. updating or joining a thread) generates the right ops on the other.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Sep 20 2023, 5:23 AM
Herald added a subscriber: ashoat. · View Herald TranscriptSep 20 2023, 5:23 AM
Harbormaster completed remote builds in B22758: Diff 31310.Sep 20 2023, 5:39 AM
tomek requested review of this revision.Sep 20 2023, 5:39 AM
tomek added a child revision: D9257: [lib] Move entries updates logic into a spec.Sep 21 2023, 8:22 AM
kamil accepted this revision.Sep 25 2023, 2:20 AM
kamil added inline comments.
lib/reducers/thread-reducer.js
70–80

I think this can be simplified to return payload.updatesResult.newUpdates.map(update => ... ).filter(Boolean).flat();

lib/shared/updates/update-thread-read-status-spec.js
17–20

nit: I think negation of this condition and early exit is more readable

This revision is now accepted and ready to land.Sep 25 2023, 2:20 AM
tomek updated this revision to Diff 31607.Oct 3 2023, 4:46 AM

Add early return and use more functional approach

lib/reducers/thread-reducer.js
70–80

Initially, I used a similar approach to what you propose, but Flow was complaining about it. Now I tried it again, and it seems to work.

Harbormaster completed remote builds in B22946: Diff 31607.Oct 3 2023, 5:04 AM
Closed by commit rCOMMf165401467fe: [lib] Create thread store ops from updates in a spec (authored by tomek). · Explain WhyOct 9 2023, 3:57 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMf165401467fe: [lib] Create thread store ops from updates in a spec.

Revision Contents
Changeset List

Diff 31310

View Options

lib/reducers/thread-reducer.js

Loading...
View Options

lib/shared/updates/bad-device-token-spec.js

Loading...
View Options

lib/shared/updates/delete-account-spec.js

Loading...
View Options

lib/shared/updates/delete-thread-spec.js

Loading...
View Options

lib/shared/updates/join-thread-spec.js

Loading...
View Options

lib/shared/updates/update-current-user-spec.js

Loading...
View Options

lib/shared/updates/update-entry-spec.js

Loading...
View Options

lib/shared/updates/update-spec.js

Loading...
View Options

lib/shared/updates/update-specs.js

Loading...
View Options

lib/shared/updates/update-thread-read-status-spec.js

Loading...
View Options

lib/shared/updates/update-thread-spec.js

Loading...
View Options

lib/shared/updates/update-user-spec.js

Loading...
View Options

lib/types/update-types.js

Loading...