-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: move filter state management to zustand store #2258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
✅ Deploy Preview for atlassify canceled.
|
Signed-off-by: Adam Setch <adam.setch@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Refactors notification filter state to live in a persisted zustand store (instead of being part of SettingsState / AppContext), and updates filtering logic + UI routes/components/tests to use the new store-backed filter state.
Changes:
- Introduce
useFiltersStore(zustand + persist) and persist filter selections under a new storage key. - Refactor notification filtering utilities and filter implementations to read filter state from the store.
- Update filter-related UI (Filters route/sections, sidebar/all-read) and tests to use the new store model.
Reviewed changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/utils/notifications/notifications.ts | Switch filtering callsite to filterNotifications(notifications) (store-backed). |
| src/renderer/utils/notifications/filters/types.ts | Update filter interface to remove SettingsState dependency and improve docs. |
| src/renderer/utils/notifications/filters/readState.ts | Read read-state filters from useFiltersStore. |
| src/renderer/utils/notifications/filters/readState.test.ts | Update tests to use store-based filters (but mocks need correction). |
| src/renderer/utils/notifications/filters/product.ts | Read product filters from useFiltersStore. |
| src/renderer/utils/notifications/filters/product.test.ts | Update tests to use store-based filters (but mocks need correction). |
| src/renderer/utils/notifications/filters/filter.ts | Refactor filtering/active-filter detection to use the zustand store. |
| src/renderer/utils/notifications/filters/filter.test.ts | Update filter tests for store model (but mocks need correction). |
| src/renderer/utils/notifications/filters/engagement.ts | Read engagement filters from useFiltersStore. |
| src/renderer/utils/notifications/filters/engagement.test.ts | Update tests to use store-based filters (but mocks need correction). |
| src/renderer/utils/notifications/filters/category.ts | Read category filters from useFiltersStore. |
| src/renderer/utils/notifications/filters/category.test.ts | Update tests to use store-based filters (but mocks need correction). |
| src/renderer/utils/notifications/filters/actor.ts | Read actor filters from useFiltersStore. |
| src/renderer/utils/notifications/filters/actor.test.ts | Update tests to use store-based filters (but mocks need correction). |
| src/renderer/utils/api/graphql/generated/graphql.ts | Regenerated GraphQL types/enums. |
| src/renderer/types.ts | Remove filter settings from SettingsState (but type cleanup needed). |
| src/renderer/stores/useFiltersStore.ts | New zustand store + persist for filter state. |
| src/renderer/routes/Filters.tsx | Use store-backed clearFilters and new filter keys. |
| src/renderer/routes/Filters.test.tsx | Update route tests to assert store action calls. |
| src/renderer/context/defaults.ts | Remove default filter settings from settings defaults. |
| src/renderer/context/App.tsx | Remove filter actions from context; trigger fetches based on store filter slices. |
| src/renderer/context/App.test.tsx | Remove tests for removed context filter methods. |
| src/renderer/constants.ts | Add FILTERS_STORE_KEY for persisted filter state. |
| src/renderer/components/filters/FilterSection.tsx | Use store-backed updateFilter and store-based isFilterSet. |
| src/renderer/components/filters/FilterSection.test.tsx | Update tests to use store-based action spying. |
| src/renderer/components/Sidebar.tsx | Use hasActiveFilters() without settings param (store-backed). |
| src/renderer/components/Sidebar.test.tsx | Update tests for store-based active filters (but mocks need correction). |
| src/renderer/components/AllRead.tsx | Use hasActiveFilters() without settings param (store-backed). |
| src/renderer/components/AllRead.test.tsx | Update tests for store-based active filters (but mocks need correction). |
| src/renderer/mocks/state-mocks.ts | Remove filter fields from mocked settings. |
| src/renderer/helpers/test-utils.tsx | Add mockFilterStoreState helper for stubbing filter store state. |
| pnpm-lock.yaml | Add zustand dependency lock entries. |
| package.json | Add zustand dependency. |
| .vscode/settings.json | Add partialize to cSpell words. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
src/renderer/components/filters/FilterSection.tsx:53
FilterSectionisn’t subscribed to the filters store state, soisCheckedis computed once fromuseFiltersStore.getState()(viafilter.isFilterSet) and won’t update afterupdateFilterruns. This can leave the checkbox UI stale and can repeatedly dispatch the wrongcheckedvalue (because!isCheckedis based on an out-of-date render). Subscribe to the relevant slice (e.g.useFiltersStore(s => s[filterSetting])) and deriveisCheckedfrom that, or pass the selected values into the filter instead of having filters read the store directly.
const { notifications } = useAppContext();
const updateFilter = useFiltersStore((s) => s.updateFilter);
return (
<Stack space="space.050">
<Heading size="small">{title}</Heading>
<Box>
{(Object.keys(filter.FILTER_TYPES) as T[]).map((type) => {
const typeDetails = filter.getTypeDetails(type);
const typeLabel = formatProperCase(typeDetails.name);
const isChecked = filter.isFilterSet(type);
const count = filter.getFilterCount(notifications, type);
return (
<Inline
alignBlock="center"
key={typeDetails.name}
space="space.050"
>
<Checkbox
aria-label={typeDetails.name}
isChecked={isChecked}
label={typeLabel}
onChange={() => updateFilter(filterSetting, type, !isChecked)}
/>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| interface FilterActions { | ||
| setFilters: (next: FiltersState) => void; | ||
| updateFilter: ( | ||
| key: keyof FiltersState, | ||
| value: any, | ||
| checked?: boolean, | ||
| ) => void; | ||
| clearFilters: () => void; | ||
| } | ||
|
|
||
| export const useFiltersStore = create<FiltersState & FilterActions>()( | ||
| persist( | ||
| (set, get, store) => ({ | ||
| ...defaultFiltersState, | ||
|
|
||
| setFilters: (next) => { | ||
| set(next); | ||
| }, | ||
|
|
||
| updateFilter: (key, value, checked) => { | ||
| const current = (get() as any)[key] || []; | ||
| const updated = | ||
| typeof checked === 'boolean' | ||
| ? checked | ||
| ? [...current, value] | ||
| : current.filter((item: any) => item !== value) | ||
| : value; | ||
|
|
||
| set({ [key]: updated }); | ||
| }, |
Copilot
AI
Feb 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateFilter uses any for value and builds the next array with [...] which can introduce duplicates. This weakens type-safety and can bloat persisted state. Consider typing updateFilter generically (value as FiltersState[K][number] | FiltersState[K]) and, when checked === true, avoid adding the value if it already exists.
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
Signed-off-by: Adam Setch <adam.setch@outlook.com>
|



No description provided.