From 1ede7e7e6acd961ece3efce4ff08cf1ed9814c0a Mon Sep 17 00:00:00 2001
From: Stefan de Vogelaere <email@stefandevogelaere.be>
Date: Sun, 18 Jan 2026 14:36:31 +0100
Subject: [PATCH] refactor: extract sanitizeForTestId to shared utility

Address PR review comments by:
- Creating shared sanitizeForTestId utility in apps/ui/src/lib/utils.ts
- Updating ProjectSwitcherItem to use the shared utility
- Adding matching helper to test utils for E2E tests
- Updating all E2E tests to use the sanitization helper

This ensures the component and tests use identical sanitization logic,
making tests robust against project names with special characters.
---
 .../components/project-switcher-item.tsx      | 17 ++---------
 apps/ui/src/lib/utils.ts                      | 28 +++++++++++++++++++
 .../feature-manual-review-flow.spec.ts        |  4 ++-
 .../projects/new-project-creation.spec.ts     |  4 ++-
 .../projects/open-existing-project.spec.ts    |  4 ++-
 apps/ui/tests/utils/core/elements.ts          | 17 +++++++++++
 6 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx b/apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx
index d269001e..f98e05ac 100644
--- a/apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx
+++ b/apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx
@@ -1,6 +1,6 @@
 import { Folder, LucideIcon } from 'lucide-react';
 import * as LucideIcons from 'lucide-react';
-import { cn } from '@/lib/utils';
+import { cn, sanitizeForTestId } from '@/lib/utils';
 import { getAuthenticatedImageUrl } from '@/lib/api-fetch';
 import type { Project } from '@/lib/electron';
 
