[lib] Move user infos update reducer into specs
ClosedPublic
Actions

Authored by tomek on Sep 26 2023, 3:51 AM.

Details

Reviewers

kamil
bartek
inka

Commits

rCOMMc48e1e23f6aa: [lib] Move user infos update reducer into specs

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

Branch

redux-update-spec

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Sep 26 2023, 3:51 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 26 2023, 3:51 AM

Harbormaster completed remote builds in B22844: Diff 31421.Sep 26 2023, 4:09 AM

tomek requested review of this revision.Sep 26 2023, 4:09 AM

tomek added a child revision: D9315: [lib] Move calendar filters update reducer into specs.Sep 28 2023, 7:38 AM

kamil requested changes to this revision.Sep 29 2023, 3:31 AM

kamil added inline comments.

lib/reducers/user-reducer.js
205–215	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	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	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	Yeah, you're right!