Deprecate dataSource table with dual-write to workspace.databaseSchema by charlesBochet · Pull Request #19059 · twentyhq/twenty

charlesBochet · 2026-03-28T09:09:30Z

Summary

Starts deprecation of the core.dataSource table by introducing a dual-write system: DataSourceService.createDataSourceMetadata now writes to both core.dataSource and core.workspace.databaseSchema
Migrates read sites (WorkspaceDataSourceService.checkSchemaExists, WorkspaceSchemaFactory, MiddlewareService, WorkspacesMigrationCommandRunner) to read from workspace.databaseSchema instead of querying the dataSource table
Removes the unused databaseUrl field from WorkspaceEntity and drops the column via migration
Adds a 1.20 upgrade command to backfill workspace.databaseSchema from dataSource.schema for existing workspaces

The navigation sidebar was displaying "Notes" instead of "All Notes" for INDEX views because getNavigationMenuItemLabel had a special case that returned objectMetadataItem.labelPlural instead of the already-resolved view.name. Since viewsSelector already resolves {objectLabelPlural} templates, the INDEX special case was redundant and incorrect. Made-with: Cursor

…elpers getNavigationMenuItemLabel and getNavigationMenuItemComputedLink were duplicating logic that already existed in per-type helper functions. Now the central functions are thin dispatchers that delegate to the single source of truth in each per-type utility. Made-with: Cursor

The dataSource table is redundant — the schema name is deterministic from workspaceId via getWorkspaceSchemaName(). This starts the deprecation by: - Dual-writing: DataSourceService.createDataSourceMetadata now writes to both core.dataSource and core.workspace.databaseSchema - Migrating reads: WorkspaceDataSourceService, WorkspaceSchemaFactory, MiddlewareService, and WorkspacesMigrationCommandRunner now read from workspace.databaseSchema instead of querying the dataSource table - Removing databaseUrl from WorkspaceEntity (unused field) - Adding upgrade command to backfill workspace.databaseSchema for existing workspaces from dataSource.schema Made-with: Cursor

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Made-with: Cursor

cubic-dev-ai

3 issues found across 20 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts">

<violation number="1" location="packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts:42">
P1: This new guard incorrectly treats partial `WorkspaceEntity` inputs as missing schema and returns an empty GraphQL schema. It breaks call sites that pass `{ id }` only (e.g. SDK client generation).</violation>
</file>

<file name="packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts">

<violation number="1" location="packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts:23">
P2: VIEW label generation dropped the INDEX-view special case, so object index items no longer use the object plural label.</violation>
</file>

<file name="packages/twenty-server/src/engine/metadata-modules/data-source/data-source.entity.ts">

<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/data-source/data-source.entity.ts:18">
P3: The new deprecation comment is outdated/inaccurate: it says `databaseUrl` is stored on `WorkspaceEntity`, but this PR removes that field.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts

...es/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts

packages/twenty-server/src/engine/metadata-modules/data-source/data-source.entity.ts

packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts

Copilot

Pull request overview

This PR starts deprecating the core.dataSource table by moving the “workspace has a schema/data source” source-of-truth to core.workspace.databaseSchema, with transitional dual-write/backfill to support existing workspaces and updated read paths.

Changes:

Added dual-write behavior so DataSourceService.createDataSourceMetadata updates workspace.databaseSchema while still writing dataSource rows.
Migrated several server read paths (middleware, GraphQL schema factory, workspace migration runner, workspace datasource service) to rely on workspace.databaseSchema instead of querying dataSource.
Removed WorkspaceEntity.databaseUrl (plus a migration to drop the column) and added a 1.20 upgrade command to backfill workspace.databaseSchema.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/twenty-server/src/engine/workspace-datasource/workspace-datasource.service.ts	Reads schema existence from `workspace.databaseSchema` via repo lookup.
packages/twenty-server/src/engine/workspace-datasource/workspace-datasource.module.ts	Switches module wiring to inject `WorkspaceEntity` repo (removes DataSourceModule dependency).
packages/twenty-server/src/engine/middlewares/middleware.service.ts	Uses `workspace.databaseSchema` presence check instead of `dataSource` table read.
packages/twenty-server/src/engine/middlewares/middleware.module.ts	Removes `DataSourceModule` import.
packages/twenty-server/src/engine/metadata-modules/data-source/data-source.service.ts	Adds dual-write to `workspace.databaseSchema` and clears it on delete; deprecates reads.
packages/twenty-server/src/engine/metadata-modules/data-source/data-source.module.ts	Adds `WorkspaceEntity` repository to support dual-write.
packages/twenty-server/src/engine/metadata-modules/data-source/data-source.entity.ts	Adds deprecation note for the entity.
packages/twenty-server/src/engine/core-modules/workspace/workspace.entity.ts	Removes `databaseUrl` field.
packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts	Switches schema-generation gating to `workspace.databaseSchema`.
packages/twenty-server/src/engine/api/graphql/core-graphql-api.module.ts	Removes `DataSourceModule` import.
packages/twenty-server/src/database/typeorm/core/migrations/common/1774688563000-drop-workspace-database-url-column.ts	Drops `core.workspace.databaseUrl` column.
packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts	Registers the new 1.20 backfill command in the upgrade sequence.
packages/twenty-server/src/database/commands/upgrade-version-command/1-20/1-20-upgrade-version-command.module.ts	Wires the new backfill command into the 1.20 command module.
packages/twenty-server/src/database/commands/upgrade-version-command/1-20/1-20-backfill-workspace-database-schema.command.ts	Adds command to backfill `workspace.databaseSchema` from `dataSource.schema`.
packages/twenty-server/src/database/commands/command-runners/workspaces-migration.command-runner.ts	Determines workspace “has datasource” based on `workspace.databaseSchema`.
packages/twenty-front/src/testing/mock-data/users.ts	Removes `databaseUrl` from mock workspace data.
packages/twenty-front/src/modules/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel.ts	Simplifies view label logic to always return view name.
packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts	Refactors label computation into per-type helpers.
packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts	Refactors computed link generation into per-type helpers.
packages/twenty-front/src/modules/navigation-menu-item/display/utils/tests/getNavigationMenuItemLabel.test.ts	Adds unit tests for `getNavigationMenuItemLabel`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T09:15:27Z

packages/twenty-server/src/engine/metadata-modules/data-source/data-source.service.ts

