Page MenuHomePhabricator

[web] Move reducing logic for picking a community to its own reducer
ClosedPublic

Authored by inka on Apr 7 2023, 6:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 18, 10:52 PM
Unknown Object (File)
Sun, Dec 15, 6:35 PM
Unknown Object (File)
Wed, Dec 11, 9:28 PM
Unknown Object (File)
Wed, Dec 11, 12:06 AM
Unknown Object (File)
Wed, Dec 11, 12:06 AM
Unknown Object (File)
Wed, Dec 11, 12:06 AM
Unknown Object (File)
Wed, Dec 11, 12:06 AM
Unknown Object (File)
Sat, Dec 7, 8:36 PM
Subscribers

Details

Summary

Addressing review on D6969 - creating a separate reducer for picked communities

Test Plan

Run web app, checked that pressing a drawer item in the Calendar tab still changes the state properly

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Apr 7 2023, 7:00 AM
tomek requested changes to this revision.Apr 11 2023, 1:56 AM
tomek added inline comments.
web/redux/community-picker-reducer.js
4–6 ↗(On Diff #24779)

These imports can be merged

20–25 ↗(On Diff #24779)

Usually a reducer acts only on a single store. I guess there might be some cases where multiple stores might be reasonable, but probably not this time. The reason is that we already have reduceCalendarFilters and all the changes to calendarFilters should be made by it. So we should add handling for these actions there and make reduceCommunityPickerStore act on just one store.

This revision now requires changes to proceed.Apr 11 2023, 1:56 AM

Merge imports

web/redux/community-picker-reducer.js
20–25 ↗(On Diff #24779)

reduceCalendarFilters is in lib/, so it cannot handle these actions, which are only on web. There is no other place on web where calendarFilters are changed (reduced).

I agree we should only be changing calendar filters in one place. Can we move the logic for changing them here to lib?

tomek requested changes to this revision.Apr 13 2023, 2:30 AM

If we move actions to lib, we can simplify the reducer and avoid splitting the login in two places.

This revision now requires changes to proceed.Apr 13 2023, 2:30 AM

Move acrtions to lib, move reducing calendar filters to reduceCalendarFilters

tomek added inline comments.
web/redux/community-picker-reducer.js
13

Why do we need threadStore?

This revision is now accepted and ready to land.May 22 2023, 3:07 AM