Page MenuHomePhabricator

[lib] Move user infos update reducer into specs
ClosedPublic

Authored by tomek on Sep 26 2023, 3:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 5:26 AM
Unknown Object (File)
Fri, Nov 1, 3:47 AM
Unknown Object (File)
Sat, Oct 12, 12:50 PM
Unknown Object (File)
Sat, Oct 12, 4:42 AM
Unknown Object (File)
Oct 2 2024, 11:52 AM
Unknown Object (File)
Oct 2 2024, 11:52 AM
Unknown Object (File)
Oct 2 2024, 11:52 AM
Unknown Object (File)
Oct 2 2024, 11:47 AM
Subscribers

Details

Summary

Create new state by merging infos from store with the ones from action, and then iterate through the updates and apply the function from a spec. Changed the approach slightly - instead of mutating the freshly created new state, return a new one from a spec (might be less performant, but is more maintainable).

Depends on D9268

https://linear.app/comm/issue/ENG-4241/handle-processupdatesactiontype-as-a-part-of-a-spec

Test Plan

Check if updating user infos work correctly: when adding a new user and deleting them.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested review of this revision.Sep 26 2023, 4:09 AM
kamil requested changes to this revision.Sep 29 2023, 3:31 AM
kamil added inline comments.
lib/reducers/user-reducer.js
205–215 ↗(On Diff #31421)

This code is very similar to this used in D9268, do you think it is possible to implement a generic function for reducing/parsing updates?
Wondering, because overall don't consider these reduce() functions to be really readable here, but given that the return function from specs will have a different name it may make things worse.

219 ↗(On Diff #31421)

This will return old values even with having new ones (userIfnos from state will overwrite updated)

This revision now requires changes to proceed.Sep 29 2023, 3:31 AM

Fix reducer bug

lib/reducers/user-reducer.js
205–215 ↗(On Diff #31421)

It is possible to create such a function, but it really challenging to type it correctly. I'm going to spend a little more time on this.

219 ↗(On Diff #31421)

Yeah, you're right!

lib/reducers/user-reducer.js
219 ↗(On Diff #31421)

This time, instead of console logging, I tested it by checking in the redux debugger what is the state after the action is reduced.

This revision is now accepted and ready to land.Oct 3 2023, 6:54 AM