Page MenuHomePhabricator

[web] Add calendar filter type for displaying only one community
ClosedPublic

Authored by inka on Jan 30 2023, 1:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 3, 2:16 PM
Unknown Object (File)
Wed, Apr 3, 2:16 PM
Unknown Object (File)
Wed, Apr 3, 2:15 PM
Unknown Object (File)
Wed, Apr 3, 2:15 PM
Unknown Object (File)
Wed, Apr 3, 2:15 PM
Unknown Object (File)
Wed, Apr 3, 2:15 PM
Unknown Object (File)
Wed, Apr 3, 2:15 PM
Unknown Object (File)
Wed, Apr 3, 2:15 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
We want to be able to press some button, and see only chats from a given community to be visible in the calendar filters. We want this information to persist when switching between tabs. So we will keep
this as a calendar filter in redux.
This diff introduces the architectural changes required for this - adding and changing types according to what will be needed.

Test Plan

Run web app, check that calendar filters work like they used to.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Jan 30 2023, 2:12 AM

Are keyserver changes strictly necessary for this? I think we could do this purely on the client, but open to arguments for why we need keyserver support

Also not clear to me why we need a new filter type. I guess the thinking is maybe that we would otherwise need to edit the filters whenever a new chat inside the community is created?

I'm not sure if this is necessary to have this filter on keyserver, but it also shouldn't hurt.

lib/types/filter-types.js
16 ↗(On Diff #21555)

What do you think about making it more explicit that this is a community?

This revision is now accepted and ready to land.Jan 31 2023, 8:38 AM
ashoat requested changes to this revision.Jan 31 2023, 11:54 AM

I'm not sure if this is necessary to have this filter on keyserver, but it also shouldn't hurt.

It hurts by introducing complexity and maintenance cost. I think we should have a good reason for why we're introducing this if we're going to do it. Passing back to @inka to discuss tradeoffs and why it makes sense to introduce this

This revision now requires changes to proceed.Jan 31 2023, 11:54 AM

Don't send community filter to the keyserver

Move types used only on web out of lib

Undo accidental changes (meant for the next diff)

Cool, glad to see the filter removed from the server side – defer to others on the rest

This revision is now accepted and ready to land.Feb 7 2023, 4:58 AM
inka requested review of this revision.Feb 7 2023, 5:05 AM

Keep only the community id in redux state

This revision is now accepted and ready to land.Feb 8 2023, 1:58 AM

If this prop is specific to calendars, should we name it something more specific? It's in the global Redux state

If this prop is specific to calendars, should we name it something more specific? It's in the global Redux state

Changing this by rebasing this whole stack and changing it separately in each diff would take me over half an hour, so I hope it's fine if I do it in a separate diff - that will take me about 2min.
I know that generally we want to amend diffs, but I feel like spending so much time on changing a name would be wasteful. Please let me know if that's a wrong assumption