-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[GIT-40]fix: apply sub-issue display filter when adding work items #8534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA conditional filter check was added to the issue store's sub-issue addition logic. When adding a new sub-issue, the code now verifies whether sub-work item display is enabled via the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/core/store/issue/helpers/base-issues.store.ts (1)
1213-1214: LGTM - Correctly prevents sub-issues from being added when filter is disabled.The logic correctly skips both the issue ID addition and count accumulation when a sub-issue is being added while the display filter is off. DELETE and REORDER operations remain unaffected, which is the expected behavior.
Minor optional improvement: The variable
isSubIssuestores theparent_idstring value rather than a boolean. For slightly clearer intent, consider usingBoolean(issue?.parent_id)or renaming toparentId:💡 Optional: Improve variable naming clarity
if (issueUpdate.action === EIssueGroupedAction.ADD) { - const isSubIssue = issue?.parent_id; - if (isSubIssue && !isShowWorkItemsEnabled) continue; + const hasParent = Boolean(issue?.parent_id); + if (hasParent && !isShowWorkItemsEnabled) continue;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/store/issue/helpers/base-issues.store.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/store/issue/helpers/base-issues.store.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/store/issue/helpers/base-issues.store.ts
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/store/issue/helpers/base-issues.store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/store/issue/helpers/base-issues.store.ts
🧠 Learnings (1)
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
Applied to files:
apps/web/core/store/issue/helpers/base-issues.store.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/core/store/issue/helpers/base-issues.store.ts (1)
1201-1203: LGTM - Filter retrieval is correctly implemented.The optional chaining and nullish coalescing handle undefined filter states safely. The default of
falsemeans sub-issues will be hidden when the filter isn't explicitly set, which aligns with the bug fix intent.
Description
This fix ensures that Sub-work items are not added to the main issue list when the user has disabled the "Show sub Work items" display filter. Previously, work items would still appear in the list regardless of this setting.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
#8505
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.