@@ -37,22 +37,9 @@ export function ProjectSwitcherItem({
   const IconComponent = getIconComponent();
   const hasCustomIcon = !!project.customIconPath;
 
-  // Create a sanitized project name for test ID:
-  // - Convert to lowercase
-  // - Replace spaces with hyphens
-  // - Remove all non-alphanumeric characters except hyphens
-  // - Collapse multiple hyphens into single hyphen
-  // - Trim leading/trailing hyphens
-  const sanitizedName = project.name
-    .toLowerCase()
-    .replace(/\s+/g, '-')
-    .replace(/[^a-z0-9-]/g, '')
-    .replace(/-+/g, '-')
-    .replace(/^-|-$/g, '');
-
   // Combine project.id with sanitized name for uniqueness and readability
   // Format: project-switcher-{id}-{sanitizedName}
-  const testId = `project-switcher-${project.id}-${sanitizedName}`;
+  const testId = `project-switcher-${project.id}-${sanitizeForTestId(project.name)}`;
 
   return (
     <button
diff --git a/apps/ui/src/lib/utils.ts b/apps/ui/src/lib/utils.ts
index 27da4859..933ea1fd 100644
--- a/apps/ui/src/lib/utils.ts
+++ b/apps/ui/src/lib/utils.ts
@@ -125,6 +125,34 @@ export const isMac =
       (/Mac/.test(navigator.userAgent) ||
         (navigator.platform ? navigator.platform.toLowerCase().includes('mac') : false));
 
+/**
+ * Sanitize a string for use in data-testid attributes.
+ * Creates a deterministic, URL-safe identifier from any input string.
+ *
+ * Transformations:
+ * - Convert to lowercase
+ * - Replace spaces with hyphens
+ * - Remove all non-alphanumeric characters (except hyphens)
+ * - Collapse multiple consecutive hyphens into a single hyphen
+ * - Trim leading/trailing hyphens
+ *
+ * @param name - The string to sanitize (e.g., project name, feature title)
+ * @returns A sanitized string safe for CSS selectors and test IDs
+ *
+ * @example
+ * sanitizeForTestId("My Awesome Project!") // "my-awesome-project"
+ * sanitizeForTestId("test-project-123")    // "test-project-123"
+ * sanitizeForTestId("  Foo  Bar  ")        // "foo-bar"
+ */
+export function sanitizeForTestId(name: string): string {
+  return name
+    .toLowerCase()
+    .replace(/\s+/g, '-')
+    .replace(/[^a-z0-9-]/g, '')
+    .replace(/-+/g, '-')
+    .replace(/^-|-$/g, '');
+}
+
 /**
  * Generate a UUID v4 string.
  *
diff --git a/apps/ui/tests/features/feature-manual-review-flow.spec.ts b/apps/ui/tests/features/feature-manual-review-flow.spec.ts
index 0f5219fc..ab8b077d 100644
--- a/apps/ui/tests/features/feature-manual-review-flow.spec.ts
+++ b/apps/ui/tests/features/feature-manual-review-flow.spec.ts
@@ -21,6 +21,7 @@ import {
   getKanbanColumn,
   authenticateForTests,
   handleLoginScreenIfPresent,
+  sanitizeForTestId,
 } from '../utils';
 
 const TEST_TEMP_DIR = createTempDirPath('manual-review-test');
@@ -132,7 +133,8 @@ test.describe('Feature Manual Review Flow', () => {
 
     // Verify we're on the correct project (project switcher button shows project name)
     // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
-    await expect(page.locator(`[data-testid$="-${projectName}"]`)).toBeVisible({
+    const sanitizedProjectName = sanitizeForTestId(projectName);
+    await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({
       timeout: 10000,
     });
 
diff --git a/apps/ui/tests/projects/new-project-creation.spec.ts b/apps/ui/tests/projects/new-project-creation.spec.ts
index be02edfd..4599e8fe 100644
--- a/apps/ui/tests/projects/new-project-creation.spec.ts
+++ b/apps/ui/tests/projects/new-project-creation.spec.ts
@@ -14,6 +14,7 @@ import {
   authenticateForTests,
   handleLoginScreenIfPresent,
   waitForNetworkIdle,
+  sanitizeForTestId,
 } from '../utils';
 
 const TEST_TEMP_DIR = createTempDirPath('project-creation-test');
@@ -79,7 +80,8 @@ test.describe('Project Creation', () => {
     // Wait for project to be set as current and visible on the page
     // The project name appears in the project switcher button
     // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
-    await expect(page.locator(`[data-testid$="-${projectName}"]`)).toBeVisible({
+    const sanitizedProjectName = sanitizeForTestId(projectName);
+    await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({
       timeout: 15000,
     });
 
diff --git a/apps/ui/tests/projects/open-existing-project.spec.ts b/apps/ui/tests/projects/open-existing-project.spec.ts
index aaaf3738..51772a25 100644
--- a/apps/ui/tests/projects/open-existing-project.spec.ts
+++ b/apps/ui/tests/projects/open-existing-project.spec.ts
@@ -18,6 +18,7 @@ import {
   authenticateForTests,
   handleLoginScreenIfPresent,
   waitForNetworkIdle,
+  sanitizeForTestId,
 } from '../utils';
 
 // Create unique temp dir for this test run
@@ -159,7 +160,8 @@ test.describe('Open Project', () => {
     // The project name appears in the project switcher button
     // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
     if (targetProjectName) {
-      await expect(page.locator(`[data-testid$="-${targetProjectName}"]`)).toBeVisible({
+      const sanitizedName = sanitizeForTestId(targetProjectName);
+      await expect(page.locator(`[data-testid$="-${sanitizedName}"]`)).toBeVisible({
         timeout: 15000,
       });
     }
diff --git a/apps/ui/tests/utils/core/elements.ts b/apps/ui/tests/utils/core/elements.ts
index af6d8df9..b46ad31e 100644
--- a/apps/ui/tests/utils/core/elements.ts
+++ b/apps/ui/tests/utils/core/elements.ts
@@ -1,5 +1,22 @@
 import { Page, Locator } from '@playwright/test';
 
+/**
+ * Sanitize a string for use in data-testid selectors.
+ * This mirrors the sanitizeForTestId function in apps/ui/src/lib/utils.ts
+ * to ensure tests use the same sanitization logic as the component.
+ *
+ * @param name - The string to sanitize (e.g., project name)
+ * @returns A sanitized string safe for CSS selectors
+ */
+export function sanitizeForTestId(name: string): string {
+  return name
+    .toLowerCase()
+    .replace(/\s+/g, '-')
+    .replace(/[^a-z0-9-]/g, '')
+    .replace(/-+/g, '-')
+    .replace(/^-|-$/g, '');
+}
+
 /**
  * Get an element by its data-testid attribute
  */