[WEB-5490] fix: intake section filter persists incorrectly across projects #8187
Conversation
…filters and sorting
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughRefactors inbox project tracking to use a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant Root as InboxIssueRoot (component)
participant Hook as useProjectInbox (hook)
participant Store as ProjectInboxStore
participant Fetch as fetchInboxIssues
Root->>Hook: call with projectId
Hook->>Store: read currentInboxProjectId (+ other state)
Store-->>Hook: return currentInboxProjectId, loaders, etc.
Hook-->>Root: provide currentInboxProjectId and helpers
Root->>Root: compare currentInboxProjectId vs projectId
alt project changed
Root->>Root: handleCurrentTab(OPEN)
Root->>Fetch: fetchInboxIssues(workspaceSlug, projectId, undefined, OPEN)
else project unchanged
alt navigationTab present and differs from currentTab
Root->>Root: keep existing navigation behavior
else
Root->>Fetch: fetchInboxIssues(workspaceSlug, projectId, undefined, navigationTab)
end
end
Fetch->>Store: request/update inbox data
Store-->>Fetch: return results
Fetch-->>Root: provide fetched issues
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/core/store/inbox/project-inbox.store.ts (1)
275-288: Consider deriving both workspaceSlug and projectId from the same source.Both methods extract
workspaceSlugfromthis.store.routerbutprojectIdfromthis.currentInboxProjectId. This mixed source of truth could be confusing and may lead to subtle bugs if the router and state become desynchronized.Consider one of these approaches:
- Option 1: Store both
workspaceSlugandprojectIdin local state (addcurrentWorkspaceSlug)- Option 2: Derive both from the router and remove
currentInboxProjectId(reverting to router as single source)- Option 3: Add a comment explaining why this mixed pattern is necessary
handleInboxIssueFilters = <T extends keyof TInboxIssueFilter>(key: T, value: TInboxIssueFilter[T]) => { - const { workspaceSlug } = this.store.router; - const projectId = this.currentInboxProjectId; + // Note: workspaceSlug from router, projectId from state to support per-project filter persistence + const { workspaceSlug } = this.store.router; + const projectId = this.currentInboxProjectId; if (workspaceSlug && projectId) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/inbox/root.tsx(1 hunks)apps/web/core/components/inbox/sidebar/root.tsx(0 hunks)apps/web/core/store/inbox/project-inbox.store.ts(3 hunks)
💤 Files with no reviewable changes (1)
- apps/web/core/components/inbox/sidebar/root.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/inbox/project-inbox.store.tsapps/web/core/components/inbox/root.tsx
🧠 Learnings (3)
📚 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/inbox/project-inbox.store.tsapps/web/core/components/inbox/root.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/inbox/root.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/core/components/inbox/root.tsx
🧬 Code graph analysis (1)
apps/web/core/components/inbox/root.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ 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 and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
apps/web/core/store/inbox/project-inbox.store.ts (2)
128-129: LGTM! Getter correctly uses currentInboxProjectId.The change from
this.store.router.projectIdtothis.currentInboxProjectIdproperly implements per-project filter tracking. The guard clause returns an empty object when no project is active, preventing errors.
136-137: LGTM! Consistent pattern with inboxFilters.The sorting getter follows the same pattern as
inboxFilters, usingcurrentInboxProjectIdas the source of truth for project-scoped state.apps/web/core/components/inbox/root.tsx (3)
32-32: LGTM! Correct extraction of currentInboxProjectId.Extracting
currentInboxProjectIdfrom the hook enables project change detection in the effect below.
36-50: Verify that missingnavigationTabin dependencies is safe.The effect reads
navigationTabandcurrentTab(line 39) but only includes[inboxAccessible, workspaceSlug, projectId]in its dependency array. While theeslint-disablecomment suggests this is intentional, consider these scenarios:
Scenario: User is on the "Closed" tab (
currentTab = CLOSED), then the parent component changesnavigationTabtoOPENwithout changingprojectId.
- Current behavior: Effect doesn't run because
projectIddidn't change- Expected behavior: Might want to sync tabs when
navigationTabchanges
currentInboxProjectIdfrom MobX: Since the component is wrapped withobserver, changes tocurrentInboxProjectIdtrigger re-renders, but the effect won't re-run. This could lead to stale comparisons in theprojectChangedcalculation if accessed mid-render.However, if the current behavior is intentional (only respond to route-level changes, not tab switches), then this is acceptable.
If
navigationTabchanges should trigger the effect, apply this diff:// eslint-disable-next-line react-hooks/exhaustive-deps -}, [inboxAccessible, workspaceSlug, projectId]); +}, [inboxAccessible, workspaceSlug, projectId, navigationTab]);Otherwise, consider adding a comment explaining why
navigationTabis intentionally excluded:+ // Note: navigationTab intentionally excluded - only respond to project-level navigation // eslint-disable-next-line react-hooks/exhaustive-deps }, [inboxAccessible, workspaceSlug, projectId]);
41-42: Excellent! Project change handling addresses the PR objective.The logic correctly detects project switches and resets to the OPEN tab, which resolves the filter persistence issue described in the PR objectives. When
currentInboxProjectIddiffers from the newprojectId, callinghandleCurrentTabwithEInboxIssueCurrentTab.OPENensures each project starts fresh.
…section-filter-persist
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
…section-filter-persist
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/inbox/root.tsx (1)
52-52: Dependency array intentionally excludes reactive values.The effect uses
navigationTab,currentTab, andcurrentInboxProjectIdbut only includes[inboxAccessible, workspaceSlug, projectId]in the dependency array. This appears intentional based on the eslint-disable comment.Rationale:
currentInboxProjectIdis compared againstprojectIdwithin the effect. WhenprojectIdchanges, the effect re-runs and detects the project change.navigationTabandcurrentTabchanges may be handled by other effects or component re-renders, avoiding duplicate logic.If the current behavior doesn't correctly respond to
navigationTaborcurrentTabupdates during the same project session, consider adding them to the dependency array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/inbox/root.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/components/inbox/root.tsx
🧠 Learnings (3)
📚 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/components/inbox/root.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/inbox/root.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/core/components/inbox/root.tsx
🧬 Code graph analysis (1)
apps/web/core/components/inbox/root.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ 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 and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/core/components/inbox/root.tsx (3)
32-32: LGTM! Clean addition of project tracking.Adding
currentInboxProjectIdto track the current inbox state's associated project enables detection of cross-project navigation.
36-37: LGTM! Correct project-change detection.The truthiness check on
currentInboxProjectIdcorrectly prevents treating the initial load as a project change, ensuring the logic only triggers when switching between projects.
44-49: LGTM! Correct fetchInboxIssues invocation.The updated call signature correctly passes four arguments:
workspaceSlug,projectId,undefined(forinboxIssueId), andnavigationTab || EInboxIssueCurrentTab.OPENas the default tab, aligning with the new signature mentioned in the AI summary.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Description
Fixes filter and sorting persistence when switching between projects in the intake page.
Fixes UI/filter state mismatch where tab selection didn't match the applied filter
Ensure each project defaults to OPEN tab when first accessed
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.