[WEB-4321]chore: workspace views refactor #7214
Conversation
WalkthroughThis update introduces layout-aware filtering and dependency management for workspace issue views, particularly enhancing Gantt chart and spreadsheet layouts. It adds new props and types for layout and dependency control, refactors several components to support dynamic layouts, and implements conditional rendering and logic for issue relations and permissions. Backend and utility changes support these features with improved data structures and access control. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GlobalIssuesHeader
participant GlobalViewLayoutSelection
participant WorkspaceActiveLayout
participant WorkspaceSpreadsheetRoot
participant GanttChartRoot
participant ChartViewRoot
participant GanttChartMainContent
participant GanttChartBlocksList
participant GanttChartBlock
participant ChartDraggable
User->>GlobalIssuesHeader: Selects layout
GlobalIssuesHeader->>GlobalViewLayoutSelection: Renders layout selector
GlobalViewLayoutSelection-->>GlobalIssuesHeader: onChange(layout)
GlobalIssuesHeader->>WorkspaceActiveLayout: Renders with activeLayout
WorkspaceActiveLayout->>WorkspaceSpreadsheetRoot: If layout == SPREADSHEET
WorkspaceActiveLayout->>WorkspaceAdditionalLayouts: If other layout
WorkspaceSpreadsheetRoot->>GanttChartRoot: Renders Gantt chart with enableDependency
GanttChartRoot->>ChartViewRoot: Passes enableDependency
ChartViewRoot->>GanttChartMainContent: Passes enableDependency
GanttChartMainContent->>GanttChartBlocksList: Passes enableDependency
GanttChartBlocksList->>GanttChartBlock: Passes enableDependency (per block)
GanttChartBlock->>ChartDraggable: Passes enableDependency
ChartDraggable->>ChartDraggable: Conditionally render dependency handles
Suggested labels
Suggested reviewers
Poem
✨ 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)
…to chore-workspace_views
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/utils/src/work-item/base.ts (1)
264-264: Potential undefined fallback when both sources are empty
filtersmay becomeundefinedifdisplayFiltersis empty anddefaultValuesisundefined.
Although optional chaining prevents runtime errors, it silently returns defaults likefalse/null, which may mask configuration mistakes.-const filters = !isEmpty(displayFilters) ? displayFilters : defaultValues; +const filters = !isEmpty(displayFilters) ? displayFilters : (defaultValues ?? {});This guarantees an object and makes the intent explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/types/src/layout/gantt.d.ts(2 hunks)packages/utils/src/work-item/base.ts(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx(6 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)
🚧 Files skipped from review as they are similar to previous changes (8)
- web/core/components/gantt-chart/blocks/block.tsx
- web/core/components/gantt-chart/chart/root.tsx
- web/core/components/gantt-chart/chart/main-content.tsx
- web/ce/store/timeline/base-timeline.store.ts
- web/core/components/gantt-chart/root.tsx
- web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx
- web/core/components/gantt-chart/blocks/blocks-list.tsx
- web/core/components/gantt-chart/helpers/draggable.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
packages/types/src/layout/gantt.d.ts (1)
12-13: Project-scoped blocks 👍Adding the optional
project_idfield toIGanttBlockimproves multi-project support and keeps the type backward-compatible.packages/utils/src/work-item/base.ts (1)
188-189: Propagatingproject_id– good, but verify downstream usage
getIssueBlocksStructurenow returnsproject_id. Ensure any existing consumers that destructureIGanttBlock(e.g. timeline stores, drag-and-drop helpers) account for the new key and don’t rely on...restpatterns that exclude it.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apiserver/plane/app/views/issue/base.py (1)
950-973: Consider simplifying the complex query structure as suggested in past reviews.While the role-based filtering logic is now correct (addressing the previous reversed comparison issue), the implementation still adds significant complexity to an already heavy query. The subquery approach, while functional, makes the code harder to maintain and potentially less performant.
As suggested by dheeru0198 in the past review, consider this simpler approach:
# Get project and user's role in a separate, simple query project = Project.objects.select_related().get(pk=project_id, workspace__slug=slug) user_project_member = ProjectMember.objects.filter( project_id=project_id, member=self.request.user, is_active=True ).first() # Apply simple filtering based on role issue_queryset = Issue.issue_objects.filter(workspace__slug=slug, project_id=project_id) # If user is guest with restricted access, filter to their own issues if (user_project_member and user_project_member.role == ROLE.GUEST.value and not project.guest_view_all_features): issue_queryset = issue_queryset.filter(created_by=self.request.user) # Continue with the rest of the query... issue = issue_queryset.prefetch_related(...)This approach:
- Separates access control logic from the main query
- Is easier to understand and maintain
- Avoids complex subqueries and OR conditions
- Performs better by reducing query complexity
🧹 Nitpick comments (1)
apiserver/plane/app/views/issue/base.py (1)
948-949: Fix line length violations flagged by static analysis.The lines exceed the 88-character limit. Consider breaking the comments into multiple lines.
- # check for the project member role, if the role is 5 then check for the guest_view_all_features - # if it is true then show all the issues else show only the issues created by the user + # check for the project member role, if the role is 5 then check for the + # guest_view_all_features if it is true then show all the issues else + # show only the issues created by the user
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/app/views/issue/base.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
apiserver/plane/app/views/issue/base.py
948-948: Line too long (104 > 88)
(E501)
949-949: Line too long (95 > 88)
(E501)
🔇 Additional comments (1)
apiserver/plane/app/views/issue/base.py (1)
955-958: Verify the role comparison logic is correct.The current logic uses
Q(role__gt=ROLE.GUEST.value)which appears correct for showing all issues to users with roles higher than GUEST (ADMIN/MEMBER). This addresses the previous issue where the comparison was reversed.The access control logic correctly handles:
- ADMIN/MEMBER users (role > GUEST): See all issues
- GUEST users with
guest_view_all_features=True: See all issues- GUEST users with
guest_view_all_features=False: See only their own issues
Description
This update refactors workspace views for better modularity.
Type of Change
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor