Conversation
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds an optional boolean Changes
Sequence DiagramsequenceDiagram
actor User
participant Modal as NewWorkspaceModal
participant Frontend as Frontend Logic
participant API as Renderer/API
participant Backend as create.ts Handler
User->>Modal: Enter title / optionally edit branch name
Modal->>Frontend: generateSlugFromTitle(title)
Frontend-->>Modal: branchSlug
Modal->>Modal: applyPrefix = !branchNameEdited
Modal->>Frontend: request resolvedPrefix (if applyPrefix)
Frontend-->>Modal: resolvedPrefix
Modal->>Modal: branchPreview = applyPrefix ? resolvedPrefix + branchSlug : branchSlug
User->>Modal: Submit
Modal->>API: Create { branchName: branchSlug, applyPrefix }
API->>Backend: create mutation
alt applyPrefix = true
Backend->>Backend: compute branchPrefix from settings & existing branches
Backend->>Backend: withPrefix(branchSlug) -> final branch name
else applyPrefix = false
Backend->>Backend: use branchSlug as-is
end
Backend-->>API: Created workspace
API-->>Modal: Success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 133-142: The preview/prefix logic must match backend collision and
empty-edit behavior: compute branchSlug via sanitizeBranchName(branchName) or
generateSlugFromTitle(title) as you already do, then set applyPrefix to false
not only when branchNameEdited is true, but also when branchSlug is empty or
when the resolvedPrefix would be dropped by backend due to collision; i.e.
change applyPrefix to something like: !branchNameEdited && branchSlug !== '' &&
!isPrefixCollision(resolvedPrefix, branchSlug) (call an existing collision-check
util or small API/utility function that mirrors backend collision logic), and
use that applyPrefix for branchPreview generation so the UI matches backend
behavior (refer to branchNameEdited, sanitizeBranchName, generateSlugFromTitle,
resolvedPrefix, branchPreview).
Comment on lines
+133
to
+142
| const branchSlug = branchNameEdited | ||
| ? sanitizeBranchName(branchName) | ||
| : generatedBranchName; | ||
| : generateSlugFromTitle(title); | ||
|
|
||
| const applyPrefix = !branchNameEdited; | ||
|
|
||
| const branchPreview = | ||
| branchSlug && applyPrefix && resolvedPrefix | ||
| ? `${resolvedPrefix}/${branchSlug}` | ||
| : branchSlug; |
Contributor
There was a problem hiding this comment.
Keep applyPrefix/preview aligned with backend collision logic and empty edits.
Right now applyPrefix depends only on branchNameEdited. If a user clears the branch field (without blur), applyPrefix becomes false and the backend will generate an unprefixed branch. Also, the backend drops the prefix when it collides with an existing branch, but the preview still shows it. Consider factoring in empty slug and prefix collisions to keep UI and backend consistent.
Proposed adjustment (frontend-only)
const branchSlug = branchNameEdited
? sanitizeBranchName(branchName)
: generateSlugFromTitle(title);
- const applyPrefix = !branchNameEdited;
+ const prefixWouldCollide = useMemo(() => {
+ if (!resolvedPrefix || !branchData?.branches?.length) return false;
+ const existingSet = new Set(
+ branchData.branches.map((b) => b.name.toLowerCase()),
+ );
+ return existingSet.has(resolvedPrefix.toLowerCase());
+ }, [resolvedPrefix, branchData?.branches]);
+
+ const applyPrefix = (!branchNameEdited || !branchSlug) && !prefixWouldCollide;
const branchPreview =
branchSlug && applyPrefix && resolvedPrefix
? `${resolvedPrefix}/${branchSlug}`
: branchSlug;📝 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.
Suggested change
| const branchSlug = branchNameEdited | |
| ? sanitizeBranchName(branchName) | |
| : generatedBranchName; | |
| : generateSlugFromTitle(title); | |
| const applyPrefix = !branchNameEdited; | |
| const branchPreview = | |
| branchSlug && applyPrefix && resolvedPrefix | |
| ? `${resolvedPrefix}/${branchSlug}` | |
| : branchSlug; | |
| const branchSlug = branchNameEdited | |
| ? sanitizeBranchName(branchName) | |
| : generateSlugFromTitle(title); | |
| const prefixWouldCollide = useMemo(() => { | |
| if (!resolvedPrefix || !branchData?.branches?.length) return false; | |
| const existingSet = new Set( | |
| branchData.branches.map((b) => b.name.toLowerCase()), | |
| ); | |
| return existingSet.has(resolvedPrefix.toLowerCase()); | |
| }, [resolvedPrefix, branchData?.branches]); | |
| const applyPrefix = (!branchNameEdited || !branchSlug) && !prefixWouldCollide; | |
| const branchPreview = | |
| branchSlug && applyPrefix && resolvedPrefix | |
| ? `${resolvedPrefix}/${branchSlug}` | |
| : branchSlug; |
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`
around lines 133 - 142, The preview/prefix logic must match backend collision
and empty-edit behavior: compute branchSlug via sanitizeBranchName(branchName)
or generateSlugFromTitle(title) as you already do, then set applyPrefix to false
not only when branchNameEdited is true, but also when branchSlug is empty or
when the resolvedPrefix would be dropped by backend due to collision; i.e.
change applyPrefix to something like: !branchNameEdited && branchSlug !== '' &&
!isPrefixCollision(resolvedPrefix, branchSlug) (call an existing collision-check
util or small API/utility function that mirrors backend collision logic), and
use that applyPrefix for branchPreview generation so the UI matches backend
behavior (refer to branchNameEdited, sanitizeBranchName, generateSlugFromTitle,
resolvedPrefix, branchPreview).
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.