+      databaseSchema: workspaceSchema,
+    });
+
    if (dataSource) {


createDataSourceMetadata updates workspace.databaseSchema but, when a DataSourceEntity already exists, it returns early without ensuring dataSource.schema matches workspaceSchema. This breaks the stated “dual-write keep both tables in sync” behavior and can leave the deprecated table with stale schema values. Consider updating the existing DataSourceEntity when schema differs (or explicitly document that the dataSource row is intentionally immutable and rename the method accordingly).

Suggested change

if (dataSource) {

if (dataSource) {

if (dataSource.schema !== workspaceSchema) {

dataSource.schema = workspaceSchema;

return this.dataSourceMetadataRepository.save(dataSource);

}

Copilot · 2026-03-28T09:15:28Z

packages/twenty-server/src/engine/metadata-modules/data-source/data-source.service.ts

+import { Injectable, Logger } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';

 import { type FindManyOptions, Repository } from 'typeorm';

+import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
 import {
  DataSourceException,
  DataSourceExceptionCode,
 } from 'src/engine/metadata-modules/data-source/data-source.exception';

 import { DataSourceEntity } from './data-source.entity';

+// @deprecated - This service is being deprecated. During the transition,
+// writes go to both the dataSource table and workspace table (dual-write).
+// Reads should progressively migrate to use workspace.databaseSchema
+// or the deterministic getWorkspaceSchemaName(workspaceId) utility.
 @Injectable()
 export class DataSourceService {
+  private readonly logger = new Logger(DataSourceService.name);
+


private readonly logger = new Logger(DataSourceService.name); is currently unused. With typescript/no-unused-vars enabled in the server lint config, this will raise warnings. Either remove the logger property/import, or start using it (e.g., log when dual-writing / deleting).

Copilot · 2026-03-28T09:15:28Z

packages/twenty-server/src/engine/metadata-modules/data-source/data-source.entity.ts

+// databaseSchema and databaseUrl directly on WorkspaceEntity.
+// During the transition, writes go to both tables (dual-write).


The new deprecation comment says WorkspaceEntity stores both databaseSchema and databaseUrl, but this PR removes databaseUrl from WorkspaceEntity. Please update the comment to reflect the current design (schema only), otherwise it becomes misleading during the transition.

Suggested change

// databaseSchema and databaseUrl directly on WorkspaceEntity.

// During the transition, writes go to both tables (dual-write).

// databaseSchema directly on WorkspaceEntity.

// During the transition, writes for the schema field go to both tables (dual-write).

...ase/commands/upgrade-version-command/1-20/1-20-backfill-workspace-database-schema.command.ts

Copilot · 2026-03-28T09:15:28Z

...ase/commands/upgrade-version-command/1-20/1-20-backfill-workspace-database-schema.command.ts

+  private hasRunOnce = false;
+
+  constructor(
+    @InjectRepository(WorkspaceEntity)
+    protected readonly workspaceRepository: Repository<WorkspaceEntity>,
+    protected readonly twentyORMGlobalManager: GlobalWorkspaceOrmManager,
+    protected readonly dataSourceService: DataSourceService,
+    @InjectDataSource()
+    private readonly coreDataSource: DataSource,
+  ) {
+    super(workspaceRepository, twentyORMGlobalManager, dataSourceService);
+  }
+
+  override async runOnWorkspace({
+    options,
+  }: RunOnWorkspaceArgs): Promise<void> {
+    if (this.hasRunOnce) {
+      return;
+    }


This command extends WorkspacesMigrationCommandRunner, which records a success entry per workspace processed. Because of the hasRunOnce guard, the SQL runs only for the first workspace but all subsequent workspaces will still be reported as “success” despite doing nothing. Consider implementing this as a non-per-workspace migration (override runMigrationCommand or use a different runner) so reporting and control flow match the actual behavior.

packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts

The backfill of workspace.databaseSchema from dataSource.schema will be handled differently — removing the upgrade command for now. Made-with: Cursor

sentry · 2026-03-28T09:18:55Z

packages/twenty-server/src/engine/middlewares/middleware.service.ts

+    if (
+      !data.workspace ||
+      !isNonEmptyString(data.workspace.databaseSchema)
+    ) {


Bug: The check for workspace.databaseSchema will fail for existing workspaces because the backfill migration to populate this new field was removed, leaving it as an empty string.
_{Severity: CRITICAL}

Suggested Fix

Reinstate the backfill migration command that was removed. This command should update the workspace.databaseSchema for all existing workspaces by copying the value from the corresponding dataSource.schema. Alternatively, implement another reliable backfill mechanism to ensure existing data is migrated before the new validation logic is enforced.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-server/src/engine/middlewares/middleware.service.ts#L108-L111 Potential issue: The PR migrates several code paths to rely on the new `workspace.databaseSchema` field. However, the backfill command intended to populate this field for existing workspaces was removed. As a result, existing workspaces will have an empty string for `databaseSchema`. The new validation `isNonEmptyString(data.workspace.databaseSchema)` in `middleware.service.ts` will consequently fail for all pre-existing workspaces, breaking REST API requests, GraphQL functionality, and workspace migrations for those installations.

Reads are now controlled by the feature flag: - Flag OFF: reads from the old dataSource table (default, safe) - Flag ON: reads from workspace.databaseSchema (new path) - Writes always go to both tables regardless of the flag This enables a safe rollout: deploy → backfill databaseSchema → enable flag per workspace → eventually drop dataSource table. The command runner keeps using dataSourceService directly since adding FeatureFlagService would cascade to ~45 subclasses and upgrade commands are transitional. Made-with: Cursor

FelixMalfait · 2026-03-28T09:22:09Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:33350

This environment will automatically shut down after 5 hours.

Made-with: Cursor

packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts

cubic-dev-ai

4 issues found across 19 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts">

<violation number="1">
P2: Inlining these label branches duplicates existing utility logic and leaves several helper functions unused. This increases drift risk and adds dead code to maintain.</violation>
</file>

<file name="packages/twenty-server/src/database/typeorm/core/migrations/common/1774688563000-drop-workspace-database-url-column.ts">

<violation number="1" location="packages/twenty-server/src/database/typeorm/core/migrations/common/1774688563000-drop-workspace-database-url-column.ts:29">
P1: The `down` migration will fail: `SET NOT NULL` is applied before the `UPDATE` that converts NULLs to empty strings. Swap the order so the `UPDATE` runs first, matching the reverse of the `up` migration.</violation>
</file>

<file name="packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts">

<violation number="1">
P2: This switch case inlines logic already implemented in dedicated helpers, creating four duplicate implementations and orphaning the original utility functions. Reuse the helpers (or delete/migrate them in the same change) to avoid divergence and dead code.</violation>
</file>

<file name="packages/twenty-server/src/database/commands/command-runners/workspaces-migration.command-runner.ts">

<violation number="1">
P1: This change regresses the migration runner to read from deprecated dataSource metadata instead of `workspace.databaseSchema`, which can break workspace migrations as the dataSource table is phased out.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

.../database/typeorm/core/migrations/common/1774688563000-drop-workspace-database-url-column.ts

...s/twenty-server/src/database/commands/command-runners/workspaces-migration.command-runner.ts

… generated types Made-with: Cursor

sentry · 2026-03-28T09:34:31Z

packages/twenty-server/src/engine/middlewares/middleware.service.ts

+    const hasSchema = isDataSourceMigrated
+      ? isNonEmptyString(data.workspace.databaseSchema)
+      : (


Bug: The removal of a data backfill script will cause workspace.databaseSchema to be NULL for existing workspaces, leading to request failures when the new feature flag is enabled.
_{Severity: CRITICAL}

Suggested Fix

Re-introduce a backfill mechanism to populate the workspace.databaseSchema field for all existing workspaces from their corresponding dataSource.schema records. This backfill must be run before the IS_DATASOURCE_MIGRATED feature flag is enabled in production.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-server/src/engine/middlewares/middleware.service.ts#L122-L124 Potential issue: A backfill command to populate `workspace.databaseSchema` for existing workspaces was removed. The associated migration sets this new `databaseSchema` column to `NULL` by default. When the `IS_DATASOURCE_MIGRATED` feature flag is enabled, a check in `middleware.service.ts` verifies `isNonEmptyString(data.workspace.databaseSchema)`. For all existing workspaces, this check will fail because the value is `NULL`, causing an error 'No data sources found' to be thrown. This will block all requests for users in existing workspaces, effectively breaking the application for them.

FelixMalfait · 2026-03-28T09:38:29Z

📊 API Changes Report

GraphQL Metadata Schema Changes

[log]
Detected the following changes (3) between schemas:

[log] ✖ Field Workspace.databaseSchema changed type from String! to String
[log] ✖ Field databaseUrl was removed from object type Workspace
[log] ⚠ Enum value IS_DATASOURCE_MIGRATED was added to enum FeatureFlagKey
[error] Detected 2 breaking changes
⚠️ Breaking changes or errors detected in GraphQL metadata schema

[log] 
Detected the following changes (3) between schemas:

[log] ✖  Field Workspace.databaseSchema changed type from String! to String
[log] ✖  Field databaseUrl was removed from object type Workspace
[log] ⚠  Enum value IS_DATASOURCE_MIGRATED was added to enum FeatureFlagKey
[error] Detected 2 breaking changes
Error generating diff

⚠️ Please review these API changes carefully before merging.

…n migration order, clean up deprecation comment Made-with: Cursor

…table

…eSchema Made-with: Cursor

sentry · 2026-03-28T09:53:50Z

packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts

+    const hasSchema = isDataSourceMigrated
+      ? isNonEmptyString(workspace.databaseSchema)
+      : (
+          await this.dataSourceService.getDataSourcesMetadataFromWorkspaceId(
+            workspace.id,
+          )
+        ).length > 0;


Bug: When IS_DATASOURCE_MIGRATED is enabled, createGraphQLSchema receives a partial WorkspaceEntity without databaseSchema, causing it to generate an empty schema for the SDK client.
_{Severity: HIGH}

Suggested Fix

In sdk-client-generation.service.ts, before calling workspaceSchemaFactory.createGraphQLSchema, fetch the full WorkspaceEntity object from the database using the workspaceId. This will ensure the databaseSchema field is populated and available for the check, preventing the generation of an empty schema.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts#L52-L58 Potential issue: When the `IS_DATASOURCE_MIGRATED` feature flag is enabled, the `generateSdkClientForApplication` function calls `createGraphQLSchema` with a partial `WorkspaceEntity` object that only contains the `id`. The new logic in `createGraphQLSchema` checks for `workspace.databaseSchema`, which is `undefined` in this case. This causes the function to return an empty `GraphQLSchema`, leading to the silent generation of an empty, non-functional SDK. This breaks the SDK generation functionality for any workspace with this feature flag enabled.

twenty-eng-sync · 2026-03-28T10:29:33Z

Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

charlesBochet added 3 commits March 27, 2026 17:54

Copilot AI review requested due to automatic review settings March 28, 2026 09:09

greptile-apps bot reviewed Mar 28, 2026

View reviewed changes

Copilot started reviewing on behalf of charlesBochet March 28, 2026 09:09 View session

Remove unrelated navigation menu item files from PR

0d091a7

Made-with: Cursor

cubic-dev-ai bot reviewed Mar 28, 2026

View reviewed changes