-
Notifications
You must be signed in to change notification settings - Fork 987
web: delete org #1338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
web: delete org #1338
Conversation
WalkthroughImplements organization deletion with soft-delete (tombstoning) functionality. Adds a Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as DeleteOrg Component
participant Dialog as DeleteOrgDialog
participant RPC as OrganisationDelete RPC
participant Backend as Organisations.deleteOrg
participant DB as Database
User->>UI: Click Delete Organization
activate UI
UI->>Dialog: open=true
activate Dialog
User->>Dialog: Enter org name & confirm
Dialog->>Dialog: Validate name matches
rect rgb(180, 220, 200)
Note over Dialog,Backend: Deletion Flow
Dialog->>RPC: deleteOrg({id})
activate RPC
RPC->>Backend: deleteOrg(id)
activate Backend
Backend->>DB: Set org.tombstoneAt = now()
Backend->>DB: Find next non-tombstoned org owned by user
Backend->>DB: Update user.activeOrganizationId
Backend->>DB: Update user.defaultOrgId
DB-->>Backend: Success
end
Backend-->>RPC: Success
deactivate Backend
RPC-->>Dialog: Success response
deactivate RPC
rect rgb(200, 220, 180)
Note over Dialog: Post-deletion
Dialog->>Dialog: Show success toast
Dialog->>Dialog: Close dialog
Dialog->>UI: Navigate to /dashboard/caps
end
deactivate Dialog
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)packages/web-backend/src/Organisations/index.ts (2)
⏰ 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/database/migrations/meta/_journal.json (1)
2-2: Fix migration journal version mismatch.The pipeline failure indicates that the migration journal version must be incremented when adding a new migration entry. Since a new migration with
idx: 6was added, the version should be updated from"5"to"6".Apply this diff:
{ - "version": "5", + "version": "6", "dialect": "mysql",apps/web/app/(org)/dashboard/dashboard-data.ts (1)
102-108: Backend deleteOrg only partially handles active organization reassignment; stale references possible when deleting a user's last organization.The backend implementation (packages/web-backend/src/Organisations/index.ts:51-90) attempts to reassign
activeOrganizationIdto another non-tombstoned organization, but this fails silently if no other organization exists. When a user deletes their only organization:
- The org is tombstoned
- The query for
otherOrgreturns empty (no other owned orgs exist)activeOrganizationIdis not updated and remains pointing to the tombstoned orgWhile the frontend fallback in
dashboard-data.tshandles this case, the backend should defensively clear or nullactiveOrganizationIdwhen no alternative organization exists to prevent stale references in other code paths that directly accessuser.activeOrganizationId.Suggested fix: After the
otherOrgquery, add a fallback update to setactiveOrganizationIdtonull(or a sentinel value) if no other organization is available.
🧹 Nitpick comments (1)
packages/database/schema.ts (1)
183-183: Consider adding an index ontombstoneAt.Since multiple queries now filter by
isNull(organizations.tombstoneAt)(in dashboard-data.ts and page.tsx), an index on this column could improve query performance, especially as the number of tombstoned organizations grows.Apply this diff to add an index:
(table) => ({ ownerIdIndex: index("owner_id_idx").on(table.ownerId), customDomainIndex: index("custom_domain_idx").on(table.customDomain), + tombstoneAtIndex: index("tombstone_at_idx").on(table.tombstoneAt), }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/app/(org)/dashboard/_components/MobileTab.tsx(4 hunks)apps/web/app/(org)/dashboard/dashboard-data.ts(2 hunks)apps/web/app/(org)/dashboard/settings/organization/Organization.tsx(2 hunks)apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx(0 hunks)apps/web/app/s/[videoId]/page.tsx(2 hunks)packages/database/migrations/meta/_journal.json(1 hunks)packages/database/schema.ts(1 hunks)packages/ui/src/components/Button.tsx(1 hunks)packages/web-backend/src/Organisations/OrganisationsRpcs.ts(1 hunks)packages/web-backend/src/Organisations/index.ts(2 hunks)packages/web-domain/src/Organisation.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/ui/src/components/Button.tsxpackages/web-backend/src/Organisations/OrganisationsRpcs.tsapps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsxpackages/web-domain/src/Organisation.tsapps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Organisations/index.tsapps/web/app/(org)/dashboard/settings/organization/Organization.tsxpackages/database/schema.tsapps/web/app/(org)/dashboard/dashboard-data.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/ui/src/components/Button.tsxpackages/web-backend/src/Organisations/OrganisationsRpcs.tsapps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsxpackages/web-domain/src/Organisation.tsapps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Organisations/index.tsapps/web/app/(org)/dashboard/settings/organization/Organization.tsxpackages/database/schema.tsapps/web/app/(org)/dashboard/dashboard-data.ts
packages/ui/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Component files in
packages/uishould use PascalCase naming if they define React/Solid components.
Files:
packages/ui/src/components/Button.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsxapps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/organization/Organization.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/queries.ts : Do not edit auto-generated files named `queries.ts`.
Applied to files:
apps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
🧬 Code graph analysis (8)
packages/web-backend/src/Organisations/OrganisationsRpcs.ts (1)
packages/web-domain/src/Errors.ts (1)
InternalError(3-6)
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsx (1)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)
packages/web-domain/src/Organisation.ts (5)
packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(20-24)packages/web-domain/src/Video.ts (1)
NotFoundError(135-139)packages/web-domain/src/Folder.ts (1)
NotFoundError(17-21)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(35-41)
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx (3)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)packages/web-domain/src/Organisation.ts (3)
Organisation(21-24)OrganisationId(16-18)OrganisationId(19-19)
apps/web/app/(org)/dashboard/_components/MobileTab.tsx (1)
apps/web/components/SignedImageUrl.tsx (1)
SignedImageUrl(11-25)
apps/web/app/s/[videoId]/page.tsx (1)
packages/database/schema.ts (2)
videos(289-344)organizations(172-207)
packages/web-backend/src/Organisations/index.ts (3)
packages/web-domain/src/Organisation.ts (3)
Organisation(21-24)OrganisationId(16-18)OrganisationId(19-19)apps/web/app/Layout/AuthContext.tsx (1)
CurrentUser(6-14)packages/web-domain/src/Authentication.ts (1)
CurrentUser(8-16)
apps/web/app/(org)/dashboard/dashboard-data.ts (1)
packages/database/schema.ts (2)
organizations(172-207)organizationMembers(210-232)
🪛 GitHub Actions: Validate Migrations
packages/database/migrations/meta/_journal.json
[error] 2-2: Migration journal version cannot be changed (was: $BASE_VERSION, now: $CURRENT_VERSION)
⏰ 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 Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (19)
apps/web/app/s/[videoId]/page.tsx (2)
32-32: LGTM!The
isNullimport is correctly added to support tombstone filtering.
304-304: LGTM!The tombstone filtering logic correctly ensures that videos from deleted organizations are not accessible. The use of
andwith the existingeq(videos.id, videoId)condition properly combines the filters.packages/ui/src/components/Button.tsx (1)
19-20: LGTM!The updated destructive variant styling improves consistency with other button variants by adding border support and using the design system's gray palette for disabled states. The transparent border default ensures the button appears borderless when not disabled.
apps/web/app/(org)/dashboard/settings/organization/Organization.tsx (2)
16-16: LGTM!The DeleteOrg component import is correctly placed and follows the naming convention.
94-94: LGTM!The DeleteOrg component is appropriately positioned at the end of the organization settings form, which is a logical placement for a destructive action.
apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx (1)
163-163: LGTM!The background color change from
bg-gray-1tobg-gray-2improves visual contrast for the settings options. This appears to be a deliberate design system refinement.apps/web/app/(org)/dashboard/dashboard-data.ts (2)
74-80: LGTM!The tombstone filtering correctly uses
and(or(...), isNull(organizations.tombstoneAt))to ensure that organizations are filtered by membership/ownership AND are not tombstoned. This prevents deleted organizations from appearing in the dashboard.
328-333: LGTM!The totalInvites query correctly filters out tombstoned organizations when counting organization members and invites. This ensures accurate quota calculations for non-deleted organizations.
packages/web-backend/src/Organisations/OrganisationsRpcs.ts (1)
17-25: Review comment is incorrect based on codebase analysis.The
deleteOrgimplementation performs only a soft delete (markstombstoneAt) and does not involve S3 operations. It contains only database operations—there is no resource cleanup or S3 interaction. The S3Error handling inOrganisationUpdateis specific to that operation, which modifies organization metadata including icon uploads. SinceOrganisationDeletedoes not perform S3 operations, catching onlyDatabaseErroris correct and appropriate.Likely an incorrect or invalid review comment.
packages/web-backend/src/Organisations/index.ts (1)
91-91: LGTM!The return statement correctly exposes both operations.
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsx (1)
23-34: LGTM!The disable logic correctly prevents deletion when only one organization remains, which aligns with the dialog's validation logic.
packages/web-domain/src/Organisation.ts (2)
26-29: LGTM!The
OrganisationDeletepayload structure is clean and follows the existing pattern.
42-45: LGTM!The RPC definition follows the same pattern as
OrganisationUpdatewith appropriate error types and middleware.apps/web/app/(org)/dashboard/_components/MobileTab.tsx (4)
46-52: LGTM!The layout restructuring improves organization of the org controls with better spacing and containment.
78-96: LGTM!The addition of
flex-auto,max-w-[224px], andtruncateclasses ensures the organization selector handles long names gracefully while maintaining responsive layout.
117-118: LGTM!The menu container adjustments provide better width management for the dropdown.
145-152: LGTM!Adding
truncateto organization names in the menu prevents overflow issues with long names.apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx (2)
36-48: LGTM!The success and error handlers appropriately manage toast notifications, dialog state, and navigation.
76-80: LGTM!The button disable conditions correctly prevent deletion when:
- Only one organization remains
- The entered name doesn't match
- Deletion is in progress
This provides good UX and prevents accidental deletions.
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrg.tsx
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx
Show resolved
Hide resolved
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web-backend/src/Organisations/index.ts (1)
81-91: Handle case when user has no remaining organizations.When
otherOrgis undefined (user has no other organizations), the user'sactiveOrganizationIdanddefaultOrgIdremain pointed at the tombstoned organization. This leaves the user in a broken state and could cause failures when accessing organization-scoped features.Apply this diff to set the fields to
nullwhen no other org exists:if (otherOrg) { yield* db.use((db) => db .update(Db.users) .set({ activeOrganizationId: otherOrg.id, defaultOrgId: otherOrg.id, }) .where(Dz.eq(Db.users.id, user.id)), ); + } else { + yield* db.use((db) => + db + .update(Db.users) + .set({ + activeOrganizationId: null, + defaultOrgId: null, + }) + .where(Dz.eq(Db.users.id, user.id)), + ); }
🧹 Nitpick comments (1)
apps/web/app/api/desktop/[...route]/root.ts (1)
215-221: Soft-delete filter correctly implemented.The logic correctly excludes tombstoned organizations while preserving the existing owner-or-member access control. The
and()condition properly combines the tombstone check with the authorization logic, and theLEFT JOINsemantics are maintained.Performance consideration: As the organizations table grows, consider adding an index on
tombstoneAtto optimize this query. Since most organizations are expected to be non-tombstoned, the database may handleIS NULLchecks efficiently, but an index would provide more predictable performance at scale.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx(1 hunks)apps/web/app/api/desktop/[...route]/root.ts(2 hunks)packages/web-backend/src/Organisations/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/dashboard/settings/organization/components/DeleteOrgDialog.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/api/desktop/[...route]/root.tspackages/web-backend/src/Organisations/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/api/desktop/[...route]/root.tspackages/web-backend/src/Organisations/index.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/api/desktop/[...route]/root.ts
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/queries.ts : Do not edit auto-generated files named `queries.ts`.
Applied to files:
apps/web/app/api/desktop/[...route]/root.ts
🧬 Code graph analysis (2)
apps/web/app/api/desktop/[...route]/root.ts (1)
packages/database/schema.ts (2)
organizations(172-207)organizationMembers(210-232)
packages/web-backend/src/Organisations/index.ts (4)
packages/web-domain/src/Organisation.ts (3)
Organisation(21-24)OrganisationId(16-18)OrganisationId(19-19)packages/web-domain/src/Authentication.ts (1)
CurrentUser(8-16)packages/web-domain/src/Policy.ts (2)
Policy(8-12)policy(29-41)packages/database/index.ts (1)
db(18-25)
⏰ 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 Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/web/app/api/desktop/[...route]/root.ts (1)
11-11: LGTM! Necessary imports for tombstone filtering.The addition of
isNullandandimports is correct and required for the soft-delete filtering logic implemented in the/organizationsendpoint.packages/web-backend/src/Organisations/index.ts (2)
2-2: LGTM! Imports support the new delete functionality.The addition of
CurrentUserandPolicyimports is necessary for the authentication and authorization checks in thedeleteOrgfunction.
54-56: Policy check correctly implemented.The ownership verification has been properly added as suggested in the previous review. The implementation correctly ensures the current user owns the organization before allowing deletion.
| }); | ||
|
|
||
| return { update }; | ||
| const deleteOrg = Effect.fn("Organisations.deleteOrg")(function* ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be better named softDelete
This PR: adds the ability to delete an org.
Summary by CodeRabbit
New Features
Bug Fixes
Style