-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-5156] fix: activity filters bug #7971
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
WalkthroughThe UI toggle logic for issue activity filters now deduplicates selections. In shared constants, the DEFAULT activity filter type and its inclusion in defaultActivityFilters were removed. No public API additions; EActivityFilterType lost the DEFAULT member. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as IssueActivity UI
participant S as State: filterValue
participant F as Activity Feed
U->>C: Click filter toggle
C->>C: Build updated _filters
C->>C: Deduplicate with uniq(_filters)
C->>S: setFilterValue(deduped)
S-->>F: Updated filters
F-->>U: Render filtered activity
note over C,S: Change: apply uniq before state update
sequenceDiagram
participant UI as IssueActivity UI
participant K as constants.defaultActivityFilters
participant E as EActivityFilterType
UI->>K: Load initial filters
K-->>UI: Array without DEFAULT
UI->>E: Reference available types
E-->>UI: Enum (DEFAULT removed)
note over K,E: DEFAULT option removed from defaults and enum
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/core/components/issues/issue-detail/issue-activity/root.tsx (1)
69-80: Defensive deduplication is reasonable, but consider explicit migration.The
uniqcall safely handles duplicates from stale localStorage (e.g., removedDEFAULTfilter) or other sources. However, since thetoggleFilterlogic itself shouldn't create duplicates (it checksincludes()before adding), consider adding an explicit one-time migration to clean up localStorage on component mount.Optional: Add a migration effect to clean up legacy filter state:
useEffect(() => { if (selectedFilters) { const cleaned = uniq(selectedFilters.filter(f => Object.values(EActivityFilterType).includes(f) )); if (cleaned.length !== selectedFilters.length || cleaned.some((f, i) => f !== selectedFilters[i])) { setFilterValue(cleaned); } } }, []); // Run once on mountThis would proactively remove invalid/duplicate filters rather than relying on user interaction to trigger cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/issues/issue-detail/issue-activity/root.tsx(2 hunks)packages/constants/src/issue/filter.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/constants/src/issue/filter.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
PR: makeplane/plane#7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/issues/issue-detail/issue-activity/root.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (2)
apps/web/core/components/issues/issue-detail/issue-activity/root.tsx (2)
5-5: LGTM: Tree-shakeable import follows best practices.The per-function import from
lodash-es/uniqis optimal for bundle size.Based on learnings.
79-79: uniq() on setFilterValue for issue_activity_filters covers all writes; no other filter manipulations found.
Description
This update fixes the duplicate issue in activity filters.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Bug Fixes