-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix(projects): keep nested workspaces out of their repository's row #10922
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,26 @@ export interface ProjectGroupingSettings { | |
|
|
||
| export type ProjectGroupingMode = SidebarProjectGroupingMode; | ||
|
|
||
| /** | ||
| * Maps the legacy "repository_path" preference onto "repository". Both group | ||
| * checkouts of one repository path, so callers and pickers only ever see the | ||
| * two modes that still differ. | ||
| */ | ||
| export function normalizeProjectGroupingMode( | ||
| mode: SidebarProjectGroupingMode, | ||
| ): SidebarProjectGroupingMode { | ||
| return mode === "repository_path" ? "repository" : mode; | ||
| } | ||
|
|
||
| export function selectProjectGroupingSettings(settings: ClientSettings): ProjectGroupingSettings { | ||
| return { | ||
| sidebarProjectGroupingMode: settings.sidebarProjectGroupingMode, | ||
| sidebarProjectGroupingOverrides: settings.sidebarProjectGroupingOverrides, | ||
| sidebarProjectGroupingMode: normalizeProjectGroupingMode(settings.sidebarProjectGroupingMode), | ||
| sidebarProjectGroupingOverrides: Object.fromEntries( | ||
| Object.entries(settings.sidebarProjectGroupingOverrides).map(([key, mode]) => [ | ||
| key, | ||
| normalizeProjectGroupingMode(mode), | ||
| ]), | ||
| ), | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -55,8 +71,12 @@ function deriveRepositoryRelativeProjectPath( | |
| return ""; | ||
| } | ||
|
|
||
| // A repository rooted at a filesystem root ("/" or "c:\\") already ends with | ||
| // its separator; appending another one stops every nested path from matching. | ||
| const separator = normalizedRootPath.includes("\\") ? "\\" : "/"; | ||
| const rootPrefix = `${normalizedRootPath}${separator}`; | ||
| const rootPrefix = normalizedRootPath.endsWith(separator) | ||
| ? normalizedRootPath | ||
| : `${normalizedRootPath}${separator}`; | ||
| if (!normalizedProjectPath.startsWith(rootPrefix)) { | ||
| return null; | ||
| } | ||
|
|
@@ -96,19 +116,20 @@ export function resolveProjectGroupingMode( | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Groups checkouts of one repository path, so the same workspace opened in | ||
| * several environments or worktrees shares a row. Nested workspaces keep their | ||
| * repo-relative path in the key: a monorepo package is its own project, and | ||
| * collapsing it into the repository row would leave no way to target it. | ||
| */ | ||
| function deriveRepositoryScopedKey( | ||
| project: Pick<EnvironmentProject, "workspaceRoot" | "repositoryIdentity">, | ||
| groupingMode: SidebarProjectGroupingMode, | ||
| ): string | null { | ||
| const canonicalKey = project.repositoryIdentity?.canonicalKey; | ||
| if (!canonicalKey) { | ||
| return null; | ||
| } | ||
|
|
||
| if (groupingMode === "repository") { | ||
| return canonicalKey; | ||
| } | ||
|
|
||
| const relativeProjectPath = deriveRepositoryRelativeProjectPath(project); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High For repositories rooted at 🤖 Copy this AI Prompt to have your agent fix this:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in 946a33e.
Two tests cover it, one for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I'm unable to act on this request because you do not have permissions within this repository. |
||
| if (relativeProjectPath === null) { | ||
| return canonicalKey; | ||
|
|
@@ -134,7 +155,7 @@ export function deriveLogicalProjectKey( | |
| } | ||
|
|
||
| return ( | ||
| deriveRepositoryScopedKey(project, groupingMode) ?? | ||
| deriveRepositoryScopedKey(project) ?? | ||
| derivePhysicalProjectKey(project) ?? | ||
| scopedProjectKey(scopeProjectRef(project.environmentId, project.id)) | ||
| ); | ||
|
|
||
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.
🟡 Medium
t3code/apps/mobile/src/features/settings/SettingsProjectGroupingRouteScreen.tsx
Line 34 in 4b63233
When persisted
projectGroupingModeis"repository_path", no radio button is checked, so the settings screen shows grouping enabled with no selected choice.resolveMobileProjectGroupingSettingspreserves this legacy value, butGROUPING_OPTIONSno longer includes it; normalize"repository_path"to"repository"before comparing or render it as therepositoryselection.🤖 Copy this AI Prompt to have your agent fix this:
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.
Confirmed and fixed in 946a33e.
Rather than patch the comparison in the mobile screen, the legacy value is normalized where the preference is read:
selectProjectGroupingSettingson web (mode and per-checkout overrides) andresolveMobileProjectGroupingSettingson mobile both maprepository_pathontorepository. The pickers then only ever see the two modes that still differ, andProjectDefaultsSettings, which reads client settings directly, normalizes its select value too.Covered by a new client-runtime test for the selector and a mobile test for the persisted legacy value.
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.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.