Move getSchemaName to util #13357
Move getSchemaName to util #13357
Conversation
Weiko
force-pushed
the
c--move-get-schema-name-to-util
branch
from
9af249d to
73ef390
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR refactors the workspace schema name generation logic by extracting the getSchemaName method from WorkspaceDataSourceService into a standalone utility function getWorkspaceSchemaName. The change affects 16 files across the codebase, removing unnecessary service dependencies for a simple, deterministic operation.
The refactoring creates two new utility files:
packages/twenty-shared/src/utils/uuidToBase36.ts- A shared utility for UUID to base36 conversionpackages/twenty-server/src/engine/workspace-datasource/utils/get-workspace-schema-name.util.ts- The main utility that generates schema names using the patternworkspace_{uuidToBase36(workspaceId)}
The change systematically replaces service method calls like this.workspaceDataSourceService.getSchemaName(workspaceId) with direct utility calls getWorkspaceSchemaName(workspaceId) across cron jobs, listeners, commands, and other services. This eliminates the need to inject WorkspaceDataSourceService solely for schema name generation, reducing coupling and making the logic more accessible throughout the codebase.
The utility approach is appropriate here since schema name generation is a pure function that doesn't require service state or complex dependencies - it simply transforms a workspace UUID into a standardized database schema name format.
Confidence score: 1/5
- This PR introduces a critical regression that will break development environments
- The shared
uuidToBase36utility removes essential logic for handling 'twenty-' prefixed UUIDs used in development - The original implementation included special handling for dev UUIDs (prefixed with 'twenty-') that added a 'twenty_' prefix to the final schema name, but this logic is completely missing from the new shared utility
18 files reviewed, 1 comment
| @@ -50,7 +51,7 @@ export class WorkspaceDataSourceService { | |||
| * @returns | |||
| */ | |||
| public async createWorkspaceDBSchema(workspaceId: string): Promise<string> { | |||
| const schemaName = this.getSchemaName(workspaceId); | |||
| const schemaName = getWorkspaceSchemaName(workspaceId); | |||
There was a problem hiding this comment.
logic: Critical issue: The shared uuidToBase36 utility doesn't handle 'twenty-' prefixed dev UUIDs. The original method added 'twenty_' prefix for dev environments, but this logic is missing.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:10344 This environment will automatically shut down when the PR is closed or after 5 hours. |
| if (uuid.startsWith('twenty-')) { | ||
| devId = true; | ||
| // Clean dev uuids (twenty-) | ||
| uuid = uuid.replace('twenty-', ''); |
There was a problem hiding this comment.
this logic is legacy, we don't create uuid with "twenty-" prefix anymore (we use "20202020" instead)
|
Unless I'm mistaken this PR introduces a typecheck error on main after being rebased on latest main 🤔 |
|
Fixed in #13374 |
Related to #13357 Ci run on a previous main version, forcing the rebase would have detect the regression
Context
The method was very simple and deterministic, I'm moving its logic to a util so we don't have to import a service to use it.