fix(web): Fix /settings/connections throwing a error when there is a git connection present
#588
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces CodeHostType and ConnectionType enums for improved type safety. Creates database migrations converting connectionType and external_codeHostType columns to enum types, updates Prisma schema definitions, and systematically migrates code to use enums instead of string literals, replacing patterns like 'generic-git-host' with 'genericGitHost' and bitbucket variants. Changes
Sequence DiagramNot applicable—this is a systematic type safety refactoring with enum introduction. No new control flows or feature interactions are introduced. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/backend/src/utils.ts (1)
62-67: Leverage the new enum constantWe just promoted these fields to enums so we can stop relying on raw strings. Please switch this comparison to
CodeHostType.genericGitHost(and import the enum) so future refactors stay type-safe.Apply this diff:
-import { PrismaClient, Repo } from "@sourcebot/db"; +import { PrismaClient, Repo, CodeHostType } from "@sourcebot/db"; @@ - if (repo.external_codeHostType === 'genericGitHost' && cloneUrl.protocol === 'file:') { + if (repo.external_codeHostType === CodeHostType.genericGitHost && cloneUrl.protocol === 'file:') {packages/web/src/app/[domain]/components/importSecretDialog.tsx (1)
87-101: Use the enum in the switchNow that
codeHostTypeis strongly typed, we can stop matching on bare strings. Please switch these cases to the correspondingCodeHostType.*members so future renames stay in sync.Here’s a concrete update:
- switch (codeHostType) { - case 'github': + switch (codeHostType) { + case CodeHostType.github: return <GitHubPATCreationStep step={1} />; - case 'gitlab': + case CodeHostType.gitlab: return <GitLabPATCreationStep step={1} />; - case 'bitbucketCloud': + case CodeHostType.bitbucketCloud: return <BitbucketCloudPATCreationStep step={1} />; - case 'bitbucketServer': + case CodeHostType.bitbucketServer: return <BitbucketServerPATCreationStep step={1} />; - case 'gitea': + case CodeHostType.gitea: return <GiteaPATCreationStep step={1} />; - case 'gerrit': + case CodeHostType.gerrit: return null;packages/web/src/app/[domain]/settings/connections/[id]/page.tsx (1)
72-76: Consider adding fallback for Bitbucket Server URL.Line 75 uses a non-null assertion (
config.url!) for Bitbucket Server, which will throw ifconfig.urlis undefined. While the config schema likely enforces this for server deployments, an explicit error or fallback would make the code more defensive.Consider replacing the non-null assertion with an explicit check:
case 'bitbucket': { const config = connection.config as unknown as BitbucketConnectionConfig; if (config.deploymentType === 'cloud') { return config.url ?? 'https://bitbucket.org'; } else { - return config.url!; + return config.url ?? (() => { throw new Error('Bitbucket Server URL is required but not configured') })(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/backend/src/constants.ts(1 hunks)packages/backend/src/repoCompileUtils.ts(5 hunks)packages/backend/src/utils.ts(1 hunks)packages/db/prisma/migrations/20251031012851_change_connection_type_to_enum/migration.sql(1 hunks)packages/db/prisma/migrations/20251031014307_change_repo_code_host_type_to_enum/migration.sql(1 hunks)packages/db/prisma/schema.prisma(4 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/chat/components/demoCards.tsx(1 hunks)packages/web/src/app/[domain]/components/configEditor.tsx(1 hunks)packages/web/src/app/[domain]/components/importSecretDialog.tsx(2 hunks)packages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsx(2 hunks)packages/web/src/app/[domain]/components/pathHeader.tsx(3 hunks)packages/web/src/app/[domain]/components/repositoryCarousel.tsx(1 hunks)packages/web/src/app/[domain]/repos/[id]/page.tsx(1 hunks)packages/web/src/app/[domain]/repos/components/repoBranchesTable.tsx(2 hunks)packages/web/src/app/[domain]/repos/components/reposTable.tsx(6 hunks)packages/web/src/app/[domain]/settings/connections/[id]/page.tsx(3 hunks)packages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsx(3 hunks)packages/web/src/app/[domain]/settings/connections/page.tsx(2 hunks)packages/web/src/app/[domain]/settings/secrets/components/importSecretCard.tsx(1 hunks)packages/web/src/features/chat/components/chatThread/referencedFileSourceListItem.tsx(2 hunks)packages/web/src/features/chat/components/searchScopeIcon.tsx(1 hunks)packages/web/src/features/fileTree/actions.ts(1 hunks)packages/web/src/features/search/schemas.ts(3 hunks)packages/web/src/lib/schemas.ts(2 hunks)packages/web/src/lib/utils.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
packages/web/src/lib/schemas.tspackages/web/src/app/[domain]/components/pathHeader.tsxpackages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsxpackages/web/src/app/[domain]/repos/[id]/page.tsxpackages/web/src/app/[domain]/settings/connections/[id]/page.tsxpackages/web/src/features/search/schemas.tspackages/backend/src/constants.tspackages/db/prisma/migrations/20251031012851_change_connection_type_to_enum/migration.sqlpackages/backend/src/repoCompileUtils.tspackages/web/src/features/chat/components/searchScopeIcon.tsxpackages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsxpackages/web/src/app/[domain]/settings/secrets/components/importSecretCard.tsxpackages/web/src/features/chat/components/chatThread/referencedFileSourceListItem.tsxpackages/db/prisma/schema.prismapackages/backend/src/utils.tspackages/web/src/app/[domain]/settings/connections/page.tsxpackages/web/src/app/[domain]/components/configEditor.tsxpackages/web/src/app/[domain]/components/importSecretDialog.tsxpackages/web/src/app/[domain]/repos/components/reposTable.tsxpackages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsxpackages/web/src/app/[domain]/chat/components/demoCards.tsxpackages/web/src/app/[domain]/components/repositoryCarousel.tsxpackages/db/prisma/migrations/20251031014307_change_repo_code_host_type_to_enum/migration.sqlpackages/web/src/features/fileTree/actions.tspackages/web/src/lib/utils.tspackages/web/src/app/[domain]/repos/components/repoBranchesTable.tsx
🧠 Learnings (1)
📚 Learning: 2025-05-14T19:20:48.667Z
Learnt from: brendan-kellam
Repo: sourcebot-dev/sourcebot PR: 307
File: packages/backend/src/repoCompileUtils.ts:491-503
Timestamp: 2025-05-14T19:20:48.667Z
Learning: SSH/SCP-style Git URLs (like `gitgithub.com:org/repo.git`) throw errors when passed directly to JavaScript's `new URL()` constructor. A fix is to convert them using: `new URL(`ssh://${url.replace(':', '/')}`)` for non-HTTP URLs.
Applied to files:
packages/web/src/app/[domain]/settings/connections/[id]/page.tsxpackages/backend/src/repoCompileUtils.tspackages/backend/src/utils.tspackages/web/src/app/[domain]/repos/components/reposTable.tsxpackages/web/src/features/fileTree/actions.tspackages/web/src/lib/utils.ts
⏰ 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: build
🔇 Additional comments (22)
packages/web/src/app/[domain]/chat/components/demoCards.tsx (1)
13-13: Import migration to new CodeHostType enum location looks good.The change correctly moves the
CodeHostTypeimport source from@/lib/utilsto@sourcebot/dband applies the type to thegetCodeHostIconcall on line 45. This aligns with the PR's goal of introducing enums for improved type safety and resolves the underlying type mismatch that was causing the bug.Ensure that the
getCodeHostIconfunction in@/lib/utilshas been updated to properly handle theCodeHostTypeenum values (including the renamed variants likegenericGitHost). If the function signature or internal logic changed as part of this PR, verify that it correctly maps enum values to the expected icon data.Also applies to: 45-45
packages/web/src/app/[domain]/components/configEditor.tsx (1)
16-16: LGTM! Import source change aligns with type safety improvements.Moving
CodeHostTypefrom@/lib/utilsto@sourcebot/dbis consistent with the PR's objective to centralize enum definitions at the schema level, improving type safety across the codebase.packages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsx (1)
94-99: Ensure fallback handling is consistent withgetCodeHostInfoForRepoupdates.Similar to
repositoryCarousel.tsx, theFileIconfallback has been removed here as well. Please ensure thatgetCodeHostInfoForRepohas been updated to handle all enum values consistently across the codebase. See the verification request inrepositoryCarousel.tsx.packages/backend/src/repoCompileUtils.ts (3)
395-395: LGTM!Explicit typing of
codeHostTypeasCodeHostTypeadds type safety and correctly uses the enum values.
428-429: Good practice mapping enum values to external system expectations.The explicit mapping from enum values (
bitbucketServer/bitbucketCloud) to zoekt's expected format (bitbucket-server/bitbucket-cloud) is correct and well-commented.
511-511: LGTM!The migration from
'generic-git-host'string literal to'genericGitHost'enum value is consistent with the enum definition inschema.prismawhere@map("generic-git-host")handles the database representation.Also applies to: 575-575
packages/db/prisma/migrations/20251031014307_change_repo_code_host_type_to_enum/migration.sql (1)
1-22: Well-documented migration with proper data verification.The migration correctly creates the
CodeHostTypeenum and converts the existing column. The detailed comment and commit references demonstrate that existing data has been verified to match the enum values. The use ofUSING "external_codeHostType"::text::"CodeHostType"will fail fast if any invalid values exist, which is the desired behavior.packages/db/prisma/schema.prisma (2)
32-45: Well-designed enum with proper backwards compatibility.The
CodeHostTypeenum definition with@mapannotations correctly maintains backwards compatibility with the existing database schema. The documentation about PascalCase behavior in Prisma v7 is helpful for future maintainers.Also applies to: 71-71
143-153: ConnectionType enum correctly includes 'git' value to fix issue #586.The
ConnectionTypeenum properly defines the connection types from the v3 config schema. Note thatConnectionTypeandCodeHostTypeare distinct enums with different value sets (e.g.,ConnectionTypehasgitandbitbucket, whileCodeHostTypehasgenericGitHostand separatebitbucketServer/bitbucketCloudvalues).Also applies to: 165-165
packages/web/src/app/[domain]/settings/secrets/components/importSecretCard.tsx (1)
7-7: LGTM!Moving the
CodeHostTypeimport to@sourcebot/dbcentralizes type definitions and is consistent with the broader refactor across the codebase.packages/web/src/lib/schemas.ts (1)
5-5: Excellent type safety improvement!Changing from
z.string()toz.nativeEnum(CodeHostType)adds runtime validation ensuring thatcodeHostTypevalues are valid enum members. This prevents invalid values from propagating through the system.Also applies to: 17-17
packages/web/src/features/fileTree/actions.ts (1)
266-266: LGTM!The update from
'generic-git-host'to'genericGitHost'correctly uses the enum value defined in theCodeHostTypeenum. The logic for handling local repositories remains unchanged.packages/web/src/app/[domain]/repos/components/repoBranchesTable.tsx (1)
18-25: LGTM! Clean enum migration.The transition from string to
CodeHostTypeenum improves type safety. The removal of the type cast at line 43 confirms the prop is now correctly typed.Also applies to: 43-43
packages/web/src/app/[domain]/repos/[id]/page.tsx (1)
68-68: LGTM! Appropriate simplification.Removing the redundant
codeHostInfo &&check is correct sincegetCodeHostInfoForRepoalways returns an object.packages/backend/src/constants.ts (1)
1-10: LGTM! Type safety improvement.Adding the
CodeHostType[]annotation ensures compile-time validation of the supported code host types.packages/web/src/features/chat/components/chatThread/referencedFileSourceListItem.tsx (1)
23-23: LGTM! Consistent enum adoption.The prop type change aligns with the codebase-wide migration to typed enums.
Also applies to: 44-44
packages/web/src/features/search/schemas.ts (1)
2-2: LGTM! Enhanced runtime validation.Using
z.nativeEnum(CodeHostType)provides both compile-time and runtime type safety for code host types in API responses.Also applies to: 37-37, 157-157
packages/web/src/app/[domain]/components/pathHeader.tsx (2)
19-19: LGTM! Prop type aligned with enum migration.The
codeHostTypeprop now correctly uses theCodeHostTypeenum.Also applies to: 30-30
205-211: LGTM! Simplified rendering logic.The code host icon link is now unconditionally rendered, simplifying the component logic.
packages/web/src/app/[domain]/settings/connections/[id]/page.tsx (2)
50-87: LGTM! Correctly uses ConnectionType enum values.The switch statement now correctly handles
ConnectionTypevalues ('git','bitbucket') instead ofCodeHostTypevalues. This fixes the root issue mentioned in the PR objectives where'git'connections were causing errors.
68-68: Review comment is resolved — config schema already enforces URL presence.The v3 config schema requires the url field for both gerrit and git connection types, so the code correctly assumes
config.urlwill be present without needing a fallback. The concern about undefined values is unfounded given the schema's enforcement.packages/db/prisma/migrations/20251031012851_change_connection_type_to_enum/migration.sql (1)
1-14: LGTM. Migration is safe—enum values match schema and all known data values are valid.The migration correctly uses the
USINGclause to safely convert existing text values to the enum type. The seven enum values (github,gitlab,gitea,gerrit,bitbucket,azuredevops,git) match the schema definition inpackages/schemas/src/v3/connection.type.tsexactly. No migrations between adding theconnectionTypecolumn and this enum migration modify its values, and only valid enum values (github) are hardcoded in the application code. TheconfigManager.tsderivesconnectionTypedirectly from the schema type field, further ensuring compatibility.Verify in your production environment that no existing
Connectionrecords have unexpected values before applying this migration.
This PR fixes a exception that was being thrown when viewing the connections page with a
gitconnection. The root-cause was that we were callinggetCodeHostIconwith aconnectionType, where it was meant to accept acodeHostType.Backstory
The
Connectiontable has aconnectionTypecolumn that stores the connection's type (i.e., the code host). This field is sourced from the config schema and can be one of the following values:The
Repotable has aexternal_codeHostTypethat stores the type of code host it belongs to. This is deceptively similar toconnectionType, but for reasons lost to time, the values this field can take on are slightly different:Both
connectionTypeandexternal_codeHostTypewere of typeString, so we didn't have any type guarantees, and there were some places where we were comparing to the wrong values (hence this bug).To help combat this, I've introduced two enums into the schema
ConnectionTypeandCodeHostTypethat explicitly define the valid values forconnectionTypeandexternal_codeHostType, respectively. Since we know all possible values for these fields, the migrations I've added simply map from the string into the corresponding enum value. This adds type safety throughout a lot of paths dealing with these fields.Fixes #586
Summary by CodeRabbit