e9d5d71cd33a4fe921bfa11da514502300ccf865
25 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul Rastoin
|
0b8368cd6c |
Refactor search vector field (#21947)
# Introduction Refactoring the search vector field validation <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21947?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Thomas Trompette
|
22baf2c6c5 |
fix(server): prevent enum migration failure for long identifier names (#21748)
Fixes https://github.com/twentyhq/twenty/issues/20524 ## Problem Adding (or renaming/removing) an option on a `SELECT` / `MULTI_SELECT` field failed for fields whose object + field name combination is long, surfacing to the user only as: > Migration action 'update' for 'fieldMetadata' failed — Migration execution failed. The real underlying Postgres error was: ``` type "_personalInsurancePolicyOrQuote_insuranceCoverageClassification" already exists ALTER TYPE "..."."_personalInsurancePolicyOrQuote_insuranceCoverageClassifications_enum" RENAME TO "..._insuranceCoverageClassifications_enum_old" ``` ## Root cause PostgreSQL truncates identifiers to **63 bytes** (`NAMEDATALEN - 1`). The enum type name `_personalInsurancePolicyOrQuote_insuranceCoverageClassifications_enum` is **69 chars**, so it was already stored truncated to 63 (`..._insuranceCoverageClassification` — the `_enum` suffix chopped off). Multi-select / select option changes go through the rename-and-recreate path in `alterEnumValues`, which renames the enum to `<name>_old`. That candidate is 73 chars → Postgres truncates it back to the **same 63-byte string** as the source → `type "..." already exists`. The failure is deterministic, so every retry on that field failed. The transaction rolls back cleanly, leaving no `_old` artifacts behind. The temporary column name (`<column>_old`) had the same latent bug for very long field names. ## Fix Add `buildTemporaryIdentifier(base, suffix)` to `WorkspaceSchemaEnumManagerService`, which trims the base name so the `_old` suffix survives within 63 bytes and stays distinct from the original. Applied to both the temporary enum name and the temporary column name. The `_old` type/column are transient (dropped within the same transaction), so the trimmed name only needs to fit and not collide — which it now does. ## Test Added `workspace-schema-enum-manager.service.spec.ts` reproducing the exact failing object/field names. Both assertions (target identifier ≤ 63 bytes; truncated source ≠ truncated target) fail on `main` and pass with the fix. ## Recovery No manual cleanup needed for affected workspaces — failed migrations rolled back cleanly. Once deployed, option edits on long-named fields work; the field remained fully usable in the meantime (only option changes were blocked). |
||
|
Paul Rastoin
|
3d49642d12 |
[AUDIT] Run knip over twenty-server (#21159)
# Introduction Run [knip](https://knip.dev/) over twenty-server Used config: ```json { "$schema": "https://unpkg.com/knip@5/schema.json", "workspaces": { "packages/twenty-server": { "entry": [ "src/main.ts", "src/command/command.ts", "src/queue-worker/queue-worker.ts", "src/database/scripts/setup-db.ts", "src/database/scripts/truncate-db.ts", "src/database/clickHouse/migrations/run-migrations.ts", "src/database/clickHouse/seeds/run-seeds.ts", "src/instrument.ts", "lingui.config.ts", "test/integration/graphql/codegen/index.ts", "test/integration/utils/setup-test.ts", "test/integration/utils/teardown-test.ts", "scripts/**/*.ts", "**/*.spec.ts", "**/*.integration-spec.ts" ], "project": ["src/**/*.ts", "test/**/*.ts", "scripts/**/*.ts"], "ignore": [ "src/database/typeorm/**/migrations/**", "src/database/typeorm/**/*.entity.ts", "**/*.workspace-entity.ts", "**/logic-function-resource/constants/seed-project/**" ], "ignoreDependencies": ["@types/psl", "@types/aws-lambda"], "ignoreBinaries": ["nest", "lingui", "typeorm"] } } } ``` |
||
|
Félix Malfait
|
d602f35cbd |
feat(data-model): custom-indexes management UI and mutations (#20846)
## Summary
Brings indexes management into the per-object Settings tab as a section
under Search (no feature flag, advanced mode only). Admins can create /
delete non-unique indexes with the UI; apps can declare indexes in code
with `defineIndex`. Composite-typed fields are now indexable by picking
a specific sub-column (e.g. `Address > City`).
A few related polish items also land here (invite-user dropdown lands on
the Invite tab; standard warning callout above the new-index form).
## What ships
### UI — custom indexes on per-object Settings
- New section directly under Search, wrapped in
`AdvancedSettingsWrapper`.
- Filter dropdown on the search bar toggles system-index visibility
(shown by default since advanced mode).
- **+ Add Index** button (disabled with tooltip once the per-object cap
is reached) navigates to a dedicated `SettingsObjectNewIndex` page
(matches the field-creation pattern, not a modal):
- Field picker mirrors the webhook event-form layout (rows of dropdowns,
implicit trailing empty row).
- Composite fields surface their sub-properties (`Address > City`,
`Currency > Amount`, …).
- BTREE / GIN type selector.
- Standard warning Callout: "Use indexes sparingly — each one speeds
reads but slows writes."
- Trash icon on `isCustom: true` rows → confirmation modal →
`deleteOneIndex`.
### Server — `createOneIndex` / `deleteOneIndex` mutations
- Gated by `SettingsPermissionGuard(DATA_MODEL)`.
- `IndexMetadataService` wraps the existing migration runner via
`WorkspaceMigrationValidateBuildAndRunService` so the metadata row and
the SQL index land atomically.
- Validation: rejects empty fields, duplicate `(fieldMetadataId,
subFieldName)` pairs, fields not on the object, requires `subFieldName`
for composite parents, forbids `subFieldName` on scalar/relation,
enforces `MAX_CUSTOM_INDEXES_PER_OBJECT = 10`.
- Delete refuses on `isCustom: false` rows so system indexes can't be
removed via this API.
- Dedicated GraphQL exception handler maps each typed error to the right
transport error class.
### Composite sub-field indexing
- Adds `subFieldName: string | null` column to
`IndexFieldMetadataEntity` (fast instance command).
- The flat-entity flow (`UniversalFlatIndexFieldMetadata`,
`FlatIndexFieldMetadata`, `from-universal-flat-index-to-flat-index`,
runner column resolution) all carry `subFieldName` through.
- For composite parents, the runner uses
`computeCompositeColumnName({...}, property)` for the picked sub-column;
for non-composite parents, behavior is unchanged.
- The `'::'` separator encodes `(fieldMetadataId, subFieldName)` for
dedup on the wire; the frontend uses the same separator inside the
Select component's string value.
### Apps can declare indexes in code (`defineIndex`)
- New `IndexManifest` + `IndexFieldManifest` types in
`twenty-shared/application` wired into the `Manifest` type.
- `defineIndex` SDK helper + `IndexConfig`. CLI manifest builder +
extractor recognize `defineIndex` / `ManifestEntityKey.Indexes`.
- Server: `from-index-manifest-to-universal-flat-index` converter
resolves field IDs, validates composite/scalar `subFieldName` rules, and
delegates to `generateFlatIndexMetadataWithNameOrThrow` for the
deterministic name.
- Orchestrator wires the loop after the field-resolution pass;
per-object cap enforced inline against the manifest.
- Cascade on uninstall is automatic — when an app disappears its indexes
drop with it (universal-flat-entity diff handles it).
- Rich-app fixture ships a real `defineIndex` on `PostCard.status`,
exercising the full manifest → install path in CI.
### Closed for now (open later if needed)
- Apps cannot declare `isUnique` indexes — unique constraints stay with
the field-creation flow.
- Apps cannot use a partial-`indexWhereClause` — the UI surface keeps
the framework's hardcoded allowlist.
- UI cannot create unique or partial indexes either; same reasons.
### Cleanups along the way
- Reused the existing `getCompositeSubFieldLabel` +
`COMPOSITE_FIELD_SUB_FIELD_LABELS` (deleted the duplicates I'd created
early in the PR).
- Moved `MAX_CUSTOM_INDEXES_PER_OBJECT` to `twenty-shared/constants`
(single source for FE + BE).
- Replaced inline `isDefined(x) && x !== ''` with `isNonEmptyString`
(from `@sniptt/guards`).
- Hoisted the per-object fields Map + inlined the cap counter into the
indexes orchestrator loop (drops the install scan from O(indexes ×
totalFields) to O(totalFields + indexes)).
- Per design-feedback: page-based create flow (not a modal), filter
dropdown on the SearchInput (not a separate toggle), webhook-style
picker, field icons.
### Unrelated polish that lands here
- "Invite user" link in the multi-workspace dropdown now lands on the
Invite tab directly (`#invite`) instead of the first tab of the members
page.
## Test plan
- [ ] `npx nx typecheck twenty-server / twenty-front / twenty-sdk /
twenty-shared` — passes
- [ ] `npx nx lint:diff-with-main twenty-server / twenty-front` — clean
- [ ] `npx jest index-metadata.service.spec` — green
- [ ] `npx jest from-index-manifest-to-universal-flat-index` — green
(new converter spec, 8 cases)
- [ ] `npx vitest run
src/sdk/define/indexes/__tests__/define-index.spec.ts` (twenty-sdk) —
green (6 cases)
- [ ] `npx vitest run --config vitest.integration.config.ts -t
"rich-app"` — green (rich-app app-dev integration exercises the new
manifest path with the PostCard.status index)
- [ ] Advanced mode → Settings → any object → Settings tab → Indexes
section is visible under Search
- [ ] Create a single-field BTREE index, confirm SQL index exists
(verify via `pg_indexes`)
- [ ] Create a composite-field index (`Address > City`) and confirm the
column is `addressAddressCity`
- [ ] Create an index spanning two columns; column order matches the
picker order
- [ ] Attempt to create an 11th custom index → button is disabled with
tooltip
- [ ] Delete a custom index → confirmation modal → row disappears, PG
index dropped
- [ ] System indexes have no trash icon and are hidden by default
|
||
|
Félix Malfait
|
084fa8eaba |
fix(server): auto-index target<X>Id join columns on polymorphic standard objects (#20820)
Closes #20726 ## The bug `timelineActivity` (and the three other polymorphic standard objects — `attachment`, `noteTarget`, `taskTarget`) store relations as N nullable `target<X>Id` columns, one per related object. Each one is a join key queried as `WHERE target<X>Id IN (...) AND deletedAt IS NULL`. For **built-in** related objects (Person, Company, Opportunity, …), each `target<X>Id` column gets a BTREE index, declared statically in `compute-{timelineActivity,attachment,noteTarget,taskTarget}-standard-flat-index-metadata.util.ts`. For **custom** related objects, the same `target<CustomObject>Id` column was added — **without an index**. On a `timelineActivity` table at issue-reporter scale (~21.9M rows, 7.1 GB), this turned record loads into 20–40s sequential scans and produced `QueryFailedError: Query read timeout` for end users. ## Diagnosis The morph/relation field generator (`generateMorphOrRelationFlatFieldMetadataPair`) already creates a BTREE index for the field that owns the join column and returns it alongside the field metadata pair. The two user-driven entry points (`fromRelationCreateFieldInput…`, `fromMorphRelationCreateFieldInput…`) correctly destructure and propagate that index. But the **custom-object creation path** — `buildDefaultRelationFlatFieldMetadatasForCustomObject`, called when a user creates a new custom object — destructured only `{ flatFieldMetadatas }` and threw away `indexMetadatas`. So every `target<CustomObject>Id` column added to the four polymorphic standard objects has been shipping unindexed since custom morph relations went in. ## The fix Three commits. ### 1. `fix(server): index target<CustomObject>Id columns on standard polymorphic objects` 13 lines across 2 files. - `build-default-relation-flat-field-metadatas-for-custom-object.util.ts` — also destructure `indexMetadatas` from the pair generator and accumulate them into the returned record (new field `standardTargetFlatIndexMetadatas`). - `from-create-object-input-to-flat-object-metadata-and-flat-field-metadatas-to-create.util.ts` — append the accumulated indexes to `flatIndexMetadataToCreate`. The migration pipeline at `object-metadata.service.ts:559–562` already passes `flatIndexMetadataToCreate` to the migration runner, so no further wiring is needed. From now on, creating a custom object also creates the four BTREE indexes — one per polymorphic standard object's new `target<CustomObject>Id` column — atomically with the rest of the migration. ### 2. `feat(server): backfill workspace command for relation join column indexes` For existing workspaces whose custom objects were created before the forward-fix. `upgrade:2-8:backfill-relation-join-column-indexes` is a `@RegisteredWorkspaceCommand('2.8.0', 1798100000000)` matching the pattern from `2-7-workspace-command-…-drop-connected-account-standard-object.command.ts`. Per workspace: 1. Load `flatObjectMetadataMaps`, `flatFieldMetadataMaps`, `flatIndexMaps` from the workspace cache. 2. Resolve the four polymorphic standard object IDs by `nameSingular` against `DEFAULT_RELATIONS_OBJECTS_STANDARD_IDS`. 3. Collect every field ID that's already covered by any existing index. 4. Filter `flatFieldMetadataMaps` to MORPH_RELATION fields on those four objects whose `settings.relationType === MANY_TO_ONE` (i.e. owns a join column) and whose ID isn't in the indexed set. 5. Generate a BTREE `UniversalFlatIndexMetadata` for each via `generateIndexForFlatFieldMetadata` (same helper the forward-fix uses). 6. Create the indexes in the workspace schema with **CONCURRENTLY** (see commit 3). 7. Submit the metadata through `WorkspaceMigrationValidateBuildAndRunService` so it lands in `indexMetadata` and the cache — same pipeline as a normal metadata change. The pipeline's own `CREATE INDEX IF NOT EXISTS` no-ops because the index already exists. Properties: - **Idempotent.** Re-running is a no-op once indexes exist. - **Scoped.** Only the four polymorphic standard objects, only their MANY_TO_ONE morph relation fields, only those with no covering index. - **Same code path as the forward-fix.** The backfill produces exactly the indexes the forward-fix would have created at custom-object creation time. - **`--dry-run` supported** via the base `ActiveOrSuspendedWorkspaceCommandRunner`. ### 3. `feat(server): create index CONCURRENTLY in relation join column backfill` Adds an opt-in `concurrently` flag to `WorkspaceSchemaIndexManagerService.createIndex` (threaded through `createIndexInWorkspaceSchema`). When `true`, emits `CREATE INDEX CONCURRENTLY IF NOT EXISTS …`. Defaults to `false` — every existing caller keeps the current transactional `CREATE INDEX` behavior. The backfill command opts in. It creates a QueryRunner **without** `startTransaction()`, issues the CONCURRENTLY indexes one-by-one (each waits for the previous to finish), then submits the metadata through the normal migration pipeline whose own `CREATE INDEX IF NOT EXISTS` is now a no-op. Why not flip the default for the helper: - `CREATE INDEX CONCURRENTLY` cannot run inside a transaction — Postgres errors out. The migration pipeline calls `createIndex` from inside a transactional schema migration. - CONCURRENTLY doesn't roll back with the transaction. If the surrounding migration fails, the index remains and you end up with metadata/schema drift. - Failed CONCURRENTLY builds leave an INVALID index behind that needs manual `DROP`. - UNIQUE indexes have different failure semantics under CONCURRENTLY (deferred, not immediate). So CONCURRENTLY is opt-in, used only where it's the right tool (post-hoc backfills on populated tables). ## Decisions / tradeoffs - **Single-column BTREE vs partial `WHERE deletedAt IS NULL` vs composite.** Twenty's queries always include `deletedAt IS NULL`. A partial index would be slightly better than a plain BTREE (smaller, no wasted seeks on soft-deleted rows). This PR ships single-column to match the existing built-in target index pattern, which already covers >95% of the available speedup (the 20s→4ms drop the reporter saw comes from having any index — composite/partial is a second-order effect). Switching all relation indexes to partial is a separate, broader change. - **CONCURRENTLY operator caveat.** If a CONCURRENTLY build is interrupted (kill, connection drop, OOM), Postgres leaves the index as INVALID. We deliberately don't probe `pg_index` for invalid leftovers on every create — catalog-table queries can be slow at multi-tenant scale and the failure mode is rare. Recovery is manual: `DROP INDEX <name>` and re-run the backfill. - **Forward-fix is not gated** behind a feature flag. The change is metadata-pipeline-internal; before, custom-object creation silently produced a degraded state. After, it produces the correct state. No new public API, no behavioural change for end users besides the indexes existing. ## Risk - Forward-fix: changes only the metadata produced during custom-object creation. New objects get four extra `FlatIndexMetadata` rows and four extra `CREATE INDEX` statements during their creation migration. Tables are empty at that point so the index builds in microseconds. - Helper change: API-compatible, default behavior unchanged. The new `concurrently` parameter is optional. - Backfill: read-only state probe → CONCURRENTLY index creation (no write blocking) → metadata insert via the normal migration pipeline. Idempotent. Reverting is `DROP INDEX`. ## Test plan - [ ] Verify forward-fix: create a custom object, confirm four new BTREE indexes appear on `timelineActivity`, `attachment`, `noteTarget`, `taskTarget` for the new `target<CustomObject>Id` columns, and that `flatIndexMaps` has matching entries. - [ ] Verify backfill on a workspace that had custom objects created before the fix: run `--dry-run` first, confirm the expected indexes are listed; then run for real, confirm the indexes appear in pg (and as `indisvalid = true` in `pg_index`) and in `flatIndexMaps`. Re-run; confirm no-op. - [ ] Verify backfill on a clean workspace: should log "no missing indexes" and exit. - [ ] Verify CONCURRENTLY behavior under load: run backfill against a workspace with active writes on `timelineActivity`; confirm inserts/updates keep working during index build (no `ShareLock` waits in `pg_stat_activity`). - [ ] On the affected reporter-scale workspace, confirm `EXPLAIN ANALYZE` switches from sequential scan to index scan and timeline activity timeouts go away. |
||
|
Weiko
|
cb0b71dbdc |
fix: validate enum values before opening transaction in alterEnumValues (#20376)
## Context The validation throws after startTransaction() and outside the surrounding try. If the empty-enum branch ever fires, a BEGIN is left open on the borrowed QueryRunner and never rolled back by this method the caller has no way of knowing it now owes a ROLLBACK. Whatever the caller does next on that QueryRunner runs inside the leftover transaction, and if its lifecycle ends with a release() instead of a rollbackTransaction(), the connection goes back to the pool with state still pending. |
||
|
Charles Bochet
|
708e53d829 |
Fix multi-select option removal crashing when records contain removed values (#18871)
## Summary
- Fixes a bug where removing an option from a MULTI_SELECT field fails
with `invalid input value for enum` when existing records contain the
removed value alongside surviving values.
- The root cause was the `ELSE` branch in `updateArrayEnum` which tried
to cast removed enum values (e.g. `DISTRIBUTOR`) to the new enum type
that no longer includes them.
- The fix replaces the `ELSE` cast with a NULL-producing implicit CASE
default and uses `array_agg(...) FILTER (WHERE mapped_value IS NOT
NULL)` to silently strip removed values from existing arrays.
### Before (bug)
```sql
-- ELSE branch tries to cast removed value to new enum → crash
CASE unnest_value::text
WHEN 'IMPL' THEN 'IMPL'::new_enum
WHEN 'APP' THEN 'APP'::new_enum
ELSE unnest_value::text::new_enum -- 'DISTRIBUTOR' fails here
END
```
### After (fix)
```sql
-- No ELSE: removed values produce NULL, filtered out by array_agg
SELECT array_agg(mapped_value) FILTER (WHERE mapped_value IS NOT NULL)
FROM (
SELECT CASE unnest_value::text
WHEN 'IMPL' THEN 'IMPL'::new_enum
WHEN 'APP' THEN 'APP'::new_enum
END AS mapped_value
FROM unnest(old_column) AS unnest_value
) enum_mapping
```
|
||
|
Charles Bochet
|
9d57bc39e5 |
Migrate from ESLint to OxLint (#18443)
## Summary Fully replaces ESLint with OxLint across the entire monorepo: - **Replaced all ESLint configs** (`eslint.config.mjs`) with OxLint configs (`.oxlintrc.json`) for every package: `twenty-front`, `twenty-server`, `twenty-emails`, `twenty-ui`, `twenty-shared`, `twenty-sdk`, `twenty-zapier`, `twenty-docs`, `twenty-website`, `twenty-apps/*`, `create-twenty-app` - **Migrated custom lint rules** from ESLint plugin format to OxLint JS plugin system (`@oxlint/plugins`), including `styled-components-prefixed-with-styled`, `no-hardcoded-colors`, `sort-css-properties-alphabetically`, `graphql-resolvers-should-be-guarded`, `rest-api-methods-should-be-guarded`, `max-consts-per-file`, and Jotai-related rules - **Migrated custom rule tests** from ESLint `RuleTester` + Jest to `oxlint/plugins-dev` `RuleTester` + Vitest - **Removed all ESLint dependencies** from `package.json` files and regenerated lockfiles - **Updated Nx targets** (`lint`, `lint:diff-with-main`, `fmt`) in `nx.json` and per-project `project.json` to use `oxlint` commands with proper `dependsOn` for plugin builds - **Updated CI workflows** (`.github/workflows/ci-*.yaml`) — no more ESLint executor - **Updated IDE setup**: replaced `dbaeumer.vscode-eslint` with `oxc.oxc-vscode` extension, configured `source.fixAll.oxc` and format-on-save with Prettier - **Replaced all `eslint-disable` comments** with `oxlint-disable` equivalents across the codebase - **Updated docs** (`twenty-docs`) to reference OxLint instead of ESLint - **Renamed** `twenty-eslint-rules` package to `twenty-oxlint-rules` ### Temporarily disabled rules (tracked in `OXLINT_MIGRATION_TODO.md`) | Rule | Package | Violations | Auto-fixable | |------|---------|-----------|-------------| | `twenty/sort-css-properties-alphabetically` | twenty-front | 578 | Yes | | `typescript/consistent-type-imports` | twenty-server | 3814 | Yes | | `twenty/max-consts-per-file` | twenty-server | 94 | No | ### Dropped plugins (no OxLint equivalent) `eslint-plugin-project-structure`, `lingui/*`, `@stylistic/*`, `import/order`, `prefer-arrow/prefer-arrow-functions`, `eslint-plugin-mdx`, `@next/eslint-plugin-next`, `eslint-plugin-storybook`, `eslint-plugin-react-refresh`. Partial coverage for `jsx-a11y` and `unused-imports`. ### Additional fixes (pre-existing issues exposed by merge) - Fixed `EmailThreadPreview.tsx` broken import from main rename (`useOpenEmailThreadInSidePanel`) - Restored truthiness guard in `getActivityTargetObjectRecords.ts` - Fixed `AgentTurnResolver` return types to match entity (virtual `fileMediaType`/`fileUrl` are resolved via `@ResolveField()`) ## Test plan - [x] `npx nx lint twenty-front` passes - [x] `npx nx lint twenty-server` passes - [x] `npx nx lint twenty-docs` passes - [x] Custom oxlint rules validated with Vitest: `npx nx test twenty-oxlint-rules` - [x] `npx nx typecheck twenty-front` passes - [x] `npx nx typecheck twenty-server` passes - [x] CI workflows trigger correctly with `dependsOn: ["twenty-oxlint-rules:build"]` - [x] IDE linting works with `oxc.oxc-vscode` extension |
||
|
Félix Malfait
|
3bd431e95d |
Use proper PostgreSQL identifier/literal escaping in workspace DDL (#18024)
## Summary - Replace the character-stripping approach (`removeSqlDDLInjection`) with standard PostgreSQL `escapeIdentifier` and `escapeLiteral` functions across all workspace schema manager services - Add missing identifier escaping to `createForeignKey` (was the only method in the FK manager without it) - Add allowlist validation for index WHERE clauses and FK action types - Harden tsvector expression builder with proper identifier quoting ## Context The workspace schema managers build DDL dynamically from metadata (table names, column names, enum values, etc.). The previous approach stripped all non-alphanumeric characters — safe but lossy (silently corrupts values with legitimate special characters). The new approach uses PostgreSQL's standard escaping: - **Identifiers**: double internal `"` and wrap → `"my""table"` (same algorithm as `pg` driver's `escapeIdentifier`) - **Literals**: double internal `'` and wrap → `'it''s a value'` (same algorithm as `pg` driver's `escapeLiteral`) `removeSqlDDLInjection` is kept only for name generation (e.g., `computePostgresEnumName`) where stripping to `[a-zA-Z0-9_]` is the correct behavior. ## Files changed | File | What | |------|------| | `remove-sql-injection.util.ts` | Added `escapeIdentifier` + `escapeLiteral` | | `validate-index-where-clause.util.ts` | New — allowlist for partial index WHERE clauses | | 5 schema manager services | Replaced strip+manual-quote with `escapeIdentifier`/`escapeLiteral` | | `build-sql-column-definition.util.ts` | `escapeIdentifier` for column names, validated `generatedType` | | `sanitize-default-value.util.ts` | `escapeLiteral` instead of stripping | | `serialize-default-value.util.ts` | `escapeLiteral` for values, `escapeIdentifier` for enum casts | | `get-ts-vector-column-expression.util.ts` | `escapeIdentifier` for field names in expressions | | `sanitize-default-value.util.spec.ts` | Updated tests for escape behavior | ## Test plan - [x] All 64 existing tests pass across 6 test suites - [x] `lint:diff-with-main` passes - [x] TypeScript typecheck — no new errors - [ ] Verify workspace sync-metadata still works end-to-end - [ ] Verify custom object/field creation works - [ ] Verify enum field option changes work Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Etienne
|
d7b7e9def1 |
Cascade delete Task targets when tasks deleted - logic + migration command (#17019)
Co-authored-by: prastoin <paul@twenty.com> |
||
|
Paul Rastoin
|
942d2fef83 |
Remove sync-metadata and IS_WORKSPACE_CREATION_V2_ENABLED feature flag (#16997)
# Introduction Followup of https://github.com/twentyhq/twenty/pull/17001#pullrequestreview-3638508738 close https://github.com/twentyhq/core-team-issues/issues/1910 We've completely decom the `sync-metadata` in production. We're now then removing its implementation in favor of the v2. ## TODO: - [x] Remove sync-metadata implem and commands - [x] Remove workspace decorators - [x] Type each deprecated field to deprecated on their workspaceEntity - [x] Remove the `workspace-sync-metadata` folder entirely - [x] remove workspace migration - [x] workspace migration removal migration - [x] remove the `v2` references from workspace manager file names - [x] remove the `v2` references from workspace manager modules - [ ] Double check impact on translation file path updates ## Note - Removed the gate logic - Remains some service v2 naming, serverless needs to be migrated on v2 fully - Removed workspaceMigration service app health consumption, making it always returning up ( no more down ) cc @FelixMalfait ( quite obsolete health check now, will require complete refactor once we introduce inter app dependency etc ) |
||
|
Marie
|
19c9f957b1 |
Improve userFriendlyMessage devX (#16815)
Two challenges with error messages - always provide a useful/meaningful error message for the end user instead of the generic one. eg: show "Wrong password" and not "An error occured" - avoid technical details unless error regards a technical feature. eg: show "An error occured" and not "Invalid post-hook payload."; but do show "Invalid issuer URL." as it occurs while configuring SSO What this PR does - Make userFriendlyMessage mandatory for widely used GraphqlQueryRunnerException and CommonQueryRunnerException, so that developers are forced to ask themselves what the error message should be, and as it contains very wide error codes (eg: "Bad request") which should not be mapped to just one default message - Keep userFriendlyMessage optional for service-specific exceptions (eg: workflowStepExecutorException), but convert the error code to userFriendlyMessage mapper to a switch case function with a typecheck ensuring that all codes are mapped to a message. These default messages are still overridable where they are thrown. |
||
|
Félix Malfait
|
04c596817a |
feat(server): enforce userFriendlyMessage on all exceptions (#16589)
## Summary
This PR enforces that all custom exceptions must provide a
`userFriendlyMessage`, ensuring end users always see readable error
messages.
## Changes
### Core Changes
- **`CustomException` simplified**: Removed the `ForceFriendlyMessage`
generic parameter - `userFriendlyMessage` is now always required
- **Type safety**: The constructor now requires `{ userFriendlyMessage:
MessageDescriptor }` (no longer optional)
### Updated Files
- **74+ exception classes** updated to provide default user-friendly
messages using Lingui `msg` macro
- Each exception class has a sensible fallback message (e.g., `msg\`An
authentication error occurred.\``)
- Exception classes that had code-specific message maps retain their
behavior
## Benefits
- **Compile-time enforcement**: Forgetting to add a user-friendly
message now causes a TypeScript error
- **Better UX**: End users always see a localized, human-readable error
message
- **Simpler API**: No more boolean generic parameter to think about
## Testing
- `npx nx run twenty-server:typecheck` passes
- `npx nx run twenty-server:lint` passes
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Enforces `userFriendlyMessage` on `CustomException` and updates all
exception classes to supply localized default messages, with
filters/tests adjusted accordingly.
>
> - **Core**:
> - Enforce required `userFriendlyMessage` in `CustomException` (remove
optional generic; constructor now requires `{ userFriendlyMessage:
MessageDescriptor }`).
> - **Exceptions**:
> - Update ~70+ exception classes to set default localized messages via
Lingui `msg` maps and pass them in constructors (e.g., `AuthException`,
`ObjectMetadataException`, `FieldMetadataException`, etc.).
> - Add fallback messages where needed (e.g., `INTERNAL_SERVER_ERROR` or
domain-specific defaults).
> - **HTTP/GraphQL Filters**:
> - Ensure fallbacks create `UnknownException` with `msg` for
user-friendly text in REST/GraphQL exception filters.
> - **Tests**:
> - Adjust unit tests to pass `userFriendlyMessage` to exceptions.
> - Update Jest snapshots to include `extensions.userFriendlyMessage` or
message objects where applicable.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
221004fdfc0d97b7d152a258b347bf571e70f10e. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
|
||
|
Paul Rastoin
|
48031a7ce2 |
Runner v2 on cascade delete_field/object + refactor workspace deletion side effect (#15830)
# Introduction While removing the v1 https://github.com/twentyhq/twenty/pull/15823 I've encountered the method `objectMetadataService.deleteObjectsMetadata` that I didn't wanted to migrate as it is and if it's not challenging its existence legitimacy. ## Motivations In a nutshell on a workspace deletion object metadata and field metadata cascading deletion is correclty handled But that's not the case for all of a workspaces entities ( roles, workspaceMigrations ) I suspect that we did not defined the foreignKey explicitly through `typeorm` ## Battle testing v2 I still decided to give a try to a complex operation in the v2 such as a workspace all object metadata deletion. Spoiler it failed due to object being interdependent between them and not being topologically sorted ( morph relation can introduce circular dep anw ). Note: Even after removing the delete field on delete object aggregator we end up with an equivalent circular dep error which an object and its field metadata identifier connection. ## Elegant `DEFERRED` and `DEFERRABLE` foreign keys The most safe, low level solution would be to make all field relations deferrable and start the runner transaction as deferred. But this requires a quite invasive migration of existing FK ## On cascade solution I've opted for the quick fix, on object or field deletion spread cacasde. It's pretty safe as the builder priorly validates the deletion and and its related entities integrity Only for both field and object deletion action types in v2 runner ## Integration coverage Added a test that will scan a new workspace database core schema tables and expect now result after workspace deletion through its only user deletion |
||
|
Paul Rastoin
|
3462a2e288 |
ViewGroup and ViewFilters side effect in v2 (#15096)
# Introduction ### Summary Implements side effect handling for `ViewGroup` and `ViewFilters` when field metadata is updated in the v2 architecture. This ensures that view-related records are properly maintained when enum field options are modified, deleted, or created. ### Side effects - **Side Effect System**: Added side effect handling for field metadata updates that manages related view groups and view filters - **Enum Field Updates**: When enum field options are modified, the system now: - **View Groups**: Creates new groups for added options, updates existing groups for modified options, and deletes groups for removed options - **View Filters**: Updates filter values to reflect option changes and removes filters that reference deleted options ### Enum runner fix Update now works for both atomic enum and array enum ( multi select for instance ) ### Compute flat entity maps from to Standardized this method usage across v2 services Next step is to require dependencies dynamically ## Conclusion closes https://github.com/twentyhq/core-team-issues/issues/1649 |
||
|
Weiko
|
c0ed246a03 |
Add is unique in migration v2 + refactor inferDeletionFromMissingEntities (#15070)
## Goal - inferDeletionFromMissingEntities is now a map instead of a single bool, allowing us to parameterise it based on the entity we want to compare - Adding index creation/update when field isUnique is set to true and index deletion when isUnique is false (for now) close https://github.com/twentyhq/core-team-issues/issues/1346 |
||
|
Weiko
|
f566f39292 |
Add FK creation in migration v2 (#14685)
## Context - packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-default-relation-flat-field-metadatas-for-custom-object.util.ts Fixing targetFlatFieldMetadata not being accurate for default relation during object creation (was not used yet) - packages/twenty-server/src/engine/twenty-orm/workspace-schema-manager/services/workspace-schema-foreign-key-manager.service.ts Simplifying API + Removing unused methods and the ones that were querying pg schema as we want to avoid those as much as possible - packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/workspace-migration-runner-v2/action-handlers/field/services/create-field-action-handler.service.ts Adding FK creation when Join column is created |
||
|
Paul Rastoin
|
2d8669ae71 |
Index v2 runner (#14537)
## Introduction https://github.com/twentyhq/twenty/pull/14363 In this PR we're creating builder and runner for index metadata Removing its previous integration within object builder ## Next: - implem index impact on field and object metadata api - Add integration testing coverage add integration test on index - Implement uniqueness toggle in field metadata api - add integration test on uniqueness related to https://github.com/twentyhq/core-team-issues/issues/1344 |
||
|
Weiko
|
ce3fcb295d | Add traces to transaction rollback exceptions (#14246) | ||
|
Paul Rastoin
|
59d90bedaa |
Integration testing v2 enum field types fail and success path (#14010)
# Introduction Migrating and improving performances of field enum integrations tests success and failing tests cases to be using the new v2 api ## Discovered issue When deleting an object in v1 it will leave related enums until the object is re-created Something not done anymore within the create in v2 but in the delete operation We should implem an upgrade command to remove such relicas ## Bugs - Update/create default value multi select runner wrong sql query -> FIX - Update default value multi select regression, we should allow option without an id to be inserted -> FIX - default value compare dynamic json stringify convertion or not in compare tools for object and fields |
||
|
Weiko
|
2fc29d41a8 |
Fix enum deletion during migration v2 (#13977)
## Context - When deleting an object, we were not properly deleting all the enums, in fact some enums were inside composite fields and should have been handled as well. This was already implemented that way for object update and creation but not deletion. - Fixing an issue where creating a field with an empty string as a default value was skipped and causing an issue with non-nullable columns |
||
|
Weiko
|
43bb8d6043 |
Workspace schema migration runner v2 - Fix Enums and Create TsVector (#13955)
## Context - Adding ts-vector generatedType/asExpression as TS_VECTOR settings - Using those settings to setup properly tsVector searchVector column through the new migration runner - Fix enum creation/suppression Note: regarding the new tsVector, we should implement a command to update existing fields TODO: - TS_VECTOR search vector column update (note: should be properly updated whenever the object labelIdentifier is updated or a new TEXT field is added to the object to follow the current logic) - relation type fields and columns are not implemented yet - index migrations |
||
|
Weiko
|
1707e00ad4 | Workspace schema migration runner v2 (#13899) | ||
|
Félix Malfait
|
8b4b9ef8da |
Change type import rule (#13751)
Forcing "type" to be explicit, works best will rollup on the frontend to exclude depdendencies |
||
|
Weiko
|
689a346c2e |
Implement Workspace Schema Manager Service (#13501)
This PR adds a new set of services that can be used to manage a DB schema (workspace schema in this case) such as creating tables, columns, enums, indexes, FK, etc... Nothing should be new as I've simply reimplemented what we can already do in the current workspace migration runner but this using raw queries for everything as we faced some performances issues in the past using the orm API directly. Some methods are new but were picked from usages in migration commands so I took the liberty to implement them as well in case they are still needed. This is isolated enough to be used in a command but the first use case will be to use it in the new WorkspaceSchemaMigrationRunnerService that will call those methods from the migration instructions |
