Page MenuHomePhabricator

[web] Add to calendar filters reducer logic for handling the COMMUNITY filters
ClosedPublic

Authored by inka on Feb 7 2023, 4:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 11:36 AM
Unknown Object (File)
Tue, Nov 12, 10:52 AM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Unknown Object (File)
Thu, Nov 7, 4:04 PM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-2794/make-it-possible-to-display-only-chats-from-a-given-community-in-the
In D6457 I added architectural changes that allow for a new type of calendar filter. In this diff I add logic to the web reducer, that will handle setting this state.
This used to be diff D6458, but I messed up somethin in my local git, and can't get that commit back, sorry.

Test Plan

Dispatch updateCalendarCommunityFilter and clearCalendarCommunityFilter actions, check in redux tools that they succeed, and redux state changes as expected.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Feb 7 2023, 5:07 AM

MAke clearCalendarCommunityFilter action set communityFilter to null to make the code more logical

Changes due to: chainng what is kept in the redux state - keep only the community id

web/redux/redux-setup.js
134–137 ↗(On Diff #22260)

We reset the threads filter, so that only chats from the picked community are picked, and all of them are picked.

145 ↗(On Diff #22260)

We reset the threads filter, so that all chats are picked

I think the order of diffs in this stack is a little bit mixed, here you use filteredCommunityThreadIDs which is introduced in child revision, but overall looks good

web/redux/redux-setup.js
20 ↗(On Diff #22260)

can be merged with line 19

136 ↗(On Diff #22260)

can be shortened

145 ↗(On Diff #22260)

Is the destruction needed? Can it be just calendarFilters: nonThreadFilters?

This revision is now accepted and ready to land.Feb 8 2023, 7:56 AM
In D6645#198370, @kamil wrote:

I think the order of diffs in this stack is a little bit mixed, here you use filteredCommunityThreadIDs which is introduced in child revision, but overall looks good

@inka encourage you to think critically about your Git workflow... I suspect you have multiple branches for a single stack or something like that. You should generally have one branch per stack and one local commit per diff

@inka encourage you to think critically about your Git workflow... I suspect you have multiple branches for a single stack or something like that. You should generally have one branch per stack and one local commit per diff

What happened here is that these commits were in the order you just saw them in my diff stack, but due to changes in logic i had to change their order. And I simply forgot to change it in the diff stack as well, will fix that now.
I actually just have a one, straight branch in my local copy, and one commit per diff. What happened lately with the aborted diff was that I accidentally dropped it during a rebase, and git cherry-pick was failing for some reason. So I created a new one, because I didn't know about the other possibilities that @ashoat explained to me later. I'll do better next time :)

This revision now requires review to proceed.Feb 9 2023, 1:35 AM
This revision is now accepted and ready to land.Feb 9 2023, 5:42 AM
In D6645#198420, @inka wrote:

@inka encourage you to think critically about your Git workflow... I suspect you have multiple branches for a single stack or something like that. You should generally have one branch per stack and one local commit per diff

What happened here is that these commits were in the order you just saw them in my diff stack, but due to changes in logic i had to change their order. And I simply forgot to change it in the diff stack as well, will fix that now.
I actually just have a one, straight branch in my local copy, and one commit per diff. What happened lately with the aborted diff was that I accidentally dropped it during a rebase, and git cherry-pick was failing for some reason. So I created a new one, because I didn't know about the other possibilities that @ashoat explained to me later. I'll do better next time :)

Thanks for explaining!