[WEB-4321]chore: workspace views refactor #7214
Conversation
WalkthroughThis update introduces dynamic issue layout selection in workspace views, refactors issue layout root components for modularity, and adds support for conditional dependency handling in Gantt chart blocks. It also enhances type definitions, reorders method parameters for issue date updates, and improves filter logic and service endpoint selection for issue fetching. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorkspaceHeader
participant GlobalViewLayoutSelection
participant Store
User->>WorkspaceHeader: Selects layout from UI
WorkspaceHeader->>GlobalViewLayoutSelection: Renders with current layout
GlobalViewLayoutSelection-->>WorkspaceHeader: onChange(newLayout)
WorkspaceHeader->>Store: Update filters with selected layout
Store-->>WorkspaceHeader: Provides layout-specific filter options
WorkspaceHeader->>User: Renders updated layout and filters
sequenceDiagram
participant GanttChartRoot
participant ChartViewRoot
participant GanttChartMainContent
participant GanttChartBlocksList
participant GanttChartBlock
participant ChartDraggable
GanttChartRoot->>ChartViewRoot: Pass enableDependency prop
ChartViewRoot->>GanttChartMainContent: Pass enableDependency prop
GanttChartMainContent->>GanttChartBlocksList: Pass enableDependency prop
GanttChartBlocksList->>GanttChartBlock: Pass enableDependency (per block)
GanttChartBlock->>ChartDraggable: Pass enableDependency
ChartDraggable-->>GanttChartBlock: Render dependency handles conditionally
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Ruff (0.11.9)apiserver/plane/app/views/issue/base.py1017-1017: Line too long (104 > 88) (E501) 1018-1018: Line too long (95 > 88) (E501) ⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
web/core/services/workspace.service.ts (1)
265-276:⚠️ Potential issuePotential
APIService.getsignature mismatch – may cause runtime/TS errors
this.getis now invoked with three positional args (path,{ params },config).
Throughout the rest of the service (searchWorkspace,fetchWorkspaceRecents, etc.)getis called with two args (url,config), withconfigembeddingparams.Unless
APIService.get()is explicitly overloaded to accept(url, paramsWrapper, config), this change will:
- Break TypeScript checks (excess argument).
- Drop
configat runtime – Axios will treat the second param as the config object and silently ignore the third.Proposed fix – merge
paramsinto the optionalconfigand keep the 2-arg call:- return this.get( - path, - { - params, - }, - config - ) + const requestConfig = { ...(config ?? {}), params }; + return this.get(path, requestConfig);Double-check
APIService.get’s signature and add a unit test to cover both branches of the newpathselection logic.
♻️ Duplicate comments (1)
web/core/components/gantt-chart/helpers/draggable.tsx (1)
33-34: Same issue for right handle – patch together with previous diffEnsure both left and right dependency draggables apply the predicate logic; see diff above.
🧹 Nitpick comments (12)
web/core/hooks/store/use-issues.ts (1)
12-13: Mixed import roots for workspace stores
IWorkspaceIssuesis now imported from@/plane-web/store/issue/workspace/issue.store, while the correspondingIWorkspaceIssuesFilterstill comes from@/store/issue/workspace.
For consistency (and to avoid bundler duplication of the same module under two paths) consider re-exporting the filter alongside the store or updating this import to the same alias.web/core/components/gantt-chart/blocks/block.tsx (1)
23-24: DefaultenableDependencyvalue
enableDependencyis required here, but many existing call-sites might not pass it yet.
Either make the prop optional with a default offalse, or ensure every upstream component forwards a definite boolean.web/ce/store/timeline/base-timeline.store.ts (1)
198-199: Minor optimisation suggestionBecause
project_idwill rarely change, you could skip the deep-compare for it inside the update loop to spare a fewisEqualcycles per frame:- const currValue = this.blocksMap[blockId][key as keyof IGanttBlock]; + if (key === "project_id") continue; // id is immutable + const currValue = this.blocksMap[blockId][key as keyof IGanttBlock];Not critical, but the inner loop runs often while dragging large datasets.
web/core/components/gantt-chart/chart/main-content.tsx (1)
49-50: Prop added – consider defaulting behaviour
enableDependencyis optional yet immediately destructured. If a parent forgets to pass it, its value isundefinedwhich is falsy – fine – but an explicit default (e.g.const { enableDependency = false } = props;) documents intent and reduces undefined checks downstream.web/core/components/gantt-chart/chart/root.tsx (1)
40-41: Prop tubing works – guard against backwards-compatOlder callers of
ChartViewRootwon’t provideenableDependency; TypeScript will catch missing required props, but if any JS callers exist consider marking the prop optional (enableDependency?: …) here as well.web/core/services/workspace.service.ts (1)
266-268: Minor: guard against non-arrayparams.expand
includesexists on bothstringandArray, but the intent seems “array contains value”.
For clarity & type-safety:const hasRelation = Array.isArray(params.expand) && params.expand.includes("issue_relation"); const path = `/api/workspaces/${workspaceSlug}/${hasRelation ? "issues-detail" : "issues"}/`;Not blocking, but improves readability and avoids false positives when
expandis a comma-separated string.web/core/store/issue/issue-details/relation.store.ts (1)
185-188: Fallback if issue metadata is not yet hydrated
projectIdis now derived from the issue cache. When the relation is created before the issue is fetched, the earlyreturnsilently does nothing.Consider surfacing an explicit error or awaiting issue hydration to avoid UX confusion (the “add relation” UI pretends nothing happened).
web/core/components/views/helper.tsx (1)
20-51: Nice centralisation of layout switchingClean switch statement and prop forwarding.
Future-proofing idea: wrap the component inReact.memoto avoid unnecessary re-renders when unrelated props mutate.web/core/components/gantt-chart/types/index.ts (1)
25-31: Redundantsort_orderre-declaration inIBlockUpdatePayload
IBlockUpdateDataalready declares an optionalsort_orderfield with exactly the same shape.
Re-adding the property provides no extra type-safety and only bloats the alias while risking divergence if one definition is ever updated.-export type IBlockUpdatePayload = IBlockUpdateData & { - sort_order?: { - destinationIndex: number; - newSortOrder: number; - sourceIndex: number; - }; -}; +export type IBlockUpdatePayload = IBlockUpdateData;web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (1)
1-17: Minor: Organize imports for better readabilityConsider grouping imports more consistently:
import React, { useCallback } from "react"; import { isEmpty } from "lodash"; import { observer } from "mobx-react"; import { useParams, useSearchParams } from "next/navigation"; -// plane constants import useSWR from "swr"; +// plane constants import { EIssueFilterType, EIssueLayoutTypes, EIssuesStoreType, ISSUE_DISPLAY_FILTERS_BY_PAGE } from "@plane/constants"; -// hooks // components -// hooks import { EmptyState } from "@/components/common"; import { WorkspaceActiveLayout } from "@/components/views/helper"; +// hooks import { useGlobalView, useIssues } from "@/hooks/store"; import { useAppRouter } from "@/hooks/use-app-router"; import { useWorkspaceIssueProperties } from "@/hooks/use-workspace-issue-properties"; -// store +// assets import emptyView from "@/public/empty-state/view.svg";web/core/components/issues/issue-layouts/spreadsheet/roots/workspace-root.tsx (2)
1-26: Clean up duplicate import section commentsimport React, { useCallback } from "react"; import { observer } from "mobx-react"; // plane constants import { ALL_ISSUES, EIssueLayoutTypes, EIssueFilterType, EIssuesStoreType, EUserPermissions, EUserPermissionsLevel, } from "@plane/constants"; import { IIssueDisplayFilterOptions } from "@plane/types"; -// hooks // components import { SpreadsheetView } from "@/components/issues/issue-layouts"; import { AllIssueQuickActions } from "@/components/issues/issue-layouts/quick-action-dropdowns"; import { SpreadsheetLayoutLoader } from "@/components/ui"; // hooks import { useIssues, useUserPermissions } from "@/hooks/store"; import { IssuesStoreContext } from "@/hooks/use-issue-layout-store"; import { useIssuesActions } from "@/hooks/use-issues-actions"; import { useWorkspaceIssueProperties } from "@/hooks/use-workspace-issue-properties"; -// store +// components import { IssuePeekOverview } from "../../../peek-overview"; import { IssueLayoutHOC } from "../../issue-layout-HOC"; +// types import { TRenderQuickActions } from "../../list/list-view-types";
90-105: Use optional chaining for cleaner codeThe static analysis correctly identifies that optional chaining would be cleaner here.
const renderQuickActions: TRenderQuickActions = useCallback( ({ issue, parentRef, customActionButton, placement, portalElement }) => ( <AllIssueQuickActions parentRef={parentRef} customActionButton={customActionButton} issue={issue} handleDelete={async () => removeIssue(issue.project_id, issue.id)} - handleUpdate={async (data) => updateIssue && updateIssue(issue.project_id, issue.id, data)} - handleArchive={async () => archiveIssue && archiveIssue(issue.project_id, issue.id)} + handleUpdate={async (data) => updateIssue?.(issue.project_id, issue.id, data)} + handleArchive={async () => archiveIssue?.(issue.project_id, issue.id)} portalElement={portalElement} readOnly={!canEditProperties(issue.project_id ?? undefined)} placements={placement} /> ), [canEditProperties, removeIssue, updateIssue, archiveIssue] );🧰 Tools
🪛 Biome (1.9.4)
[error] 97-97: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx(6 hunks)web/ce/components/views/helper.tsx(1 hunks)web/ce/store/issue/workspace/issue.store.ts(1 hunks)web/ce/store/timeline/base-timeline.store.ts(2 hunks)web/core/components/gantt-chart/blocks/block.tsx(3 hunks)web/core/components/gantt-chart/blocks/blocks-list.tsx(3 hunks)web/core/components/gantt-chart/chart/main-content.tsx(3 hunks)web/core/components/gantt-chart/chart/root.tsx(3 hunks)web/core/components/gantt-chart/helpers/draggable.tsx(3 hunks)web/core/components/gantt-chart/root.tsx(3 hunks)web/core/components/gantt-chart/types/index.ts(2 hunks)web/core/components/issues/issue-layouts/gantt/base-gantt-root.tsx(1 hunks)web/core/components/issues/issue-layouts/properties/labels.tsx(1 hunks)web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx(6 hunks)web/core/components/issues/issue-layouts/spreadsheet/roots/workspace-root.tsx(1 hunks)web/core/components/views/helper.tsx(1 hunks)web/core/hooks/store/use-issues.ts(1 hunks)web/core/services/workspace.service.ts(1 hunks)web/core/store/issue/helpers/base-issues.store.ts(2 hunks)web/core/store/issue/issue-details/relation.store.ts(1 hunks)web/core/store/issue/root.store.ts(2 hunks)web/core/store/issue/workspace/filter.store.ts(1 hunks)web/helpers/issue.helper.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
web/core/components/views/helper.tsx (2)
web/core/components/issues/issue-layouts/spreadsheet/roots/workspace-root.tsx (1)
WorkspaceSpreadsheetRoot(42-137)web/ce/components/views/helper.tsx (1)
WorkspaceAdditionalLayouts(12-12)
web/ce/components/views/helper.tsx (1)
web/core/components/views/helper.tsx (1)
TWorkspaceLayoutProps(5-18)
web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (5)
web/core/hooks/store/use-issues.ts (1)
useIssues(83-158)web/core/hooks/use-workspace-issue-properties.ts (1)
useWorkspaceIssueProperties(7-46)web/core/store/router.store.ts (2)
workspaceSlug(69-71)globalViewId(117-119)web/core/store/issue/workspace/filter.store.ts (1)
issueFilters(109-112)web/core/components/views/helper.tsx (1)
WorkspaceActiveLayout(20-51)
web/core/store/issue/helpers/base-issues.store.ts (2)
web/core/store/router.store.ts (1)
projectId(85-87)web/core/components/gantt-chart/types/index.ts (1)
IBlockUpdateDependencyData(33-38)
🪛 Biome (1.9.4)
web/core/components/issues/issue-layouts/spreadsheet/roots/workspace-root.tsx
[error] 97-97: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (14)
web/ce/store/issue/workspace/issue.store.ts (1)
1-1: Path alias consistency checkGood call adding a thin re-export; it shields callers from the path shuffle.
Double-check thattsconfig.paths(and Jest/Vite aliases, if any) map@/store/*identically in both CE and EE bundles so we don’t create a split-brain import situation at build time.web/core/components/gantt-chart/blocks/block.tsx (1)
95-96: Prop forwarding verifiedForwarding
enableDependencydown toChartDraggablecompletes the dependency-toggle plumbing – looks correct.web/core/store/issue/root.store.ts (2)
17-17: Import path refactor looks goodPulling
IWorkspaceIssuesandWorkspaceIssuesdirectly fromworkspace/issue.storeremoves an unnecessary re-export layer and makes the dependency graph clearer. 👍
36-36: Sanity-check build after the split import
WorkspaceIssuesFilterstill comes from./workspace, while the interfaces now live inworkspace/issue.store.
Make sure the barrel file in./workspaceno longer re-exportsIWorkspaceIssues/WorkspaceIssues; otherwise duplicate‐identifier errors may slip past CI depending on ts-configpaths.web/ce/store/timeline/base-timeline.store.ts (1)
25-26: Type extension acknowledged – verify downstream typingsAdding
project_idtoBlockDatais a non-breaking, optional extension. Ensure the correspondingIGanttBlockdefinition has been updated everywhere (IDE still finds only one declaration?) to avoid structural type mismatches during assignment.web/core/components/gantt-chart/chart/main-content.tsx (1)
225-226: Forwarding prop ✅ – double-check list component signature
GanttChartBlocksListnow receivesenableDependency, but its prop definition wasn’t shown in this diff. Make sure the new boolean/function union is reflected there; TS won’t complain if the component is untyped.web/core/components/gantt-chart/chart/root.tsx (2)
74-75: Pipeline: verify prop plumbed from GanttChartRoot
GanttChartRoot(outer wrapper) must now accept & forwardenableDependency; ensure its public API and docs are updated, otherwise consumers compiling against previous types will break.
209-210: Prop forwarding looks correctThe prop is forwarded to
GanttChartMainContent; once upstream changes are in place no further action is required here. 👍web/core/components/gantt-chart/root.tsx (1)
26-51: Prop plumbing looks good – verify downstream types
enableDependencyis threaded through with a sensible default.
Please confirmChartViewRoot’s prop definition has been updated; otherwise TypeScript will complain or the value will be silently dropped.web/core/components/gantt-chart/blocks/blocks-list.tsx (1)
15-47: 👍 Dependency enablement correctly cascadedGood use of the existing pattern (
typeof === "function" ? fn(id) : bool). Implementation is consistent with other feature toggles.web/core/components/gantt-chart/types/index.ts (1)
12-12:project_idaddition looks goodIncluding
project_idon the block interface keeps timeline data project-aware and is consistent with other fields. No issues spotted.web/helpers/issue.helper.ts (2)
174-182: Project context propagated correctly
project_idis now carried into the Gantt block structure – good consistency!
254-258: Possibleundefinedleak after new emptiness checkWhen both
displayFiltersis an empty object anddefaultValuesisundefined,filtersbecomesundefined, yet the subsequent code dereferences it (filters?.calendar…).
While optional-chaining prevents crashes, default fallbacks will all evaluate toundefined, which differs from the previous behaviour that returned explicit defaults.Consider defaulting to an empty object to preserve previous semantics:
-const filters = !isEmpty(displayFilters) ? displayFilters : defaultValues; +const filters = !isEmpty(displayFilters) ? displayFilters : (defaultValues ?? {});web/core/components/issues/issue-layouts/roots/all-issue-layout-root.tsx (1)
135-148: Excellent refactoring for improved modularity!The delegation of layout rendering to
WorkspaceActiveLayoutsignificantly improves the separation of concerns. This component now focuses solely on data fetching and filter management while letting specialized components handle the rendering logic.
web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx
Outdated
Show resolved
Hide resolved
web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/app/views/issue/base.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
apiserver/plane/app/views/issue/base.py
1018-1018: Line too long (104 > 88)
(E501)
1019-1019: Line too long (95 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
Description
This update refactors workspace views for better modularity.
Type of Change
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Chores