[WEB-4028]feat: sub work item filters and grouping#6997
[WEB-4028]feat: sub work item filters and grouping#6997sriramveeraghanta merged 8 commits intopreviewfrom
Conversation
WalkthroughThis update introduces a comprehensive overhaul of sub-issue filtering, grouping, and display in the issue tracking UI. It adds new utility functions for date and filter handling, restructures the sub-issues filter store, enhances UI components for dynamic grouping and filtering, and updates type definitions and constants to support new group-by and filter options. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SubIssueFilters
participant Store
participant UI
User->>SubIssueFilters: Opens filter dropdown, selects filters
SubIssueFilters->>Store: updateSubWorkItemFilters(filterType, filters, workItemId)
Store->>Store: Update filters, recompute filtered/grouped sub-issues
Store-->>SubIssueFilters: Updated filtered/grouped sub-issues
SubIssueFilters-->>UI: Rerender sub-issue list with applied filters/grouping
sequenceDiagram
participant User
participant SubIssuesListRoot
participant Store
participant UI
User->>SubIssuesListRoot: Navigates to sub-issues section
SubIssuesListRoot->>Store: getSubIssueFilters(workItemId)
SubIssuesListRoot->>Store: getGroupedSubWorkItems(workItemId)
Store-->>SubIssuesListRoot: Filtered and grouped sub-issue IDs
SubIssuesListRoot-->>UI: Render SubIssuesListGroup for each group
UI-->>User: Shows grouped, filterable sub-issues
Possibly related PRs
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 (
|
vamsikrishnamathala
force-pushed
the
feat-sub_work_item_filters
branch
from
f6b672a to
711421e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (4)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx (2)
46-67: 🛠️ Refactor suggestion
disabled={!disabled}flips the semanticsThroughout the component every dropdown receives
disabled={!disabled}.
That means the dropdowns become enabled when the outerdisabledflag istrue, and disabled when the outer flag isfalse, which is counter-intuitive.Please verify the intended UX and either:
- Pass the flag directly (
disabled={disabled}), or- Rename the prop to something like
isEditableto avoid the double-negation.Failing to align the semantics can easily reopen editing for read-only users.
87-111:⚠️ Potential issue
maxDateshould be applied only to the start-date pickerYou correctly cap the start date so it cannot exceed
issue.target_date, but the due date picker (below) re-uses the samemaxDate, effectively limiting itself to its current value and preventing any future extension.Suggested fix:
• For the start-date picker keepmaxDate.
• For the due-date picker passminDatecomputed fromissue.start_date.-<DateDropdown - ... - maxDate={maxDate} +<DateDropdown + ... + maxDate={maxDate} // OK for start_date…and in the due-date block:
-const minDate = getDate(issue.start_date); -... -<DateDropdown - ... - maxDate={maxDate} +<DateDropdown + ... + minDate={minDate}This prevents selecting a due date earlier than the start date while still allowing extensions beyond the current target date.
web/core/store/issue/issue-details/sub_issues.store.ts (2)
94-101:⚠️ Potential issue
rootIssueDetailStoreis set after it is consumed byWorkItemSubIssueFiltersStore
this.filters = new WorkItemSubIssueFiltersStore(this);executes beforethis.rootIssueDetailStoreis assigned, so any access inside the filter-store constructor tostore.rootIssueDetailStoreisundefined.- this.filters = new WorkItemSubIssueFiltersStore(this); - // root store - this.rootIssueDetailStore = rootStore; + // root store + this.rootIssueDetailStore = rootStore; + // dependent stores + this.filters = new WorkItemSubIssueFiltersStore(this);This simple swap avoids undefined-access errors during app start-up.
127-161: 🛠️ Refactor suggestion
fetchSubIssuesnever clearsloaderon API failureIf the request rejects, the method will exit before reaching
this.loader = undefined, leaving the UI in a perpetual loading state.- this.loader = "init-loader"; - const response = await this.issueService.subIssues(workspaceSlug, projectId, parentIssueId); + this.loader = "init-loader"; + let response; + try { + response = await this.issueService.subIssues(workspaceSlug, projectId, parentIssueId); + ... + } finally { + this.loader = undefined; // always clear + }Wrapping the body in
try/finally(or catching and re-throwing) guarantees loader consistency.
🧹 Nitpick comments (14)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx (1)
86-86: Improved null safety with optional chaining.Using optional chaining (
?.) and providing a fallback empty object (??) improves code robustness by safely handling potential undefined values for subIssueFilters.While this change improves null safety, consider ensuring consistent usage of optional chaining throughout the component by also adding similar safety checks for other properties accessed from subIssueFilters elsewhere in the file.
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx (2)
29-32:useMemomay not give the expected optimisationBoth
displayPropertiesanddisplayFiltersare new object literals on almost every render, so the memoisation rarely hits.
Consider:- const isFilterApplied = useMemo( - () => isDisplayFiltersApplied({ displayProperties, displayFilters }), - [displayProperties, displayFilters] - ); + // Simple inline call – the helper is cheap enough that a memo provides + // negligible benefit given the high-churn dependencies. + const isFilterApplied = isDisplayFiltersApplied({ displayProperties, displayFilters });
40-49: Accessibility: add a descriptive label for the filter toggleScreen-reader users cannot infer the purpose of the icon-only button.
Addaria-label(and optionallytitle) to the wrapper to make it self-describing.- <div + <div + aria-label="Sub-issue display filters" + title="Sub-issue display filters"web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx (1)
78-85:preventDefault()on a non-interactive wrapper is unnecessaryCalling
e.preventDefault()on a<div>has no effect and may unintentionally block child interactions in some browsers.
e.stopPropagation()is sufficient to keep clicks from bubbling to the row-level handler.- e.stopPropagation(); - e.preventDefault(); + e.stopPropagation();web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx (2)
42-45: RedundantsetDatecall and potentialundefinedaccess
maxDate?.setDate(maxDate.getDate())is a no-op (it sets the date to exactly the same value) and may introduce confusion.
Ifissue.target_dateisundefined,maxDateis alsoundefined, yet the optional-chaining hides that fact and nothing happens. Consider removing the call altogether or, if you intended to clamp the value (e.g. set it to “today”), update the logic accordingly.-const maxDate = getDate(issue.target_date); -maxDate?.setDate(maxDate.getDate()); +const maxDate = getDate(issue.target_date); // already the correct value
70-85: MissingoldIssueargument breaks optimistic-update parity
updateSubIssueoptionally acceptsoldIssue; you forward it in the State dropdown, but omit it here.
Down-stream logic (e.g. query cache updates, rollback on failure) often relies on the previous snapshot. Supply the same{ ...issue }object for consistency.-updateSubIssue(workspaceSlug, issue.project_id, parentIssueId, issueId, { - priority: val, -}) +updateSubIssue( + workspaceSlug, + issue.project_id, + parentIssueId, + issueId, + { priority: val }, + { ...issue } +)web/helpers/date-time.helper.ts (1)
437-452:parseDateFilterignores the optional third segmentThe comment mentions filters like
"1_weeks;after;fromnow"but the implementation discards everything after index 1. Either update the doc-string or handleparts[2]to avoid surprising callers.Additionally, consider validating that
typeis"after"or"before"; otherwiseundefinedleaks downstream.web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-group.tsx (1)
72-73: Conditional class name can be simplified
cn("hidden", !isAllIssues && "block")always includes"hidden"(even when"block"is added), which relies on Tailwind’s CSS ordering to override the rule. This works today but is fragile.-buttonClassName={cn("hidden", !isAllIssues && "block")} +buttonClassName={isAllIssues ? "hidden" : "block"}This is clearer, avoids redundant classes, and prevents future regressions if utility ordering changes.
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx (1)
41-43: Prop type indicates required but default value treats it as optional
spacingLeftis declared inPropsas mandatory (number) but a default of0is supplied in the destructuring:spacingLeft = 0,Either mark the prop as optional in the interface (
spacingLeft?: number) or remove the default assignment to keep the types aligned.Inconsistent typings make it harder for consumers to understand the API and may hide misuse in TypeScript code.
web/core/components/issues/issue-detail-widgets/sub-issues/filters.tsx (1)
75-163: Repetition → hard to maintain – pull common JSX into a map-driven loopThe eight largely identical filter blocks differ only by:
• component,
• key name,
• optional extra props.Driving them from a configuration array (or object) would reduce ~80 lines, ease future additions and avoid copy-paste bugs (e.g. forgetting
statesforFilterState). Nice-to-have, but keeps this file lean.web/core/components/issues/issue-layouts/utils.tsx (1)
760-776: Optional safeguard forisDisplayFiltersAppliedWhen
filters.displayPropertiesisundefined, every key check evaluates truthy, so the function flags applied even for a fresh/default state. If this is not desired, add an explicit early‐return:- const isDisplayPropertiesApplied = Object.keys(DEFAULT_DISPLAY_PROPERTIES).some( - (key) => !filters.displayProperties?.[key as keyof IIssueDisplayProperties] - ); + if (!filters.displayProperties && !filters.displayFilters) return false; + const isDisplayPropertiesApplied = Object.keys(DEFAULT_DISPLAY_PROPERTIES).some( + (key) => !filters.displayProperties?.[key as keyof IIssueDisplayProperties] + );web/core/store/issue/helpers/base-issues-utils.ts (1)
240-268:getFilteredWorkItems()– potential type pitfalls & missed optimisations
activeFilters.everyrepeatsfilterValues!assertions; prefer safe-casting once.- For scalar fields you compare with
includes; using==allows numeric / enum coercion and can hide bugs.- Consider early-exit (
some) when any filter fails to match to avoid checking the remaining filters.web/core/store/issue/issue-details/sub_issues_filter.store.ts (2)
88-96: NestedrunInAction()inside anactionis redundant
updateSubWorkItemFiltersis already declared as an action. Wrapping the body in anotherrunInActionadds noise and extra transaction overhead.- runInAction(() => { - updateFilters(this.subIssueFilters, filterType, filters, workItemId); - }); + updateFilters(this.subIssueFilters, filterType, filters, workItemId);
121-131: Consider memoisingsubIssueIdslookup insidegetFilteredSubWorkItems
subIssueStore.subIssuesByIssueId(workItemId)is executed on every call even whenworkItemId's sub-issue set hasn’t changed.
Wrapping that selector itself withcomputedFnwould avoid unnecessary array allocations and improve UI performance when scrolling large issue lists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/constants/src/issue/common.ts(2 hunks)packages/constants/src/issue/filter.ts(5 hunks)packages/types/src/issues/issue.d.ts(1 hunks)packages/ui/src/collapsible/collapsible-button.tsx(4 hunks)web/core/components/empty-state/section-empty-state-root.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx(2 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx(3 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/filters.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-group.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx(5 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx(4 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx(3 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx(2 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx(1 hunks)web/core/components/issues/issue-layouts/utils.tsx(11 hunks)web/core/store/issue/helpers/base-issues-utils.ts(2 hunks)web/core/store/issue/helpers/base-issues.store.ts(1 hunks)web/core/store/issue/issue-details/sub_issues.store.ts(5 hunks)web/core/store/issue/issue-details/sub_issues_filter.store.ts(1 hunks)web/helpers/date-time.helper.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/ui/src/collapsible/collapsible-button.tsx (1)
packages/ui/src/icons/index.ts (1)
ISvgIcons(1-1)
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx (1)
web/core/components/issues/issue-layouts/utils.tsx (1)
isDisplayFiltersApplied(760-776)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx (5)
web/core/components/dropdowns/state.tsx (1)
StateDropdown(42-262)web/core/components/dropdowns/priority.tsx (1)
PriorityDropdown(304-504)web/core/store/router.store.ts (2)
workspaceSlug(69-71)issueId(149-151)web/core/components/dropdowns/date.tsx (1)
DateDropdown(36-196)web/helpers/date-time.helper.ts (1)
renderFormattedPayloadDate(58-68)
packages/constants/src/issue/common.ts (2)
packages/types/src/view-props.d.ts (1)
IIssueFilterOptions(83-99)packages/types/src/issues/issue.d.ts (1)
TIssue(54-68)
web/core/store/issue/helpers/base-issues.store.ts (2)
space/core/store/helpers/base-issues.store.ts (1)
TIssueDisplayFilterOptions(28-28)packages/types/src/issues/issue.d.ts (1)
TIssue(54-68)
packages/constants/src/issue/filter.ts (1)
packages/types/src/issues/activity/base.d.ts (1)
TIssueActivityComment(48-68)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (21)
web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx (2)
36-36: Code simplification looks good.The simplified destructuring of
subIssuesByIssueIdandstateDistributionByIssueIdfrom thesubIssuesobject aligns well with the centralization of filtering and grouping logic in the sub-issues feature.
62-68: LGTM! Prop removal is consistent with refactoring.The removal of the
workspaceSlugprop fromSubWorkItemTitleActionsis consistent with the broader refactoring where filter update calls have been simplified by centralizing filtering logic.packages/types/src/issues/issue.d.ts (1)
67-67: Good addition for state group support.Adding the
state__groupproperty to theTIssuetype correctly supports the new grouping and filtering capabilities for issues by state groups.web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx (2)
4-4: Import added for store type support.The addition of
EIssuesStoreTypeimport supports the new store type context functionality.
120-121: Good implementation of store type context.Adding the
storeTypeprop withEIssuesStoreType.PROJECTvalue correctly establishes the store type context for the sub-issues display, which is essential for the new filtering and grouping capabilities.web/core/components/empty-state/section-empty-state-root.tsx (4)
4-4: Added utility for class name handling.The
cnutility import is appropriate for handling class name combinations.
11-11: Good prop addition for customization.Adding the optional
customClassNameprop enhances component flexibility for styling customization.
15-15: Prop destructuring updated correctly.The prop destructuring is properly updated to include the new
customClassNameprop.
17-22: Well implemented class name handling.The use of the
cnutility to merge the base classes with the optionalcustomClassNameis well-implemented and follows best practices for component styling customization.packages/ui/src/collapsible/collapsible-button.tsx (4)
3-3: Good addition of ISvgIcons import.The import statement has been properly updated to include the ISvgIcons interface, which is needed for the new ChevronIcon prop type.
13-13: Well-implemented icon customization.Adding an optional ChevronIcon prop with the correct type of React.FC allows for flexible customization of the chevron icon while maintaining type safety.
25-25: Good use of default value.Setting DropdownIcon as the default value for ChevronIcon maintains backward compatibility with existing usage of the component.
37-37: Proper implementation of customizable icon.The component now properly uses the ChevronIcon variable instead of hardcoding DropdownIcon, while preserving all existing styling and rotation logic.
packages/constants/src/issue/common.ts (2)
1-7: Good import organization.Updated imports to include the necessary types (IIssueFilterOptions and TIssue) that are used in the new FILTER_TO_ISSUE_MAP constant.
371-383: Well-structured filter to issue property mapping.The new FILTER_TO_ISSUE_MAP constant provides a clear mapping between filter option keys and their corresponding issue property keys, facilitating consistent filtering operations.
The use of
as constensures type safety and immutability, which is a good practice for these kinds of mapping constants.web/core/store/issue/helpers/base-issues.store.ts (1)
121-121: Improved state group key mapping.The export of ISSUE_GROUP_BY_KEY and updating "state_detail.group" to map to "state__group" properly aligns with the TIssue type structure and the new FILTER_TO_ISSUE_MAP constant, ensuring consistency across the codebase for state group filtering and grouping operations.
Also applies to: 124-124
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/list-item.tsx (3)
6-6: Good import addition.Added import for EIssuesStoreType which is needed for the new storeType prop.
40-40: Well-implemented store type propagation.Adding an optional storeType prop with a sensible default value (EIssuesStoreType.PROJECT) allows for flexible store type selection while maintaining backward compatibility.
Also applies to: 55-55
270-270: Proper prop forwarding.The storeType prop is correctly passed down to the SubIssuesListRoot component, ensuring consistent store type context throughout the component hierarchy.
packages/constants/src/issue/filter.ts (1)
356-362: Verify type support fornullingroup_byoptions
group_by: ["state", "priority", "assignees", null]introducesnullas a valid entry.
Please confirm that:
ILayoutDisplayFiltersOptions["display_filters"]["group_by"]allowsnull(otherwise Typescript will flag).- Down-stream UI components gracefully handle a
nulloption (e.g., treat it as “No grouping”).If either is untrue, replace
nullwith an explicit sentinel (e.g.,"__none__").web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/properties.tsx (1)
114-137: Potential mutation of sharedDateobject
maxDate(created above) is passed to both date pickers. Inside some date-picker implementations the object can be mutated (e.g.setHours(…)). Because the same reference is reused, this could unexpectedly affect the other picker. Passing fresh instances (new Date(maxDate)) or using primitives is safer.
web/core/components/issues/issue-detail-widgets/sub-issues/display-filters.tsx
Show resolved
Hide resolved
web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx
Show resolved
Hide resolved
web/core/components/issues/issue-detail-widgets/sub-issues/filters.tsx
Outdated
Show resolved
Hide resolved
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx (1)
66-76: 🛠️ Refactor suggestionRename function to clearly indicate its purpose and fix dependencies
The function's current name doesn't clearly indicate it returns work item IDs for a specific group.
- const getWorkItemIds = useCallback( + const getWorkItemIdsForGroup = useCallback( (groupId: string) => { if (isRootLevel) { const groupedSubIssues = getGroupedSubWorkItems(parentIssueId); return groupedSubIssues?.[groupId] ?? []; } const subIssueIds = subIssuesByIssueId(parentIssueId); return subIssueIds ?? []; }, - [isRootLevel, subIssuesByIssueId, parentIssueId, getGroupedSubWorkItems] + [isRootLevel, subIssuesByIssueId, parentIssueId, getGroupedSubWorkItems, rootIssueId] );web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx (1)
58-76:⚠️ Potential issueAvoid mutating store arrays directly and simplify filter toggle logic
The current implementation mutates filter arrays directly and has complex logic for adding/removing values.
const handleFiltersUpdate = useCallback( (key: keyof IIssueFilterOptions, value: string | string[]) => { - const newValues = cloneDeep(subIssueFilters?.filters?.[key]) ?? []; + // Make a defensive copy to avoid mutating store state directly + const prevValues = subIssueFilters?.filters?.[key] ?? []; + const newValues = [...prevValues]; if (Array.isArray(value)) { - // this validation is majorly for the filter start_date, target_date custom - value.forEach((val) => { - if (!newValues.includes(val)) newValues.push(val); - else newValues.splice(newValues.indexOf(val), 1); - }); + // Simplify toggle logic for arrays of values + value.forEach((val) => { + const index = newValues.indexOf(val); + if (index === -1) newValues.push(val); + else newValues.splice(index, 1); + }); } else { - if (subIssueFilters?.filters?.[key]?.includes(value)) newValues.splice(newValues.indexOf(value), 1); - else newValues.push(value); + const index = newValues.indexOf(value); + if (index === -1) newValues.push(value); + else newValues.splice(index, 1); } updateSubWorkItemFilters(EIssueFilterType.FILTERS, { [key]: newValues }, parentId); }, [subIssueFilters?.filters, updateSubWorkItemFilters, parentId] );
🧹 Nitpick comments (9)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx (3)
54-56: Consider memoizing filter application variablesThe filters are recomputed on every render, which could be inefficient for complex filter logic.
Memoize the filtered work items count to avoid unnecessary recalculations:
const filters = getSubIssueFilters(parentIssueId); const isRootLevel = useMemo(() => rootIssueId === parentIssueId, [rootIssueId, parentIssueId]); const group_by = isRootLevel ? (filters?.displayFilters?.group_by ?? null) : null; - const filteredSubWorkItemsCount = (getFilteredSubWorkItems(rootIssueId, filters.filters ?? {}) ?? []).length; + const filteredSubWorkItemsCount = useMemo( + () => (getFilteredSubWorkItems(rootIssueId, filters.filters ?? {}) ?? []).length, + [getFilteredSubWorkItems, rootIssueId, filters.filters] + );
58-64: Memoize grouping columns to prevent unnecessary recalculationsThe
getGroupByColumnscall creates new objects on every render. Consider memoizing the result.- const groups = getGroupByColumns({ - groupBy: group_by as GroupByColumnTypes, - includeNone: true, - isWorkspaceLevel: isWorkspaceLevel(storeType), - isEpic: issueServiceType === EIssueServiceType.EPICS, - projectId, - }); + const groups = useMemo( + () => getGroupByColumns({ + groupBy: group_by as GroupByColumnTypes, + includeNone: true, + isWorkspaceLevel: isWorkspaceLevel(storeType), + isEpic: issueServiceType === EIssueServiceType.EPICS, + projectId, + }), + [group_by, storeType, issueServiceType, projectId] + );
100-117: Improve empty state messaging with dynamic variablesThe empty state text could be more succinct and DRY by using a variable.
<SectionEmptyState title={ - !isSubWorkItems - ? "You don't have work items that match the filters you've applied." - : "You don't have sub-work items that match the filters you've applied." + `You don't have ${!isSubWorkItems ? 'work' : 'sub-work'} items that match the filters you've applied.` } description={ - !isSubWorkItems - ? "To see all work items, clear all applied filters." - : "To see all sub-work items, clear all applied filters." + `To see all ${!isSubWorkItems ? 'work' : 'sub-work'} items, clear all applied filters.` } icon={<ListFilter />} customClassName={storeType !== EIssuesStoreType.EPIC ? "border-none" : ""} actionElement={ <Button variant="neutral-primary" size="sm" onClick={() => resetFilters(rootIssueId)}> Clear filters </Button> } />web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx (1)
80-86: Consider separating event handler functions from JSXThe inline event handlers could be extracted for better readability.
+ const preventEventPropagation = useCallback((e: React.MouseEvent) => { + e.stopPropagation(); + e.preventDefault(); + }, []); + return ( // prevent click everywhere <div className="flex gap-2 items-center" - onClick={(e) => { - e.stopPropagation(); - e.preventDefault(); - }} + onClick={preventEventPropagation} >web/core/store/issue/issue-details/sub_issues_filter.store.ts (5)
27-28: Standardize naming conventions in interface and implementationThe interface uses
IWorkItemSubIssueFiltersStorewhile the implementation isWorkItemSubIssueFiltersStore.For consistency, consider either:
- Renaming the interface to
ISubIssueFiltersStoreto match domain terminology- Renaming the class to
SubIssueFiltersStore(without the prefix)
91-93: UnnecessaryrunInActionwrapper in an already marked action methodThe method is already marked as an action, so the
runInActionwrapper is redundant.updateSubWorkItemFilters = ( filterType: EIssueFilterType, filters: IIssueDisplayFilterOptions | IIssueDisplayProperties | IIssueFilterOptions, workItemId: string ) => { - runInAction(() => { - updateFilters(this.subIssueFilters, filterType, filters, workItemId); - }); + updateFilters(this.subIssueFilters, filterType, filters, workItemId); };
120-130: Rename parameter to avoid shadowing class propertyThe
filtersparameter shadows thefiltersproperty in the class, which could cause confusion.- getFilteredSubWorkItems = computedFn((workItemId: string, filters: IIssueFilterOptions) => { + getFilteredSubWorkItems = computedFn((workItemId: string, filterOptions: IIssueFilterOptions) => { const subIssueIds = this.subIssueStore.subIssuesByIssueId(workItemId); const workItems = this.subIssueStore.rootIssueDetailStore.rootIssueStore.issues.getIssuesByIds( subIssueIds, "un-archived" ); - const filteredWorkItems = getFilteredWorkItems(workItems, filters); + const filteredWorkItems = getFilteredWorkItems(workItems, filterOptions); return filteredWorkItems; });
136-138: Rename method to better reflect what it doesThe
resetFiltersmethod doesn't actually reset filters to empty but initializes them to default values.- resetFilters = (workItemId: string) => { + resetFiltersToDefault = (workItemId: string) => { this.initializeFilters(workItemId); };Or update the implementation to actually clear the filters:
resetFilters = (workItemId: string) => { - this.initializeFilters(workItemId); + set(this.subIssueFilters, [workItemId, "displayProperties"], DEFAULT_DISPLAY_PROPERTIES); + set(this.subIssueFilters, [workItemId, "filters"], {}); + set(this.subIssueFilters, [workItemId, "displayFilters"], {}); };
101-113: Consider adding null checks for subIssueFilters propertiesThe code may risk errors when accessing properties of potentially undefined objects.
getGroupedSubWorkItems = computedFn((parentWorkItemId: string) => { const subIssueFilters = this.getSubIssueFilters(parentWorkItemId); const filteredWorkItems = this.getFilteredSubWorkItems(parentWorkItemId, subIssueFilters.filters ?? {}); // get group by and order by - const groupByKey = subIssueFilters.displayFilters?.group_by; - const orderByKey = subIssueFilters.displayFilters?.order_by; + const groupByKey = subIssueFilters?.displayFilters?.group_by; + const orderByKey = subIssueFilters?.displayFilters?.order_by; const groupedWorkItemIds = getGroupedWorkItemIds(filteredWorkItems, groupByKey, orderByKey); return groupedWorkItemIds; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
web/core/components/issues/issue-detail-widgets/sub-issues/filters.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx(3 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title-actions.tsx(2 hunks)web/core/components/issues/issue-layouts/utils.tsx(11 hunks)web/core/store/issue/helpers/base-issues-utils.ts(2 hunks)web/core/store/issue/issue-details/sub_issues_filter.store.ts(1 hunks)web/helpers/date-time.helper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- web/core/components/issues/issue-detail-widgets/sub-issues/filters.tsx
- web/core/store/issue/helpers/base-issues-utils.ts
- web/core/components/issues/issue-layouts/utils.tsx
- web/helpers/date-time.helper.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx
Show resolved
Hide resolved
vamsikrishnamathala
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
* feat: added filters for sub issues * feat: added list groups for sub issues * chore: updated order for sub work item properties * feat: filters for sub work items * feat: added filtering and ordering at frontend * chore: reverted backend filters * feat: added empty states * chore: code improvemnt --------- Co-authored-by: sangeethailango <sangeethailango21@gmail.com>
* feat: added filters for sub issues * feat: added list groups for sub issues * chore: updated order for sub work item properties * feat: filters for sub work items * feat: added filtering and ordering at frontend * chore: reverted backend filters * feat: added empty states * chore: code improvemnt --------- Co-authored-by: sangeethailango <sangeethailango21@gmail.com>
5 participants
Description
This update includes ordering and filtering for sub work items.
Filtering and grouping is done at frontend to reduce api calls.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
WEB-4028
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor