fix: branch deletion in template copies by querying database directly #2956
Conversation
|
@naaa760 is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReplaced in-memory branch checks with a live database query during deletion, added selection of a valid target branch before deleting an active branch, and simplified the Delete button state and title. The deletion flow now depends on querying remaining branches and switching to a determined target branch when required. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as BranchManagement UI
participant API as api.branch.getByProjectId
participant EE as EditorEngine
U->>UI: Click "Delete branch"
UI->>API: Query remaining branches for project
API-->>UI: Remaining branches list
alt ≤1 remaining
UI-->>U: Error: cannot delete last/only branch
else Active branch being deleted?
alt Yes
UI->>UI: Determine targetBranch (default or first)
alt targetBranch exists
UI->>EE: Switch to targetBranch
else
UI-->>U: Error: no valid target branch
end
UI->>API: Delete branch
API-->>UI: Deletion success
UI-->>U: Update UI (branch removed)
else No
UI->>API: Delete branch
API-->>UI: Deletion success
UI-->>U: Update UI (branch removed)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx (1)
1-1: Add 'use client' directive.This component uses state (
useState), event handlers, and is an observer component, so it must be a client component.As per coding guidelines: observer components must be client components.
Apply this diff to add the directive:
+'use client'; + import { useEditorEngine } from '@/components/store/editor';
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx (1)
76-78: Consider validatingprojectIdbefore querying.If
editorEngine.projectIdis undefined or null, the query will fail with a less clear error message.Apply this diff to add validation:
try { setIsDeleting(true); + if (!editorEngine.projectId) { + throw new Error('No project selected'); + } + // Check if this is the last branch by querying the database directly const remainingBranches = await api.branch.getByProjectId.query({ projectId: editorEngine.projectId });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx (1)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)
🔇 Additional comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/branches-tab/branch-management.tsx (1)
199-200: LGTM! Simplified Delete button logic.Moving the last-branch validation to runtime in
handleDeleteallows for a cleaner button implementation. Users get a clear error message if they attempt an invalid deletion, which is better UX than a disabled button with no explanation.
Description
fixes: #2896
Related Issues
Type of Change
Summary by CodeRabbit