[WEB-4338] fix: incorrect error code in project retrieve API #7234
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAPI now enforces membership checks with network-type-specific errors; web routing moved to projectId-based layouts; ProjectAuthWrapper made projectId required and replaced JoinProject with ProjectAccessRestriction; multiple UI loading spinners removed; new i18n keys added for project access/invalid states. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Key attention areas:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
sangeethailango
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
sangeethailango
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the error code returned by the project retrieve API when a workspace member attempts to access a project they are not a member of. Previously, the code returned a 404 (Not Found) error for both non-existent projects and permission issues. The fix correctly separates these cases: returning 404 when a project doesn't exist or is archived, and 403 (Forbidden) when the user lacks project membership.
Key Changes
- Simplified project query to only check for archived status and project existence
- Added explicit membership validation that returns 403 when user is not a project member
- Separated permission errors from not-found errors for better API semantics
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/plane/app/views/project/base.py (1)
217-217: Consider a more efficient membership check.Building a list of all member IDs and then checking membership is less efficient than using a generator with
any().Apply this diff for a more efficient check:
-member_ids = [project_member.member.id for project_member in project.members_list] - -if request.user.id not in member_ids: +if not any(project_member.member.id == request.user.id for project_member in project.members_list): return Response( {"error": "You do not have permission"}, status=status.HTTP_403_FORBIDDEN, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/views/project/base.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/app/views/project/base.py (2)
apps/api/plane/app/views/project/member.py (1)
get_queryset(27-38)apps/api/plane/api/views/project.py (2)
get_queryset(72-130)get_queryset(327-385)
⏰ 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: Agent
- GitHub Check: Build and lint web apps
prateekshourya29
force-pushed
the
fix-project-status-code
branch
from
77be274 to
c0ce2ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/app/views/project/base.py (1)
213-231: Membership check and status codes inretrieveThe new flow correctly separates “project not found” (404) from “no project membership” (403/409), and the containment check now uses matching types, so the earlier type-mismatch issue is resolved.
A couple of improvements/clarifications you might want to consider:
- The code assumes
project.members_listis always present because of thePrefetchinget_queryset(). Ifget_queryset()is ever changed (orretrieveis overridden) and that prefetch is removed, this could raise anAttributeError. A small defensive guard keepsretrieveresilient.- You don’t need to cast IDs to strings here; comparing raw IDs keeps this aligned with other usages (e.g.,
ProjectListSerializer.get_members) and avoids extra allocations. Using a set also gives O(1) membership checks.You can address both with something like:
- member_ids = [str(project_member.member_id) for project_member in project.members_list] - - if str(request.user.id) not in member_ids: + members = getattr(project, "members_list", None) + member_ids = {m.member_id for m in (members or [])} + + if request.user.id not in member_ids: if project.network == ProjectNetwork.SECRET.value: return Response( {"error": "You do not have permission"}, status=status.HTTP_403_FORBIDDEN, ) else: return Response( {"error": "You are not a member of this project"}, status=status.HTTP_409_CONFLICT, )Also, behavior-wise:
list/list_detaillet workspaceROLE.MEMBERusers see public (network=2) projects they are not members of, whileretrievestill blocks them (409). If that “discoverable but not viewable until joined” behavior is intentional, it may be worth adding/adjusting tests or a short comment to lock that contract in; otherwise, you might want to revisit it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/app/views/project/base.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/base.py
🧬 Code graph analysis (1)
apps/api/plane/app/views/project/base.py (3)
apps/api/plane/db/models/project.py (4)
ROLE(21-24)Project(65-159)ProjectMember(192-236)ProjectNetwork(27-33)apps/api/plane/app/serializers/project.py (1)
ProjectListSerializer(100-118)apps/api/plane/bgtasks/webhook_task.py (2)
model_activity(460-502)webhook_activity(374-456)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/app/views/project/base.py (1)
4-37: Imports and base wiring look consistentAll newly marked imports (boto3, settings, timezone, Response, permissions, serializers, BaseViewSet, tasks, models) are used in this module and line up with the classes/endpoints below. Nothing to change here.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/issues/issue-layouts/roots/project-layout-root.tsx (1)
50-58: Add error handling for filter fetch failures.The
useSWRhook doesn't capture or handle errors fromfetchFilters. If the API call fails, users will see a blank screen with no feedback about what went wrong.Consider capturing the error state and providing user feedback:
- useSWR( + const { error } = useSWR( workspaceSlug && projectId ? `PROJECT_ISSUES_${workspaceSlug}_${projectId}` : null, async () => { if (workspaceSlug && projectId) { await issuesFilter?.fetchFilters(workspaceSlug, projectId); } }, { revalidateIfStale: false, revalidateOnFocus: false } );Then add an error state check before the guard at line 60:
if (error) return <div>Error loading filters: {error.message}</div>;
🧹 Nitpick comments (7)
apps/web/core/components/issues/issue-layouts/roots/archived-issue-layout-root.tsx (1)
38-38: Guard clause correctly prevents rendering without required data.The early return ensures the component only renders when all required data is available, which is appropriate given the removal of loading states.
Optional: Consider returning
nullinstead of<></>as it's slightly more idiomatic for "render nothing":- if (!workspaceSlug || !projectId || !workItemFilters) return <></>; + if (!workspaceSlug || !projectId || !workItemFilters) return null;apps/web/app/routes/core.ts (1)
358-363: Consider whether the nested automations layout is necessary.Unlike sibling routes (members, features, states, labels, estimates), automations has its own nested layout creating three levels of nesting:
settings/projects/[projectId]/layout.tsx→automations/layout.tsx→page.tsx.If automations will have multiple child routes in the future, this structure makes sense. Otherwise, consider flattening it to match the pattern used by other project settings routes:
- // Project Automations - layout("./(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/automations/layout.tsx", [ - route( - ":workspaceSlug/settings/projects/:projectId/automations", - "./(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/automations/page.tsx" - ), - ]), + // Project Automations + route( + ":workspaceSlug/settings/projects/:projectId/automations", + "./(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/automations/page.tsx" + ),If there are plans for multiple automation pages (e.g.,
/automations/rules,/automations/history), then the current nested layout is appropriate.apps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsx (1)
51-61: SWR-based filter bootstrap looks good; you can trim some redundancyThis side-effect-only
useSWRto ensure module filters are fetched whenworkspaceSlug,projectId, andmoduleIdare present looks fine and matches the pattern used in other layout roots. Since you already normalize the router params with.toString()above, the extra.toString()calls in the key and fetcher here are redundant, and the innerif (workspaceSlug && projectId && moduleId)is also defensive given the key gating.None of this is blocking, but simplifying the key/fetcher would make the code a bit cleaner while preserving behavior. Also, continuing to use
useParamsin this non-route component is aligned with the prior guidance about restricting theparamsprop to page/layout files. Based on learnings.apps/web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx (2)
61-69: SWR side-effect usage is fine; consider small cleanups for clarityUsing SWR keyed by
workspaceSlug/projectId/cycleIdto orchestrateissuesFilter.fetchFilterslooks correct and matches the new “always fetch when identifiers exist” pattern. Since this hook is purely for a side effect and you never readdata/error, you might consider:
- dropping the inner
if (workspaceSlug && projectId && cycleId)(it’s redundant with the key check), and/or- adding a short comment explaining that this SWR instance exists only to keep filters in sync, so future readers aren’t surprised there’s no
datausage.
80-90: Early guard avoids partial renders but hides filter-load failuresThe early return on
!workspaceSlug || !projectId || !cycleId || !workItemFiltersensures the HOC and layouts never mount with missing filters, which aligns with the “no intermediate spinner, don’t render until ready” direction and guaranteesupdateFiltersalways has valid IDs.One trade-off: if
fetchFiltersfails (network/server error or a store-level issue),workItemFiltersstays undefined and this component silently renders nothing. If that’s a realistic failure mode, consider wiring an error flag (e.g., from SWR’serroror the store) into this guard and rendering a minimal empty/error state instead of a blank area.Also,
ISSUE_DISPLAY_FILTERS_BY_PAGE.issues.filtershere is consistent with the separation between layout-level and page-level filter presets in this codebase, so the constant choice looks correct. Based on learnings.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx (1)
23-23: Minor: Redundant projectId check.The
projectIdcheck is redundant since this layout only renders when the route includes[projectId]in the path. The params will always contain a validprojectIdstring at this point.Apply this diff to simplify:
- <div className="hidden md:block">{projectId && <ProjectSettingsSidebar />}</div> + <div className="hidden md:block"><ProjectSettingsSidebar /></div>apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
59-70: IncludeworkspaceSlugincanAddProjectcall for consistency withisWorkspaceAdminThe review identifies a valid inconsistency. The
allowPermissionsfunction signature shows thatworkspaceSlugis optional and falls back to the router's current workspace when not provided:allowPermissions = ( allowPermissions: ETempUserRole[], level: TUserPermissionsLevel, workspaceSlug?: string, // optional projectId?: string, onPermissionAllowed?: () => boolean ): boolean => { const { workspaceSlug: currentWorkspaceSlug, projectId: currentProjectId } = this.store.router; if (!workspaceSlug) workspaceSlug = currentWorkspaceSlug; // falls back to router // ... }In
project-wrapper.tsx,canAddProjectomitsworkspaceSlugwhileisWorkspaceAdminpasses it explicitly. Though both resolve to the same workspace in this context (both props reference the same workspace), the inconsistency makes intent unclear. The suggested fix improves clarity by being explicit:const canAddProject = allowPermissions( [EUserPermissions.ADMIN, EUserPermissions.MEMBER], - EUserPermissionsLevel.WORKSPACE + EUserPermissionsLevel.WORKSPACE, + workspaceSlug );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/api/plane/db/models/__init__.py(1 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/page.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/page.tsx(2 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/layout.tsx(2 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/page.tsx(2 hunks)apps/web/app/routes/core.ts(3 hunks)apps/web/ce/layouts/project-wrapper.tsx(1 hunks)apps/web/core/components/auth-screens/project/access-restriction.tsx(1 hunks)apps/web/core/components/issues/issue-layouts/roots/archived-issue-layout-root.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/roots/project-layout-root.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx(2 hunks)apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsx(1 hunks)apps/web/core/components/workspace/settings/workspace-details.tsx(1 hunks)apps/web/core/layouts/auth-layout/project-wrapper.tsx(4 hunks)apps/web/core/services/project/project.service.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsx
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/workspace/settings/workspace-details.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/page.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.tsxapps/web/core/components/issues/issue-layouts/roots/project-layout-root.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsxapps/web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/layout.tsxapps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsxapps/web/app/routes/core.ts
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/core/components/issues/issue-layouts/roots/project-view-layout-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/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/page.tsxapps/web/core/components/issues/issue-layouts/roots/project-layout-root.tsxapps/web/ce/layouts/project-wrapper.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
Repo: makeplane/plane PR: 6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.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/issues/issue-layouts/roots/project-layout-root.tsxapps/web/ce/layouts/project-wrapper.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-01-24T09:37:19.339Z
Learnt from: mathalav55
Repo: makeplane/plane PR: 6452
File: web/helpers/issue.helper.ts:6-6
Timestamp: 2025-01-24T09:37:19.339Z
Learning: In the Plane codebase, `ISSUE_DISPLAY_FILTERS_BY_LAYOUT` and `ISSUE_DISPLAY_FILTERS_BY_PAGE` are two distinct constants serving different purposes - one for layout-level filters and another for page-level filters. They are not interchangeable and should coexist.
Applied to files:
apps/web/core/components/issues/issue-layouts/roots/archived-issue-layout-root.tsxapps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsxapps/web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsxapps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsx
📚 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/ce/layouts/project-wrapper.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-06-18T09:46:08.566Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7188
File: web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx:40-45
Timestamp: 2025-06-18T09:46:08.566Z
Learning: When reviewing breadcrumb components that accept projectId or similar props, check if empty strings are being passed during loading states, which can result in invalid URLs. The preferred approach is to handle these loading states internally within the component rather than requiring each consumer to manage the loading logic.
Applied to files:
apps/web/ce/layouts/project-wrapper.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx
📚 Learning: 2025-07-08T13:41:01.659Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/core/components/issues/select/dropdown.tsx:9-11
Timestamp: 2025-07-08T13:41:01.659Z
Learning: The `getProjectLabelIds` function in the label store handles undefined projectId internally, so it's safe to pass undefined values to it without explicit checks in the calling component.
Applied to files:
apps/web/ce/layouts/project-wrapper.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx
🧬 Code graph analysis (13)
apps/web/core/components/workspace/settings/workspace-details.tsx (1)
apps/web/core/store/workspace/index.ts (1)
currentWorkspace(120-125)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx (4)
apps/web/core/components/settings/mobile/nav.tsx (1)
SettingsMobileNav(14-47)apps/web/core/components/settings/project/sidebar/root.tsx (1)
ProjectSettingsSidebar(18-52)apps/web/ce/layouts/project-wrapper.tsx (1)
ProjectAuthWrapper(11-20)apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
ProjectAuthWrapper(42-142)
apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsx (1)
apps/web/core/store/issue/workspace-draft/issue.store.ts (1)
issueIds(145-151)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/page.tsx (1)
apps/web/core/store/project/project.store.ts (1)
currentProjectDetails(214-217)
apps/web/core/components/auth-screens/project/access-restriction.tsx (3)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)apps/web/core/components/auth-screens/project/join-project.tsx (1)
JoinProject(17-67)packages/propel/src/empty-state/detailed-empty-state.tsx (1)
EmptyStateDetailed(8-54)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.tsx (4)
apps/web/core/components/core/page-title.tsx (1)
PageHead(8-18)apps/web/core/components/settings/heading.tsx (1)
SettingsHeading(17-46)apps/web/core/components/profile/profile-setting-content-header.tsx (1)
ProfileSettingContentHeader(6-14)apps/web/core/components/profile/preferences/language-timezone.tsx (1)
LanguageTimezone(10-144)
apps/web/core/components/issues/issue-layouts/roots/archived-issue-layout-root.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/layouts/auth-layout/project-wrapper.tsx (4)
apps/web/ce/layouts/project-wrapper.tsx (2)
ProjectAuthWrapper(11-20)IProjectAuthWrapper(5-9)apps/web/core/constants/fetch-keys.ts (9)
PROJECT_DETAILS(140-141)PROJECT_ME_INFORMATION(143-144)PROJECT_LABELS(146-147)PROJECT_MEMBERS(149-150)PROJECT_STATES(152-153)PROJECT_ESTIMATES(155-156)PROJECT_ALL_CYCLES(158-159)PROJECT_MODULES(161-162)PROJECT_VIEWS(164-165)apps/web/core/components/auth-screens/project/access-restriction.tsx (1)
ProjectAccessRestriction(16-48)packages/constants/src/event-tracker/core.ts (1)
PROJECT_TRACKER_ELEMENTS(51-64)
apps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx (2)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)apps/web/core/store/issue/helpers/base-issues.store.ts (1)
cycleId(269-271)
apps/api/plane/db/models/__init__.py (1)
apps/api/plane/db/models/project.py (1)
ProjectNetwork(27-33)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/layout.tsx (1)
apps/web/core/hooks/use-app-router.tsx (1)
useAppRouter(4-4)
apps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsx (2)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)apps/web/core/store/issue/helpers/base-issues.store.ts (1)
moduleId(264-266)
⏰ 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 (23)
apps/api/plane/db/models/__init__.py (1)
55-55: LGTM! ProjectNetwork export added correctly.The addition of
ProjectNetworkto the public exports is appropriate and necessary for the bug fix. It enables the view layer to check whether a project is SECRET or PUBLIC and return the correct HTTP status codes (403 for secret projects, 409 for non-members).apps/web/core/components/issues/issue-layouts/roots/project-layout-root.tsx (1)
60-60: LGTM: Unified guard aligns with PR objectives.The single guard clause that checks all required dependencies before rendering aligns well with the PR's goal to "render nothing until data ready" and removes intermediate loading states.
However, this approach works best when combined with proper error handling (see comment on lines 50-58) to distinguish between "still loading" and "failed to load" states.
apps/web/core/components/issues/issue-layouts/roots/archived-issue-layout-root.tsx (1)
28-36: LGTM! Spinner removal aligns with PR objectives.The removal of loading state tracking and the LogoSpinner is consistent with the PR's stated goal to eliminate intermediate spinners and render nothing until data is ready. This pattern has been applied consistently across other layout components.
apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsx (1)
74-74: No issues found - code change is correct.The SpreadsheetView component properly handles the empty data case by returning an empty fragment. This is appropriate because the actual loading state management is handled by the
IssueLayoutHOCwrapper component that wraps both parent components (workspace-root.tsxandbase-spreadsheet-root.tsx). The HOC checksgetIssueLoader()and displays anActiveLoaderduring initial data fetch. By the timeSpreadsheetViewreceives an emptyissueIdsarray, it means data has already loaded with no results—rendering nothing is the correct behavior in this scenario.apps/web/app/routes/core.ts (3)
111-117: Verify the distinction between workspace and project archives is clear.This introduces a workspace-level archives route (
:workspaceSlug/projects/archives) separate from the existing project-level archives (:workspaceSlug/projects/:projectId/archives/*). Ensure the UI clearly distinguishes between:
- Workspace archives: viewing all archived projects
- Project archives: viewing archived items (issues, cycles, modules) within a project
The route structure is correct, but user experience could suffer if the distinction isn't obvious in the interface.
326-356: Project settings layout properly implements the restructure.Verification confirms:
- ✓
projectIdis correctly extracted from params (line 12)- ✓ Project context is provided to child routes via
ProjectAuthWrapperwithprojectIdprop (line 15)- ✓ Child routes render via
Outlet(line 19)The layout is well-structured. Error and loading state handling is appropriately delegated to
ProjectAuthWrapper, which wraps the entire content tree.
129-129: All consuming layouts and pages properly handle the projectId parameter ✓Verification confirms the refactor is correctly implemented:
- Page files: All 20+ page.tsx files under [projectId] properly use
Route.ComponentPropsand destructureparams(includingprojectId)- Layout files: No
useParams()misuse detected. Only layouts that need params declare them (4 layouts), while container layouts without parameter dependencies correctly omit them—this is the correct Next.js App Router pattern- Parent layout:
ProjectDetailLayoutproperly receives and can propagate params to childrenThe structural change is sound and follows Next.js conventions.
apps/web/core/components/issues/issue-layouts/roots/module-layout-root.tsx (1)
63-63: Guard onworkItemFiltersmay render a blank screen if filters never materializeThe stricter guard (
!workspaceSlug || !projectId || !moduleId || !workItemFilters) ensures we don’t render until filters exist, which is good for avoiding intermediate loading UI. However, this also means that ifissuesFilter.getIssueFilters(moduleId)can legitimately returnundefined(e.g., on a fetch error or for a brand-new module before defaults are created), this layout will silently render nothing with no visible error.Please double-check the contract of
getIssueFiltersfor the MODULE store and confirm it always yields at least a default filter set for accessible modules afterfetchFiltersruns. If not, consider either creating defaults on the store side or surfacing an explicit error/empty state here instead of an empty fragment.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/preferences/page.tsx (2)
22-34: Clean refactor using early return pattern.The simplified structure improves readability by removing the conditional wrapper in favor of the early guard at line 18. This is functionally equivalent and aligns with the PR objective to streamline loading state handling.
18-18: Early guard is necessary and correctly implemented.Both child components that
PreferencesListrenders—ThemeSwitcherandStartOfWeekPreference—use theuseUserProfile()hook and directly accessuserProfiledata. Specifically,StartOfWeekPreferenceaccessesuserProfile.start_of_the_weekwithout null checks (line 33 of start-of-week-preference.tsx), making the parent's early guard essential to prevent crashes. The guard ensures both components render only when data is available.apps/web/core/services/project/project.service.ts (1)
56-62: LGTM - Intentional error shape change to surface status codes.The change from throwing
error?.response?.datatoerror?.responseenables downstream code to inspect HTTP status codes (403, 409, 404) for access control decisions. Note that this creates an intentional inconsistency with other methods in this service that still throwerror?.response?.data.apps/web/core/components/issues/issue-layouts/roots/project-view-layout-root.tsx (2)
61-68: LGTM - Data fetching without loading state.The useSWR call properly initializes the filters fetch without exposing loading state, aligning with the PR's pattern of removing intermediate spinners.
79-79: LGTM - Enhanced guard with workItemFilters check.Adding
workItemFiltersto the guard condition ensures the component renders nothing until all necessary data is available, eliminating the need for intermediate loading states.apps/web/core/components/workspace/settings/workspace-details.tsx (1)
131-131: LGTM - Removed loading spinner in favor of empty render.This change aligns with the PR's pattern of removing intermediate loading spinners and relying on layout-level data handling. The component now renders nothing when
currentWorkspaceis unavailable.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/page.tsx (2)
26-26: LGTM - Data loading delegated to layout.Removing the SWR fetch from this page component indicates that project details are now loaded at the layout level, which is a cleaner separation of concerns for route layouts.
54-63: LGTM - Simplified rendering logic.The condition now only checks for
currentProjectDetailsexistence, relying on the parent layout to handle loading states and data fetching. This simplification aligns with the PR's pattern of centralizing data loading.apps/web/ce/layouts/project-wrapper.tsx (1)
5-9: LGTM - Required projectId improves type safety.Making
projectIdrequired (instead of optional) strengthens the prop contract and aligns with the PR's goal of requiring explicit project identifiers for access control. This ensures all consumers provide a valid projectId.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/layout.tsx (2)
17-22: LGTM - Redirect logic for projects without explicit projectId.The effect properly redirects to the first joined project when no
projectIdis present in the route. WhenjoinedProjectIdsis empty, the component will render the child route (likely the empty state page), which is the correct behavior.
24-24: LGTM - Simplified layout delegates to child routes.Rendering only the
Outletis a clean pattern that delegates all content rendering to child routes, with the layout handling only navigation logic.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/page.tsx (1)
1-1: LGTM - Enhanced with MobX observer and typed asset imports.The refactor adds MobX observation for reactive updates and switches from string-based paths to imported asset references, improving type safety and asset management. The direct import approach is cleaner and better aligned with modern bundler practices.
Also applies to: 9-10, 19-19, 44-44
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx (1)
13-31: LGTM - Well-structured project settings layout.The layout properly handles mobile navigation, authentication, and the two-column settings structure. The
ProjectAuthWrappercentralizes access control at the layout level, and theOutletpattern cleanly delegates content rendering to child routes.apps/web/core/layouts/auth-layout/project-wrapper.tsx (2)
35-40: MakingprojectIdrequired onIProjectAuthWrapperis a solid contract improvementRequiring
projectId: stringhere is a good change; it prevents empty/undefined IDs from leaking into fetch keys and downstream hooks, and aligns with the CE wrapper interface you’ve updated. Just ensure all route/layout call sites now pass the real param instead of placeholders; from the provided CE layout snippet this looks consistent.
78-122: SWR wiring for project resources looks coherent and cache-friendlyThe SWR setup for project details, membership info, labels, members, states, estimates, cycles, modules, and views is consistent, with stable keys and
revalidateIfStale/revalidateOnFocusdisabled where appropriate. The combined modules fetcher usingPromise.allis a reasonable way to keep both slim and full module stores in sync from a single SWR subscription.
apps/web/core/components/auth-screens/project/access-restriction.tsx
Outdated
Show resolved
Hide resolved
apps/web/core/components/auth-screens/project/access-restriction.tsx
Outdated
Show resolved
Hide resolved
…enhance access restriction logic in ProjectAccessRestriction
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/auth-screens/project/access-restriction.tsx (1)
16-48: Clarify 403 handling (SECRET projects) and consider using the private-project variantThe new branching fixes the earlier issue with admins seeing a join screen on 404, and 409 now cleanly maps to the joinable “not a member” state.
Given the backend now uses:
- 404 → project missing
- 409 → non‑member (joinable)
- 403 → SECRET project without access
it might be worth double‑checking the 403 behavior here:
- For non‑admins, 403 currently shows the generic workspace projects empty state instead of a “private project” message.
- For admins, 403 shows the same join flow as 409, even though the API is signalling “Forbidden for SECRET”.
If the intent is that SECRET projects are non‑joinable and should show a distinct “private project” message, you could route 403 (especially for non‑admins) through
JoinProject’sisPrivateProjectmode instead of the generic empty state, and reserve the join CTA strictly for 409.Example tweak:
- if (errorStatusCode === 409 || (errorStatusCode === 403 && isWorkspaceAdmin)) - return <JoinProject projectId={projectId} />; + if (errorStatusCode === 409) { + return <JoinProject projectId={projectId} />; + } + + if (errorStatusCode === 403 && !isWorkspaceAdmin) { + // SECRET project without access: private-project messaging, no join button + return <JoinProject projectId={projectId} isPrivateProject />; + }(And you can decide separately whether admins should ever see a join CTA for 403, or if they should also get the private-project view.)
Please confirm the intended UX with backend/product before adjusting this logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx(1 hunks)apps/web/core/components/auth-screens/project/access-restriction.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/web/core/components/auth-screens/project/access-restriction.tsx
📚 Learning: 2025-06-18T09:46:08.566Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7188
File: web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx:40-45
Timestamp: 2025-06-18T09:46:08.566Z
Learning: When reviewing breadcrumb components that accept projectId or similar props, check if empty strings are being passed during loading states, which can result in invalid URLs. The preferred approach is to handle these loading states internally within the component rather than requiring each consumer to manage the loading logic.
Applied to files:
apps/web/core/components/auth-screens/project/access-restriction.tsx
🧬 Code graph analysis (2)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx (4)
apps/web/core/components/settings/mobile/nav.tsx (1)
SettingsMobileNav(14-47)apps/web/core/components/settings/project/sidebar/root.tsx (1)
ProjectSettingsSidebar(18-52)apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
ProjectAuthWrapper(42-144)apps/web/ce/layouts/project-wrapper.tsx (1)
ProjectAuthWrapper(11-20)
apps/web/core/components/auth-screens/project/access-restriction.tsx (3)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)apps/web/core/components/auth-screens/project/join-project.tsx (1)
JoinProject(17-67)packages/propel/src/empty-state/detailed-empty-state.tsx (1)
EmptyStateDetailed(8-54)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx
Show resolved
Hide resolved
Merge branch 'preview' of github.com:makeplane/plane into fix-project-status-code
…sion and enhance error handling - Deleted the old ProjectAccessRestriction component. - Introduced a new ProjectAccessRestriction component with improved error handling and user prompts for joining projects. - Updated translations for new error states in multiple languages.
Merge branch 'preview' of github.com:makeplane/plane into fix-project-status-code
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
141-152: Consider also gating onprojectDetailsErrorto handle stale-permission and API-denied edge casesCurrently, access restriction triggers only when
hasPermissionToCurrentProject === false:const isProjectLoading = (isParentLoading || isProjectDetailsLoading) && !projectDetailsError; if (isProjectLoading) return null; if (!isProjectLoading && hasPermissionToCurrentProject === false) { return ( <ProjectAccessRestriction errorStatusCode={projectDetailsError?.status} ... /> ); }If the local permission cache says the user has access (still
true) but the canonical API has started returning 403/404/409 (e.g., user removed from project, project deleted, or access tightened), this wrapper will renderchildreneven thoughprojectDetailsErrorindicates access should be denied.To make this more robust, you could also gate on the presence of an error:
- if (!isProjectLoading && hasPermissionToCurrentProject === false) { + if (!isProjectLoading && (hasPermissionToCurrentProject === false || projectDetailsError)) { return ( <ProjectAccessRestriction errorStatusCode={projectDetailsError?.status} isWorkspaceAdmin={isWorkspaceAdmin} handleJoinProject={handleJoinProject} isJoinButtonDisabled={isJoiningProject} /> ); }If you go this route, ensure that after a successful join you clear or revalidate the
PROJECT_DETAILSSWR key soprojectDetailsErroris updated to reflect the new 200 response; otherwise the restriction UI could persist even after join.
🧹 Nitpick comments (5)
packages/i18n/src/locales/it/empty-state.ts (1)
30-40: New Italian empty-state keys look correct; consider tiny wording polish (optional).The new
no_accessandinvalid_projectentries are structurally correct, match the expected keys, and the Italian is clear and natural. If you want to polish further (non-blocking):
- Line 32: you could add a period for consistency with other titles:
Sembra che tu non abbia accesso a questo progetto.- Line 33: consider slightly smoother phrasing such as:
Contatta l'amministratore per richiedere l'accesso e poter continuare qui.- Line 34: optionally make the object explicit:
Clicca sul pulsante qui sotto per unirti al progetto.- Line 35:
cta_loadingcould be shortened to something closer to other loading CTAs, e.g.Accesso al progetto in corsoorUnione al progetto...depending on how EN is phrased.All of these are style-only; current copy is perfectly acceptable to ship as is.
packages/i18n/src/locales/ja/empty-state.ts (1)
27-37: Japanese translations look correct and consistent; one minor phrasing tweak is optionalThe new
no_access/invalid_projectentries read naturally and match the tone of nearby Japanese strings. Key names and structure also align with the described access-restriction empty states.If you’d like a slightly clearer phrase, you could make
restricted_descriptionexplicitly reference “this project” instead of “here”:- restricted_description: "管理者に連絡してアクセス権をリクエストすると、ここで作業を続けられます。", + restricted_description: "管理者に連絡してアクセス権をリクエストすると、このプロジェクトで作業を続けられます。",apps/web/core/components/auth-screens/project/project-access-restriction.tsx (1)
6-69: Access restriction logic correctly maps HTTP status to UI statesThe component cleanly centralizes access handling:
409or403+isWorkspaceAdmin→ join CTA usingproject_empty_state.no_access.join_description/cta_*.403non-admin → restricted no-access state usingrestricted_description.- All other statuses (incl.
404) → invalid project state usingproject_empty_state.invalid_project.*.The props shape and translation keys line up with the new i18n entries and
EmptyStateDetailedAPI, and the return paths are exhaustive for the expected statuses.If you want to tighten readability, you could wrap the first
ifbody in braces to avoid a long JSX return on a brace-lessif, but that’s purely stylistic:- if (errorStatusCode === 409 || (errorStatusCode === 403 && isWorkspaceAdmin)) - return ( + if (errorStatusCode === 409 || (errorStatusCode === 403 && isWorkspaceAdmin)) { + return ( <div className="grid h-full w-full place-items-center bg-custom-background-100"> {/* ... */} </div> - ); + ); + }apps/web/app/routes/core.ts (1)
129-213: Project detail routing is normalized around :projectId; verify base path usageThe new
projects/(detail)/[projectId]/layout.tsxhierarchy cleanly centralizes all project-level routes (issues,cycles,modules,views,pages,intake) under:workspaceSlug/projects/:projectId/..., and the file paths for each layout/page match the route strings.One thing to double-check: there is no explicit route here for
:workspaceSlug/projects/:projectIditself. If any links, redirects, or bookmarks rely on the bare project URL (without/issues,/cycles, etc.), you may want either:
- a dedicated route for
:workspaceSlug/projects/:projectId(e.g., redirecting to the default tab), or- a redirect entry similar to the other legacy redirects at the bottom of this file.
apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
47-49: Avoid callinguseUserPermissionstwice; destructure all methods from a single hook callYou invoke
useUserPermissions()on Line 47 and again on Line 49 just to grabjoinProject. This is redundant hook work and slightly harder to maintain.You can destructure everything in one call:
- const { fetchUserProjectInfo, allowPermissions } = useUserPermissions(); - const { fetchProjectDetails } = useProject(); - const { joinProject } = useUserPermissions(); + const { fetchUserProjectInfo, allowPermissions, joinProject } = useUserPermissions(); + const { fetchProjectDetails } = useProject();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/api/plane/app/views/project/base.py(2 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsx(2 hunks)apps/web/app/routes/core.ts(3 hunks)apps/web/core/components/auth-screens/project/join-project.tsx(0 hunks)apps/web/core/components/auth-screens/project/project-access-restriction.tsx(1 hunks)apps/web/core/layouts/auth-layout/project-wrapper.tsx(3 hunks)packages/i18n/src/locales/cs/empty-state.ts(1 hunks)packages/i18n/src/locales/de/empty-state.ts(1 hunks)packages/i18n/src/locales/en/empty-state.ts(1 hunks)packages/i18n/src/locales/es/empty-state.ts(1 hunks)packages/i18n/src/locales/fr/empty-state.ts(1 hunks)packages/i18n/src/locales/id/empty-state.ts(1 hunks)packages/i18n/src/locales/it/empty-state.ts(1 hunks)packages/i18n/src/locales/ja/empty-state.ts(1 hunks)packages/i18n/src/locales/ko/empty-state.ts(1 hunks)packages/i18n/src/locales/pl/empty-state.ts(1 hunks)packages/i18n/src/locales/pt-BR/empty-state.ts(1 hunks)packages/i18n/src/locales/ro/empty-state.ts(1 hunks)packages/i18n/src/locales/ru/empty-state.ts(1 hunks)packages/i18n/src/locales/sk/empty-state.ts(1 hunks)packages/i18n/src/locales/tr-TR/empty-state.ts(1 hunks)packages/i18n/src/locales/ua/empty-state.ts(1 hunks)packages/i18n/src/locales/vi-VN/empty-state.ts(1 hunks)packages/i18n/src/locales/zh-CN/empty-state.ts(1 hunks)packages/i18n/src/locales/zh-TW/empty-state.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/core/components/auth-screens/project/join-project.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/app/views/project/base.py
🧰 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:
packages/i18n/src/locales/de/empty-state.tspackages/i18n/src/locales/ua/empty-state.tsapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsxpackages/i18n/src/locales/es/empty-state.tspackages/i18n/src/locales/vi-VN/empty-state.tspackages/i18n/src/locales/fr/empty-state.tspackages/i18n/src/locales/ro/empty-state.tspackages/i18n/src/locales/cs/empty-state.tspackages/i18n/src/locales/ko/empty-state.tspackages/i18n/src/locales/ru/empty-state.tsapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/core/components/auth-screens/project/project-access-restriction.tsxpackages/i18n/src/locales/pt-BR/empty-state.tspackages/i18n/src/locales/zh-TW/empty-state.tspackages/i18n/src/locales/sk/empty-state.tspackages/i18n/src/locales/tr-TR/empty-state.tspackages/i18n/src/locales/pl/empty-state.tspackages/i18n/src/locales/id/empty-state.tsapps/web/app/routes/core.tspackages/i18n/src/locales/ja/empty-state.tspackages/i18n/src/locales/zh-CN/empty-state.tsapps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsxpackages/i18n/src/locales/en/empty-state.tspackages/i18n/src/locales/it/empty-state.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-05-22T11:21:49.370Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7092
File: web/core/components/issues/issue-detail-widgets/sub-issues/issues-list/root.tsx:109-113
Timestamp: 2025-05-22T11:21:49.370Z
Learning: Both translation keys "sub_work_item.empty_state.list_filters.action" and "sub_work_item.empty_state.sub_list_filters.action" have identical values across all language files in the Plane project, so they can be used interchangeably.
Applied to files:
packages/i18n/src/locales/de/empty-state.tspackages/i18n/src/locales/es/empty-state.tspackages/i18n/src/locales/fr/empty-state.tspackages/i18n/src/locales/zh-TW/empty-state.tspackages/i18n/src/locales/pl/empty-state.tspackages/i18n/src/locales/zh-CN/empty-state.ts
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/app/routes/core.ts
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.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/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsxapps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
Repo: makeplane/plane PR: 6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
Applied to files:
packages/i18n/src/locales/cs/empty-state.tspackages/i18n/src/locales/sk/empty-state.tspackages/i18n/src/locales/pl/empty-state.ts
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.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/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-07-08T13:41:01.659Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/core/components/issues/select/dropdown.tsx:9-11
Timestamp: 2025-07-08T13:41:01.659Z
Learning: The `getProjectLabelIds` function in the label store handles undefined projectId internally, so it's safe to pass undefined values to it without explicit checks in the calling component.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.tsx
📚 Learning: 2025-06-18T09:46:08.566Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7188
File: web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx:40-45
Timestamp: 2025-06-18T09:46:08.566Z
Learning: When reviewing breadcrumb components that accept projectId or similar props, check if empty strings are being passed during loading states, which can result in invalid URLs. The preferred approach is to handle these loading states internally within the component rather than requiring each consumer to manage the loading logic.
Applied to files:
apps/web/core/layouts/auth-layout/project-wrapper.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsx
🧬 Code graph analysis (3)
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsx (2)
apps/web/core/layouts/auth-layout/project-wrapper.tsx (1)
ProjectAuthWrapper(42-157)apps/web/ce/layouts/project-wrapper.tsx (1)
ProjectAuthWrapper(11-20)
apps/web/core/layouts/auth-layout/project-wrapper.tsx (5)
apps/web/ce/layouts/project-wrapper.tsx (2)
ProjectAuthWrapper(11-20)IProjectAuthWrapper(5-9)apps/web/core/constants/fetch-keys.ts (11)
PROJECT_DETAILS(151-152)PROJECT_ME_INFORMATION(154-155)PROJECT_MEMBER_PREFERENCES(181-182)PROJECT_LABELS(157-158)PROJECT_MEMBERS(160-161)PROJECT_STATES(163-164)PROJECT_INTAKE_STATE(166-167)PROJECT_ESTIMATES(169-170)PROJECT_ALL_CYCLES(172-173)PROJECT_MODULES(175-176)PROJECT_VIEWS(178-179)apps/web/core/services/project/project-member.service.ts (1)
fetchProjectMembers(18-24)apps/web/core/services/user.service.ts (1)
joinProject(247-253)apps/web/core/components/auth-screens/project/project-access-restriction.tsx (1)
ProjectAccessRestriction(13-70)
apps/web/core/components/auth-screens/project/project-access-restriction.tsx (4)
apps/web/core/components/comments/comment-reaction.tsx (1)
TProps(13-17)packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)packages/propel/src/empty-state/detailed-empty-state.tsx (1)
EmptyStateDetailed(8-54)packages/i18n/src/store/index.ts (1)
t(223-244)
⏰ 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). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (26)
packages/i18n/src/locales/id/empty-state.ts (1)
28-38: Access/invalid-project empty states look correct and consistentThe new
no_accessandinvalid_projectentries are structurally consistent with the rest ofproject_empty_state, use clear and idiomatic Indonesian, and preserve the expected keys (title,description,cta_*). No changes needed.packages/i18n/src/locales/pt-BR/empty-state.ts (1)
29-39: Newproject_empty_stateentries look correct and consistentThe
no_accessandinvalid_projectblocks follow the existing structure, use clear pt-BR phrasing, and keep tone consistent with other empty states. No structural or key-name issues from an i18n perspective.packages/i18n/src/locales/ro/empty-state.ts (1)
28-38: New project access empty-state messages look correct and consistentThe
no_accessandinvalid_projectentries follow the existing Romanian tone, are grammatically correct, and match the key structure used in other locales (title/description/CTAs). I don’t see any issues here.packages/i18n/src/locales/vi-VN/empty-state.ts (1)
28-38: New project access/invalid-project empty states look correctKeys and structure under
project_empty_state.no_accessand.invalid_projectmatch the expected shape, the Vietnamese wording is clear and idiomatic, and typing remains precise viaas const. No changes needed.packages/i18n/src/locales/ua/empty-state.ts (1)
28-38: LGTM! Clean i18n additions for access control.The Ukrainian translations are well-structured and align with the PR's new access restriction flows. The keys match the expected usage patterns for the ProjectAccessRestriction component and empty states introduced in this PR.
packages/i18n/src/locales/ko/empty-state.ts (1)
27-37: LGTM! Translations align with access control requirements.The structure and keys are correct. The dual-description pattern (
restricted_descriptionfor admin-gated 403,join_descriptionfor member-join 409) properly supports the ProjectAccessRestriction component flows described in the PR objectives.packages/i18n/src/locales/zh-CN/empty-state.ts (2)
27-33: LGTM! Access restriction translations properly structured.The
no_accesstranslation object correctly provides messaging for both access-denial scenarios:
restricted_descriptionfor 403 (SECRET project without permission)join_descriptionfor 409 (non-member attempting access)The structure aligns well with the ProjectAccessRestriction component introduced in this PR.
34-37: LGTM! Invalid project translations correctly structured.The
invalid_projecttranslation object appropriately handles the 404 scenario for missing or non-existent projects, complementing the access restriction flows.packages/i18n/src/locales/zh-TW/empty-state.ts (2)
27-33: LGTM! Access restriction translations align with PR objectives.The
no_accessblock correctly provides Traditional Chinese translations for the new ProjectAccessRestriction UI component. The two description variants (restricted_descriptionfor admin-restricted 403 scenarios andjoin_descriptionfor self-join 409 scenarios) appropriately map to the error-code-specific flows described in the PR objectives.
34-37: LGTM! Invalid project translations handle 404 scenario.The
invalid_projectblock provides appropriate Traditional Chinese translations for when a project cannot be found (404 response per PR objectives). The concise title and description pattern is consistent with other error states in this locale file.packages/i18n/src/locales/fr/empty-state.ts (2)
30-36: LGTM: Access denial translations added correctly.The
no_accesstranslations are well-structured and cover both the restricted access scenario (requiring admin contact) and the join flow scenario, aligning with the PR's centralized access-restriction changes.
37-40: LGTM: Invalid project translations added correctly.The
invalid_projecttranslations provide a clear message for 404 scenarios when a project doesn't exist, matching the PR's error code handling improvements.packages/i18n/src/locales/pl/empty-state.ts (1)
27-37: LGTM! Polish translations added for project access restriction flows.The new
no_accessandinvalid_projecttranslation entries are properly structured and grammatically consistent with the existing Polish translations in the file. They appropriately cover the access denial scenarios described in the PR (403 forbidden, 409 conflict join flows, and 404 missing project).packages/i18n/src/locales/sk/empty-state.ts (1)
27-37: LGTM! Translation structure aligns with PR objectives.The added
no_accessandinvalid_projectentries correctly support the newProjectAccessRestrictioncomponent flows mentioned in the PR objectives. The distinction betweenrestricted_description(for 403 scenarios) andjoin_description(for 409 scenarios) provides appropriate messaging for different access denial cases.packages/i18n/src/locales/ru/empty-state.ts (2)
35-38: Verified: Translations properly integrated in project access flow.The
invalid_projectkey is actively used inapps/web/core/components/auth-screens/project/project-access-restriction.tsx(lines 63-64), where it displays the title and description when a project doesn't exist. The translation structure (title + description, no CTA) is consistent with the 404 scenario and matches all other locale implementations across the codebase.
28-34: LGTM! All i18n keys verified across all 19 language locales and properly integrated in the codebase.The verification confirms that:
Consistency across locales: Both
no_accessandinvalid_projectkeys exist in all 19 language files (cs, de, en, es, fr, id, it, ja, ko, pl, pt-BR, ro, ru, sk, tr-TR, ua, vi-VN, zh-CN, zh-TW) with matching structures.Complete field coverage: The Russian
no_accessblock includes all 5 required fields (title, restricted_description, join_description, cta_primary, cta_loading) andinvalid_projectincludes both title and description, matching the English version exactly.Active component integration: Both key sets are properly used in
apps/web/core/components/auth-screens/project/project-access-restriction.tsxfor handling dual access restriction scenarios (join-eligible flow vs. admin-restricted flow) and project-not-found cases.Translation quality: Russian translations are grammatically correct and contextually appropriate for all use cases.
apps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsx (1)
85-122: I'll verify the review comment by inspecting the code, understanding the state management, and testing the logic flow. Let me start by exploring the file structure and reading the relevant code.
<function_calls>
Explore the file and understand the state management
#!/bin/bashFirst, find the file and check its size
fd -t f "page.tsx" apps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/ 2>/dev/null | head -5
Show file size
wc -l "apps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsx" 2>/dev/null || echo "File not found with that exact path"
Try alternative approach - search for the file
fd -t f -n "page.tsx" | grep -i browse | head -10
</function_calls>
<function_calls>
Read the full page.tsx file content to understand state management
#!/bin/bashRead the specific file
cat "apps/web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/page.tsx"
</function_calls>packages/i18n/src/locales/tr-TR/empty-state.ts (1)
27-37: New project empty-state keys look consistent
no_accessandinvalid_projectentries follow the existing structure and match the English key set; keys and semantics are aligned with other locales.packages/i18n/src/locales/cs/empty-state.ts (1)
27-37: Czech project empty-state additions are structurally correctThe
no_accessandinvalid_projecttranslations mirror the English key structure and fit cleanly intoproject_empty_state.packages/i18n/src/locales/es/empty-state.ts (1)
29-39: Spanish project empty-state translations align with the new keys
project_empty_state.no_accessandproject_empty_state.invalid_projectare added with the correct field set and consistent meaning versus the English source.packages/i18n/src/locales/de/empty-state.ts (1)
29-39: German project empty-state keys are correctly wiredThe
no_accessandinvalid_projectentries use the right key names and structure, matching other locales and the consuming component expectations.packages/i18n/src/locales/en/empty-state.ts (1)
27-37: English source strings for project access states are well defined
project_empty_state.no_accessandproject_empty_state.invalid_projectprovide clear, distinct copy for joinable, restricted, and invalid project cases and establish the correct key surface for other locales.apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/layout.tsx (1)
14-50: ProjectAuthWrapper integration and params usage look correctThe layout now correctly wraps its nested routes in
ProjectAuthWrapperwithworkspaceSlugandprojectIdfromRoute.ComponentProps.params, and forwardsOutletas children. This matches the wrapper’s expected API and respects the establishedparams-based pattern forlayout.tsxfiles.apps/web/app/routes/core.ts (3)
111-117: Workspace Archived Projects route wiring is consistentThe new
:workspaceSlug/projects/archivesroute is correctly nested under the workspace projects layout and points toprojects/(detail)/archiveslayout/page, matching the filesystem structure and keeping archives at workspace scope.
215-246: Per-project archives routes are consistent with the new detail structureThe archives routes for issues, cycles, and modules:
- Use paths of the form
:workspaceSlug/projects/:projectId/archives/....- Map to matching layout/page files under
projects/(detail)/[projectId]/archives/....This keeps archived resources clearly namespaced under the project and matches the updated detail layout strategy.
313-355: Project settings routes now consistently use :projectId with a redirect for legacy URLsThe new settings structure under
settings/projects/[projectId]:
- Provides dedicated routes for project, members, features, states, labels, estimates, and automations, all including
:projectId.- Shares a common
[projectId]/layout.tsx, which aligns with the filesystem-based routing.The redirect
route(":workspaceSlug/projects/:projectId/settings/*", "routes/redirects/core/project-settings.tsx");correctly preserves backward compatibility for old project-settings URLs by delegating to the new
/settings/projects/:projectId/...structure.It’s worth quickly scanning for any hard-coded links still pointing at
/projects/:projectId/settings/...to ensure they’re either updated or intentionally relying on this redirect.Also applies to: 384-387
| // handle join project | ||
| const handleJoinProject = () => { | ||
| setIsJoiningProject(true); | ||
| joinProject(workspaceSlug, projectId) | ||
| .then(() => fetchProjectDetails(workspaceSlug, projectId)) | ||
| .finally(() => setIsJoiningProject(false)); | ||
| }; |
There was a problem hiding this comment.
Handle join-project errors explicitly to avoid unhandled rejections and silent failures
handleJoinProject never catches joinProject failures, so any rejection will be unhandled and the user receives no feedback; only the loading state is reset in finally.
Consider switching to async/await and adding at least minimal error handling (e.g., toast or logging), while still ensuring the loading flag is cleared:
- const handleJoinProject = () => {
- setIsJoiningProject(true);
- joinProject(workspaceSlug, projectId)
- .then(() => fetchProjectDetails(workspaceSlug, projectId))
- .finally(() => setIsJoiningProject(false));
- };
+ const handleJoinProject = async () => {
+ setIsJoiningProject(true);
+ try {
+ await joinProject(workspaceSlug, projectId);
+ await fetchProjectDetails(workspaceSlug, projectId);
+ } catch (error) {
+ // TODO: surface error to the user (toast/log) as per your UX pattern.
+ // console.error("Failed to join project", error);
+ } finally {
+ setIsJoiningProject(false);
+ }
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // handle join project | |
| const handleJoinProject = () => { | |
| setIsJoiningProject(true); | |
| joinProject(workspaceSlug, projectId) | |
| .then(() => fetchProjectDetails(workspaceSlug, projectId)) | |
| .finally(() => setIsJoiningProject(false)); | |
| }; | |
| // handle join project | |
| const handleJoinProject = async () => { | |
| setIsJoiningProject(true); | |
| try { | |
| await joinProject(workspaceSlug, projectId); | |
| await fetchProjectDetails(workspaceSlug, projectId); | |
| } catch (error) { | |
| // TODO: surface error to the user (toast/log) as per your UX pattern. | |
| // console.error("Failed to join project", error); | |
| } finally { | |
| setIsJoiningProject(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/web/core/layouts/auth-layout/project-wrapper.tsx around lines 133 to
139, handleJoinProject currently calls joinProject without catching rejections,
causing unhandled promise rejections and no user feedback; refactor to an async
function using try/catch/finally, await joinProject and fetchProjectDetails,
show minimal error feedback (e.g., call a toast error or console.error with the
caught error), and ensure setIsJoiningProject(false) is invoked in the finally
block so loading state is always cleared.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/app/views/project/base.py (1)
218-230: Consider database query for membership check instead of in-memory list.The current implementation builds a list of all member IDs in memory and performs a containment check. For projects with many members, a direct database query would be more efficient:
is_member = project.members_list and any( pm.member_id == request.user.id for pm in project.members_list )Or alternatively, query the database directly:
is_member = ProjectMember.objects.filter( project=project, member=request.user, is_active=True ).exists()Additionally, the
str()conversions on lines 218 and 220 add overhead. Since bothmember_idandrequest.user.idare UUID objects, you can compare them directly without converting to strings.Apply this diff to use direct type comparison:
-member_ids = [str(project_member.member_id) for project_member in project.members_list] +member_ids = [project_member.member_id for project_member in project.members_list] -if str(request.user.id) not in member_ids: +if request.user.id not in member_ids: if project.network == ProjectNetwork.SECRET.value: return Response( {"error": "You do not have permission"}, status=status.HTTP_403_FORBIDDEN, ) else: return Response( {"error": "You are not a member of this project"}, status=status.HTTP_409_CONFLICT, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/app/views/project/base.py(2 hunks)apps/api/plane/db/models/__init__.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/models/init.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/base.py
⏰ 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). (3)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Cursor Bugbot
| route( | ||
| ":workspaceSlug/projects/:projectId/archives/modules", | ||
| "./(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/modules/page.tsx" | ||
| ), |
There was a problem hiding this comment.
Bug: Project archive routes missing authentication wrapper
The project archive routes (archived issues, cycles, and modules) are now defined as siblings to the [projectId]/layout.tsx layout rather than being nested inside it. Since ProjectAuthWrapper was moved from the deleted (detail)/layout.tsx to [projectId]/layout.tsx, these archive routes no longer have project membership verification. This allows users to potentially access archived project content at paths like /:workspaceSlug/projects/:projectId/archives/issues without proper authorization checks. The archive routes need to be nested inside the [projectId]/layout.tsx layout to maintain the same security protection as other project routes.
…ne#7234) * fix: project error message and status code * fix: incorrect member role check * fix: project error message and status code * fix: improve project permission checks and error handling in ProjectViewSet * feat: enhance project settings layout with better loading strategy and fix all flicker * fix: prevent rendering during project loading in ProjectAuthWrapper * refactor: adjust layout structure in ProjectDetailSettingsLayout and enhance access restriction logic in ProjectAccessRestriction * refactor: replace ProjectAccessRestriction component with updated version and enhance error handling - Deleted the old ProjectAccessRestriction component. - Introduced a new ProjectAccessRestriction component with improved error handling and user prompts for joining projects. - Updated translations for new error states in multiple languages. * fix: enhance error handling in IssueDetailsPage and remove JoinProject component --------- Co-authored-by: Dheeraj Kumar Ketireddy <[email protected]> Co-authored-by: Prateek Shourya <[email protected]>
…ne#7234) * fix: project error message and status code * fix: incorrect member role check * fix: project error message and status code * fix: improve project permission checks and error handling in ProjectViewSet * feat: enhance project settings layout with better loading strategy and fix all flicker * fix: prevent rendering during project loading in ProjectAuthWrapper * refactor: adjust layout structure in ProjectDetailSettingsLayout and enhance access restriction logic in ProjectAccessRestriction * refactor: replace ProjectAccessRestriction component with updated version and enhance error handling - Deleted the old ProjectAccessRestriction component. - Introduced a new ProjectAccessRestriction component with improved error handling and user prompts for joining projects. - Updated translations for new error states in multiple languages. * fix: enhance error handling in IssueDetailsPage and remove JoinProject component --------- Co-authored-by: Dheeraj Kumar Ketireddy <[email protected]> Co-authored-by: Prateek Shourya <[email protected]>
…ne#7234) * fix: project error message and status code * fix: incorrect member role check * fix: project error message and status code * fix: improve project permission checks and error handling in ProjectViewSet * feat: enhance project settings layout with better loading strategy and fix all flicker * fix: prevent rendering during project loading in ProjectAuthWrapper * refactor: adjust layout structure in ProjectDetailSettingsLayout and enhance access restriction logic in ProjectAccessRestriction * refactor: replace ProjectAccessRestriction component with updated version and enhance error handling - Deleted the old ProjectAccessRestriction component. - Introduced a new ProjectAccessRestriction component with improved error handling and user prompts for joining projects. - Updated translations for new error states in multiple languages. * fix: enhance error handling in IssueDetailsPage and remove JoinProject component --------- Co-authored-by: Dheeraj Kumar Ketireddy <[email protected]> Co-authored-by: Prateek Shourya <[email protected]>
Description
This PR will fix the incorrect error code in the project retrieve API.
Type of Change
Note
Aligns backend project retrieve error codes with access rules and introduces a frontend access-restriction flow with routing updates, reduced spinners, and new i18n strings.
ProjectViewSet.retrievepermission checks to allow non-member visibility only per network; return403forSECRETprojects and409for non-members; stop pre-filtering by membership.ProjectNetworkviaplane.db.models.ProjectAuthWrapper; addProjectAccessRestrictionscreen (join/no-access/invalid project states).settings/projects/[projectId]layout.ProjectService.getProjectfor status-based handling.Written by Cursor Bugbot for commit 932843e. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.