chore(web): Reorganize git apis #831
Merged
+403
−385
Conversation
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
This comment has been minimized.
This comment has been minimized.
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request refactors Git-related APIs and types by consolidating them into a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ 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 |
Code ReviewI found 3 issues where Issues Found
The Other files in this PR ( Fix: Change from Reference: sourcebot/packages/web/src/features/git/getFileSourceApi.ts Lines 4 to 11 in ff5b11a |
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/features/git/utils.test.ts (1)
34-42:⚠️ Potential issue | 🟡 MinorMinor typo in test name.
The test name contains a grammatical error: "a empty" should be "an empty".
📝 Proposed fix
-test('buildFileTree handles a empty flat list', () => { +test('buildFileTree handles an empty flat list', () => {
🤖 Fix all issues with AI agents
In `@packages/web/src/features/git/getFilesApi.ts`:
- Around line 6-31: The current getFiles function (and similar logic in
getTreeApi and getFolderContentsApi) calls Next.js's notFound() which throws and
results in a 500; instead import and return the service-error notFound value so
the handler returns a ServiceError with 404. Replace usage of next/navigation's
notFound() in getFiles (and the occurrences in getTreeApi and
getFolderContentsApi) with the service-error variant (import the notFound from
the service-error module) and return it (e.g., return notFound) rather than
calling the Next.js function that throws.
In `@packages/web/src/features/git/getFileSourceApi.ts`:
- Around line 45-88: The code resolves the effective git reference into gitRef
but still uses the original ref when building metadata (e.g., getBrowsePath's
revisionName and the returned branch property), causing drift when ref is
undefined; update usages to use the resolved gitRef instead of ref —
specifically pass gitRef into getBrowsePath's revisionName and set the returned
object's branch to gitRef (the other call getCodeHostBrowseFileAtBranchUrl
already uses gitRef for branchName), ensuring the response metadata matches the
actual file content.
In `@packages/web/src/features/git/getFolderContentsApi.ts`:
- Around line 61-64: The lines.map parsing in getFolderContentsApi.ts currently
assumes every `line` contains a comma; check `commaIndex` returned by
`line.indexOf(',')` and handle `-1` explicitly (e.g., skip the malformed line,
or throw/log an error) before computing `type` and `path`; ensure the resulting
`contents: FileTreeItem[]` excludes or marks malformed entries and preserves
proper parsing for valid lines.
- Around line 6-40: The getFolderContents flow uses notFound imported from
next/navigation which throws and gets wrapped into an unexpectedError by sew;
replace that import with the service-error version (import notFound from
'@/lib/serviceError') so calls to notFound inside getFolderContents (and any
early returns inside sew/withOptionalAuthV2) will produce a proper 404
ServiceError; update the top-level import statement and keep all existing uses
(e.g., the notFound() checks after repo lookup and isPathValid) unchanged so sew
will propagate the ServiceError rather than converting it to a 500.
In `@packages/web/src/features/git/getTreeApi.ts`:
- Around line 75-78: Guard against malformed git ls-tree lines in the mapping
that builds flatList: inside the lines.map callback that computes commaIndex,
type and path, check if commaIndex === -1 and handle explicitly (e.g., throw a
clear Error or skip the line with logging) so you don't create empty type or
malformed path values; update the mapping in getTreeApi (the lines.map lambda
that references commaIndex, type, path) to validate the comma and surface a
helpful message including the offending line when it fails.
- Around line 5-46: The getTree handler currently imports notFound from
'next/navigation' which, when thrown inside the sew(...) wrapper, is converted
into a 500; replace that import with the 404 variant from '@/lib/serviceError'
and ensure all uses inside the getTree function (and any other uses in this
file) call the service error notFound so the sew-wrapped API returns a proper
404; update the top-level import and keep calls like return notFound() in the
getTree flow (e.g., the repo existence check and path validation) to use the new
serviceError notFound.
🧹 Nitpick comments (1)
packages/web/src/app/api/(server)/tree/route.ts (1)
3-4: Consider using consistent import style.
getTreeis imported from the direct module path whilegetTreeRequestSchemauses the barrel export. For consistency, both could be imported from@/features/git:♻️ Proposed fix for consistent imports
-import { getTree } from "@/features/git/getTreeApi"; -import { getTreeRequestSchema } from "@/features/git"; +import { getTree, getTreeRequestSchema } from "@/features/git";
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Re-orgs much of the git related apis into
features/git.Also Fixes #830
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.