[WEB-4979] fix: filters row missing for preset workspace views#7836
[WEB-4979] fix: filters row missing for preset workspace views#7836
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughIntroduces a memoized computation for initialWorkItemFilters in all-issue-layout-root.tsx, adding conditional handling for missing globalViewId and distinguishing static vs non-static views using STATIC_VIEW_TYPES and viewDetails. Preserves output structure, sourcing richFilters from viewDetails.rich_filters or defaulting to an empty object. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as AllIssueLayoutRoot
participant State as State/Props
participant View as ViewDetails
participant Const as STATIC_VIEW_TYPES
UI->>State: Read globalViewId, workItemFilters
UI->>View: Access viewDetails
UI->>Const: Check if view is static
alt No globalViewId
UI-->>UI: initialWorkItemFilters = undefined
else Non-static AND no viewDetails
UI-->>UI: initialWorkItemFilters = undefined
else Valid context
UI-->>UI: Build {displayFilters, displayProperties, kanbanFilters, richFilters}
end
note over UI: Memoized with deps: [globalViewId, viewDetails, workItemFilters]
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (1)
1-1: Avoid useMemo with MobX observables; drop unused import after refactor.useMemo around MobX values can freeze updates; computing inline is cheap and safer.
Apply:
-import React, { useCallback, useMemo } from "react"; +import React, { useCallback } from "react";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (1)
packages/types/src/workspace-views.ts (1)
STATIC_VIEW_TYPES(35-35)
⏰ 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). (1)
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
apps/web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (1)
7-7: Confirmed: STATIC_VIEW_TYPES usage is correct and router param is the slugSTATIC_VIEW_TYPES = ["all-issues","assigned","created","subscribed"]. Evidence: packages/types/src/workspace-views.ts:35; apps/web/core/store/issue/workspace/filter.store.ts checks STATIC_VIEW_TYPES.includes(viewId) and treats non-static IDs as saved views (lines ~138–174, 260–282); routes/hrefs use slug paths (packages/constants/src/workspace.ts:254). No changes required.
| // Determine initial work item filters based on view type and availability | ||
| const initialWorkItemFilters = useMemo(() => { | ||
| if (!globalViewId) return undefined; | ||
|
|
||
| const isStaticView = STATIC_VIEW_TYPES.includes(globalViewId); | ||
| const hasViewDetails = Boolean(viewDetails); | ||
|
|
||
| if (!isStaticView && !hasViewDetails) return undefined; | ||
|
|
||
| return { | ||
| displayFilters: workItemFilters?.displayFilters, | ||
| displayProperties: workItemFilters?.displayProperties, | ||
| kanbanFilters: workItemFilters?.kanbanFilters, | ||
| richFilters: viewDetails?.rich_filters ?? {}, | ||
| }; | ||
| }, [globalViewId, viewDetails, workItemFilters]); | ||
|
|
There was a problem hiding this comment.
MobX reactivity hazard + richFilters precedence.
- useMemo may not recompute when nested observable fields change, causing stale filters.
- Prefer store richFilters before viewDetails.rich_filters to preserve user state.
Apply:
- // Determine initial work item filters based on view type and availability
- const initialWorkItemFilters = useMemo(() => {
- if (!globalViewId) return undefined;
-
- const isStaticView = STATIC_VIEW_TYPES.includes(globalViewId);
- const hasViewDetails = Boolean(viewDetails);
-
- if (!isStaticView && !hasViewDetails) return undefined;
-
- return {
- displayFilters: workItemFilters?.displayFilters,
- displayProperties: workItemFilters?.displayProperties,
- kanbanFilters: workItemFilters?.kanbanFilters,
- richFilters: viewDetails?.rich_filters ?? {},
- };
- }, [globalViewId, viewDetails, workItemFilters]);
+ // Determine initial work item filters based on view type and availability
+ const isStaticView = globalViewId ? STATIC_VIEW_TYPES.includes(globalViewId) : false;
+ const hasViewDetails = Boolean(viewDetails);
+ const initialWorkItemFilters =
+ !globalViewId
+ ? undefined
+ : !isStaticView && !hasViewDetails
+ ? undefined
+ : {
+ displayFilters: workItemFilters?.displayFilters,
+ displayProperties: workItemFilters?.displayProperties,
+ kanbanFilters: workItemFilters?.kanbanFilters,
+ // Prefer in-store richFilters; fall back to view details; then empty object.
+ richFilters: workItemFilters?.richFilters ?? viewDetails?.rich_filters ?? {},
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine initial work item filters based on view type and availability | |
| const initialWorkItemFilters = useMemo(() => { | |
| if (!globalViewId) return undefined; | |
| const isStaticView = STATIC_VIEW_TYPES.includes(globalViewId); | |
| const hasViewDetails = Boolean(viewDetails); | |
| if (!isStaticView && !hasViewDetails) return undefined; | |
| return { | |
| displayFilters: workItemFilters?.displayFilters, | |
| displayProperties: workItemFilters?.displayProperties, | |
| kanbanFilters: workItemFilters?.kanbanFilters, | |
| richFilters: viewDetails?.rich_filters ?? {}, | |
| }; | |
| }, [globalViewId, viewDetails, workItemFilters]); | |
| // Determine initial work item filters based on view type and availability | |
| const isStaticView = globalViewId ? STATIC_VIEW_TYPES.includes(globalViewId) : false; | |
| const hasViewDetails = Boolean(viewDetails); | |
| const initialWorkItemFilters = | |
| !globalViewId | |
| ? undefined | |
| : !isStaticView && !hasViewDetails | |
| ? undefined | |
| : { | |
| displayFilters: workItemFilters?.displayFilters, | |
| displayProperties: workItemFilters?.displayProperties, | |
| kanbanFilters: workItemFilters?.kanbanFilters, | |
| // Prefer in-store richFilters; fall back to view details; then empty object. | |
| richFilters: workItemFilters?.richFilters ?? viewDetails?.rich_filters ?? {}, | |
| }; |
🤖 Prompt for AI Agents
In apps/web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx
around lines 48-64, the current useMemo can produce stale values because it
closes over nested MobX observables and doesn't recompute when those change;
also richFilters should prefer the store value over viewDetails.rich_filters.
Fix by removing the useMemo (or if you must keep it, add the observable store
fields to the dependency array) and compute initialWorkItemFilters from live
values so MobX reactivity works: derive richFilters as storeRichFilters ??
viewDetails?.rich_filters, preserve
displayFilters/displayProperties/kanbanFilters from the store, and only return
undefined when globalViewId is falsy or when a non-static view lacks
viewDetails.
Description
This PR fixes an issue where predefined workspace views (like
All,Assigned,Subscribedwork items etc) were not displaying the filters bar. The problem occurred because for preset views, viewDetails are not available from the API, causing the filters to not be properly initialized in the initialWorkItemFilters logic.Type of Change
Summary by CodeRabbit