[WEB-3838]feat:sub work items sorting#6967
Conversation
WalkthroughThis change set implements a comprehensive refactor and enhancement of the sub-issues functionality within an issue tracking system. Key updates include the introduction of dynamic ordering and grouping for sub-issues retrieval on the backend, the addition of new constants and types for sub-issue display properties, and significant frontend changes. The frontend now features new and restructured React components for sub-issue lists, display filters, and property editing, with MobX stores managing filter and grouping state. Several obsolete files and components were removed, and new hooks and utilities were introduced for improved clipboard operations and filter management. The API and store layers were updated to support flexible query parameters, grouping, and filtering of sub-issues. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SubIssuesEndpoint (API)
participant IssueService
participant SubIssuesStore
participant FiltersStore
Client->>SubIssuesEndpoint: GET /sub-issues?order_by=...&group_by=...
SubIssuesEndpoint->>IssueService: Fetch sub-issues with order/group params
IssueService->>SubIssuesEndpoint: Return ordered/grouped sub-issues
SubIssuesEndpoint-->>Client: Respond with list or grouped dict
Client->>SubIssuesStore: fetchSubIssues(parentId)
SubIssuesStore->>FiltersStore: Get filter params for parentId
SubIssuesStore->>IssueService: subIssues(..., queries)
IssueService-->>SubIssuesStore: sub-issues response
SubIssuesStore->>FiltersStore: processSubIssueResponse(response)
FiltersStore-->>SubIssuesStore: { issueList, groupedIssues }
SubIssuesStore-->>Client: Update observable sub-issues/grouped state
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ 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:
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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (4)
web/core/store/issue/issue-details/sub_issues.store.ts (2)
91-103: 🛠️ Refactor suggestionMobX annotation mismatch for
groupedSubIssuesByIssueId
groupedSubIssuesByIssueIdis a getter that does not mutate state.
Annotating it as anactionprevents MobX from memoising the value and causes
unnecessary re-executions. Prefercomputed, or drop the annotation entirely:makeObservable(this, { … - groupedSubIssuesByIssueId: action, + groupedSubIssuesByIssueId: computed, });Also consider turning
groupedSubIssuesMapinto anobservable.refif the map
is replaced wholesale, or anobservable.shallowif only its keys change.
140-175: 🛠️ Refactor suggestion
⚠️ Potential issueLoader & grouped-count state never updated – UX regressions
this.loaderis declared (observable.ref) but never set to"loading" | "success" | "error". The UI will no longer show spinners or error states.
groupedSubIssuesCountis declared but never populated; any badge/summary
that relies on it will always show0.
issueListcan beundefinedwhenprocessSubIssueResponsereturns only
grouped data, yet it is passed directly toaddIssue. Guard against this
to prevent a runtime exception.Quick fix:
- const { issueList, groupedIssues } = this.filters.processSubIssueResponse(response.sub_issues); + const { issueList = [], groupedIssues, groupedCount } = + this.filters.processSubIssueResponse(response.sub_issues); // set grouped issues count set(this.groupedSubIssuesMap, [parentIssueId], groupedIssues); + set(this.groupedSubIssuesCount, [parentIssueId], groupedCount); - this.rootIssueDetailStore.rootIssueStore.issues.addIssue(issueList); + if (issueList.length) { + this.rootIssueDetailStore.rootIssueStore.issues.addIssue(issueList); + } + this.loader = "success";…and remember to set
this.loader = "loading"before the API call and
"error"insidecatch.web/core/components/issues/issue-detail-widgets/sub-issues/helper.ts (2)
196-211: 🛠️ Refactor suggestionLoader never cleared on failed “remove sub-issue”
Inside the
catchclause the helper flag set with
setSubIssueHelpers(parentIssueId, "issue_loader", issueId)is not reset, leaving the UI stuck in a loading state after an error.} catch { + // clear loader so UI doesn’t remain in a busy state + setSubIssueHelpers(parentIssueId, "issue_loader", issueId);
224-235: 🛠️ Refactor suggestionSame loader-reset issue in
deleteSubIssueReplicate the fix suggested above to keep behaviour consistent.
🧹 Nitpick comments (3)
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx (1)
17-70: Component implementation is well-structuredThe component follows React best practices with proper props destructuring and conditional rendering based on the available display filters and properties. The dropdown menu implementation with stopping event propagation is a good approach.
One suggestion: The className string is quite long - consider extracting it to a constant or using a utility like
cn()for better readability.- className="vertical-scrollbar scrollbar-sm relative h-full w-full divide-y divide-custom-border-200 overflow-hidden overflow-y-auto px-2.5 max-h-[25rem] text-left" + className={cn( + "vertical-scrollbar scrollbar-sm relative", + "h-full w-full max-h-[25rem]", + "divide-y divide-custom-border-200", + "overflow-hidden overflow-y-auto", + "px-2.5 text-left" + )}web/core/components/issues/issue-detail-widgets/relations/helper.tsx (1)
57-57: Consider adding specific error handling.The error handling has been simplified by removing the explicit error capture. While this works, you might want to consider capturing and logging specific error types for better debugging.
- } catch { + } catch (error) { + console.error("Failed to update issue:", error);Also applies to: 83-83
web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx (1)
40-52: Similar guard pattern in handleDisplayPropertiesUpdate.The
handleDisplayPropertiesUpdatefunction has the same guard pattern ashandleDisplayFilters. Consider applying the same recommendation for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apiserver/plane/app/views/issue/sub_issue.py(3 hunks)packages/constants/src/issue/common.ts(1 hunks)packages/constants/src/issue/filter.ts(11 hunks)packages/types/src/issues/issue_sub_issues.d.ts(1 hunks)packages/ui/src/icons/display-properties.tsx(1 hunks)packages/ui/src/icons/index.ts(1 hunks)web/core/components/issues/issue-detail-widgets/relations/helper.tsx(6 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx(4 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/helper.ts(8 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/index.ts(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx(9 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/root.tsx(2 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx(2 hunks)web/core/components/issues/relations/issue-list-item.tsx(1 hunks)web/core/components/issues/sub-issues/index.ts(0 hunks)web/core/components/issues/sub-issues/issues-list.tsx(0 hunks)web/core/components/issues/sub-issues/progressbar.tsx(0 hunks)web/core/components/issues/sub-issues/properties.tsx(0 hunks)web/core/components/issues/sub-issues/root.tsx(0 hunks)web/core/services/issue/issue.service.ts(1 hunks)web/core/store/issue/issue-details/sub_issues.store.ts(9 hunks)web/core/store/issue/issue-details/sub_issues_filter.store.ts(1 hunks)
💤 Files with no reviewable changes (5)
- web/core/components/issues/sub-issues/index.ts
- web/core/components/issues/sub-issues/issues-list.tsx
- web/core/components/issues/sub-issues/properties.tsx
- web/core/components/issues/sub-issues/progressbar.tsx
- web/core/components/issues/sub-issues/root.tsx
🧰 Additional context used
🧬 Code Graph Analysis (8)
web/core/components/issues/issue-detail-widgets/sub-issues/root.tsx (1)
web/core/store/router.store.ts (2)
projectId(85-87)workspaceSlug(69-71)
packages/constants/src/issue/common.ts (1)
packages/types/src/view-props.d.ts (1)
IIssueDisplayProperties(114-131)
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx (4)
packages/types/src/view-props.d.ts (2)
IIssueDisplayProperties(114-131)IIssueDisplayFilterOptions(101-113)packages/types/src/issues.d.ts (1)
ILayoutDisplayFiltersOptions(245-258)packages/ui/src/icons/display-properties.tsx (1)
DisplayPropertiesIcon(5-14)web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsx (1)
FilterDisplayProperties(24-94)
web/core/services/issue/issue.service.ts (2)
packages/types/src/view-props.d.ts (1)
TIssueParams(54-79)packages/types/src/issues/issue_sub_issues.d.ts (1)
TIssueSubIssues(11-14)
packages/constants/src/issue/filter.ts (1)
packages/constants/src/issue/common.ts (2)
ISSUE_DISPLAY_PROPERTIES_KEYS(149-166)SUB_ISSUES_DISPLAY_PROPERTIES_KEYS(168-175)
packages/ui/src/icons/display-properties.tsx (1)
packages/ui/src/icons/index.ts (1)
ISvgIcons(1-1)
web/core/components/issues/issue-detail-widgets/sub-issues/helper.ts (4)
packages/types/src/issues/issue.d.ts (1)
TIssueServiceType(123-123)packages/types/src/issues/issue_sub_issues.d.ts (1)
TSubIssueOperations(26-41)packages/utils/src/string.ts (1)
copyUrlToClipboard(81-87)web/core/store/router.store.ts (4)
workspaceSlug(69-71)projectId(85-87)issueId(149-151)epicId(173-175)
web/core/store/issue/issue-details/sub_issues_filter.store.ts (6)
packages/types/src/view-props.d.ts (4)
IIssueFilters(138-143)IIssueDisplayFilterOptions(101-113)IIssueDisplayProperties(114-131)TIssueParams(54-79)packages/types/src/issues/issue_sub_issues.d.ts (1)
TSubIssueResponse(16-16)packages/types/src/issues/issue.d.ts (1)
TIssue(54-67)packages/types/src/issues/base.d.ts (4)
TIssues(29-29)TGroupedIssueCount(41-43)TGroupedIssues(21-23)TSubGroupedIssues(25-27)web/core/store/issue/issue-details/sub_issues.store.ts (1)
IIssueSubIssuesStore(50-67)packages/constants/src/issue/common.ts (1)
ALL_ISSUES(3-3)
🔇 Additional comments (42)
packages/constants/src/issue/common.ts (1)
168-175: Great addition for sub-issues display propertiesThis addition clearly defines which display properties are relevant for sub-issues - a good subset of the full issue properties that makes sense for the sub-items context. The typing is correct, using the
keyof IIssueDisplayPropertiesto ensure type safety.packages/ui/src/icons/index.ts (1)
54-54: LGTM! New icon export for display propertiesThis export addition aligns with the PR objective of adding display filter toggles for sub-work items, making the icon available throughout the application.
web/core/components/issues/issue-detail-widgets/sub-issues/index.ts (1)
5-5: LGTM! Exporting new display filters componentAdding the export for display filters is consistent with the PR objective of adding display filter toggles for sub-work items.
web/core/components/issues/relations/issue-list-item.tsx (1)
112-112: Method name updated for better semanticsChanged from
copyTexttocopyLinkwhich more accurately describes the operation being performed and aligns with the refactored clipboard operations in the codebase.web/core/components/issues/issue-detail-widgets/sub-issues/root.tsx (2)
24-24: Improved comment clarity for isCollapsibleOpenThe comment change from "derived state" to "derived values" provides a more accurate description of what
isCollapsibleOpenrepresents, as it's not actually managing state but extracting a value from the store.
36-37: Props update for SubIssuesCollapsibleTitle componentAppropriate addition of
projectIdandworkspaceSlugprops to support the new sub-work items sorting functionality. These props are required by the updatedSubWorkItemTitleActionscomponent that handles filtering and sorting options.web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx (4)
11-13: Updated imports to use new component structureGood refactoring to use the new
SubIssuesListRootcomponent. This aligns with the broader refactoring of sub-issues components to support new filtering and sorting capabilities.
57-59: Improved destructuring of useIssueDetail hooksConsolidating the destructured modal toggle functions improves code readability.
67-79: Function optimization with useCallbackConverting
handleIssueCrudStateto useuseCallbackis a good optimization. This prevents unnecessary re-creation of the function on each render, improving performance particularly if this function is passed down to child components.
119-130: Updated component usage for sub-issues listReplaced the previous implementation with the new
SubIssuesListRootcomponent, which provides the enhanced sorting and filtering functionality required by the feature.web/core/services/issue/issue.service.ts (2)
270-275: Enhanced subIssues method with flexible query parametersThe addition of the optional
queriesparameter to thesubIssuesmethod is a key enhancement that enables the new sorting and filtering functionality. This allows clients to specify sort order, grouping, and other filter parameters when fetching sub-issues.
277-279: Updated API request to include query parametersThe request now properly forwards the queries to the API endpoint, enabling server-side filtering and sorting of sub-issues based on the provided parameters.
web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx (5)
3-12: Updated imports and added SubWorkItemTitleActions componentReplaced
SubIssuesActionButtonwith the newSubWorkItemTitleActionscomponent, which provides the enhanced filtering and sorting functionality for sub-work items.
19-21: Added required props for sub-issue filteringThe addition of
projectIdandworkspaceSlugprops is necessary for the new sub-work items sorting functionality. These props are needed by theSubWorkItemTitleActionscomponent to make API calls with the correct context.
24-31: Improved destructuring of propsReorganized the props destructuring for better readability and to accommodate the new props.
36-40: Updated store access for sub-issuesSimplified the destructuring of store values, maintaining only the required properties.
65-72: Replaced action button with enhanced SubWorkItemTitleActionsThe replacement of the previous action button with
SubWorkItemTitleActionsimplements the new sorting and filtering functionality for sub-work items. The component now receives the necessary props to function correctly within the context of the parent issue.packages/ui/src/icons/display-properties.tsx (1)
1-14: Well-structured SVG icon component implementationThis component is correctly implemented according to React best practices, with proper props handling, default className, and SVG attributes. The icon represents a display properties control, visually showing three adjustable sliders.
apiserver/plane/app/views/issue/sub_issue.py (2)
26-26: Good addition of utility importThe import of
order_issue_querysetaligns with the new ordering functionality being implemented.
106-114: Ordering implementation looks goodThe implementation of order_by functionality is well structured, getting the parameter from request.GET with a sensible default value and using the imported utility function.
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx (2)
1-7: Dependencies and imports look goodThe imports are well-organized, properly separating React core, third-party libraries, and application components with appropriate type imports.
8-15: Well-defined type interfaceThe props interface is comprehensive and properly typed, which enhances type safety and makes the component's requirements clear to consumers.
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx (6)
5-13: Import organization is improvedThe reorganized imports with the "plane imports" comment make the code more maintainable by clearly separating different types of imports.
25-40: Type definition is clearThe type definition provides a clear contract for the component props, making it easier for other developers to understand the component's requirements.
156-168: Good use of HOC for conditional renderingThe component correctly uses the
WithDisplayPropertiesHOCto conditionally render the issue identifier based on the display properties. This approach keeps the rendering logic clean and reusable.
181-189: Improved props passing to SubIssuesListItemPropertiesThe component now passes more specific props to the
SubIssuesListItemPropertiescomponent, including the whole issue object for better control and rendering.
214-214: Updated method name for better clarityRenaming from
copyTexttocopyLinkmore accurately reflects the action being performed, making the code more maintainable and understandable.
267-267: Updated component for nested sub-issuesThe component now correctly uses the new
SubIssuesListRootcomponent for rendering nested sub-issues, which ensures consistency with the new sub-issues implementation.web/core/components/issues/issue-detail-widgets/relations/helper.tsx (4)
9-9: Import refactoring looks good.The new import of
copyUrlToClipboardfrom@plane/utilsreplaces a previous clipboard helper import, simplifying clipboard operations with a standardized utility.
14-14: Method renaming improves consistency.Renaming from
copyTexttocopyLinkbetter reflects the method's purpose and aligns with similar changes in sub-issues operations.
31-39: Simplified clipboard implementation.The clipboard implementation has been simplified using the
copyUrlToClipboardutility, removing manual URL concatenation and making the code more maintainable.
92-92: Fixed useMemo dependency array.The dependency array has been properly expanded to include
captureIssueEvent,entityName, andt, which prevents potential stale closure issues and improves memoization correctness.web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx (3)
1-8: Imports look good.The imports are well-organized with clear separation between external libraries, plane imports, hooks, and local imports.
10-24: Well-structured props interface.The props interface is comprehensive and well-typed, providing all necessary information for the component to function properly.
26-42: Clean component implementation with appropriate store access.The component implementation is clean and follows good practices for accessing store data. The destructuring of props and store values improves readability.
packages/types/src/issues/issue_sub_issues.d.ts (2)
13-13: Good type enhancement for grouped sub-issues.The type of
sub_issueshas been updated from a simple array to a union typeTSubIssueResponse, which can handle both flat arrays and grouped objects. This change supports the new grouping functionality in the sub-issues feature.Also applies to: 16-16
26-41: Well-defined operations interface.The new
TSubIssueOperationstype provides a comprehensive interface for sub-issue management operations, including copying links, fetching, adding, updating, removing, and deleting sub-issues. This standardizes the API for interacting with sub-issues across components.web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx (5)
1-8: Clean imports with appropriate dependencies.The imports are well-organized and include all necessary dependencies for the component.
9-15: Well-typed component props.The props interface is well-defined with appropriate types for all properties.
17-26: Good store integration with MobX observer pattern.The component correctly uses the observer pattern and accesses the store to get sub-issue filters for the parent issue.
32-38: Ensure workspaceSlug and projectId guard against early returns.The
handleDisplayFiltersfunction correctly guards against undefinedworkspaceSlugorprojectIdbefore updating filters. However, this might hide potential errors in component usage if these props are expected to be defined.Consider logging a warning or throwing an error in development mode when these required values are missing:
if (!workspaceSlug || !projectId) return; + if (process.env.NODE_ENV === 'development') { + console.warn('Missing workspaceSlug or projectId in handleDisplayFilters'); + }
54-68: Clean and concise render method.The render method is clean and conditionally renders the
SubIssuesActionButtononly when not disabled. The component effectively passes all necessary props to its children.
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx
Show resolved
Hide resolved
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx
Show resolved
Hide resolved
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx
Show resolved
Hide resolved
* refactor: sub-work items components, hooks and types * feat: added orderby and display properties toggle for sub work items * fix: build errors * chore: removed issue type from filters * chore: added null check * fix: added null check --------- Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com>
Description
Refactored sub-work items components.
Added sort and display filters toggle for sub work items
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
WEB-3838
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores