[lib] Move calendar filters update reducer into specs
ClosedPublic
Actions

Authored by tomek on Sep 28 2023, 7:38 AM.

Details

Reviewers

kamil
bartek
inka

Commits

rCOMMdc850ca7e1ef: [lib] Move calendar filters update reducer into specs

Summary

Changed a solution slightly. Instead of mutating a set for each update, we're creating a new one, which might be less performant, but is better for maintainability.

Depends on D9296

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

Test Plan

Bring back the old code and check if it produces the same result while handling delete, join, or update thread update.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Sep 28 2023, 7:38 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 28 2023, 7:38 AM

Harbormaster completed remote builds in B22894: Diff 31525.Sep 28 2023, 7:56 AM

tomek requested review of this revision.Sep 28 2023, 7:56 AM

tomek added a child revision: D9318: [lib] Move local id update reducer into specs.Sep 28 2023, 8:10 AM

kamil accepted this revision.Sep 29 2023, 3:55 AM

kamil added inline comments.

lib/reducers/calendar-filters-reducer.js
142–146 ↗	(On Diff #31525)	Maybe we can make this logic here more consistent with other `reduce` from previous diffs, but don't have a strong opinion on that.
149 ↗	(On Diff #31525)	This also makes sense in a way it is, but just want to make sure, is the operator `!==` intentional and better than `_isEqual` in this case?

This revision is now accepted and ready to land.Sep 29 2023, 3:55 AM

Make the approach more consistent

lib/reducers/calendar-filters-reducer.js
142–146 ↗	(On Diff #31525)	Makes sense!
149 ↗	(On Diff #31525)	Each of the reducers returns the original object if the update doesn't change it, so it makes sense to check just the references. The result should be the same, but the current approach is less wasteful.