refactor(twenty-orm): migrate 23 grandfathered entities to WorkspaceScopedRepository (#20987)

## Summary Follow-up to #20953. Migrates 23 of the 30 entities that were left in `WORKSPACE_SCOPED_EXEMPTIONS` last time, so the lint rule's workspaceId-enforcement default now covers most of the core/metadata schema. ### Migrated (23 entities, 88 files, 22 commits) | Family | Entities | |---|---| | Trivial caches | `NavigationMenuItem`, `Skill`, `DataSource`, `Webhook`, `CommandMenuItem`, `IndexMetadata` | | Views | `View`, `ViewField`, `ViewFieldGroup`, `ViewFilter`, `ViewFilterGroup`, `ViewGroup`, `ViewSort` | | Layouts | `PageLayout`, `PageLayoutTab`, `PageLayoutWidget` | | Roles & permissions | `Role`, `RoleTarget`, `PermissionFlag`, `ObjectPermission`, `FieldPermission`, `RowLevelPermissionPredicate`, `RowLevelPermissionPredicateGroup` | For each entity: swap `@InjectRepository(X)` → `@InjectWorkspaceScopedRepository(X)` (and the field type → `WorkspaceScopedRepository<X>`); rewrite every call site to pass `workspaceId` as the first arg (stripped from `where`/criteria — the wrapper throws if you include it now); register `provideWorkspaceScopedRepository(X)` in every owning NestJS module; update affected spec providers to `getWorkspaceScopedRepositoryToken(X)`. ### Rule update - `ApplicationRegistrationVariableEntity` was misclassified — moved to `STRUCTURAL_EXEMPTIONS` (no `workspaceId` column; it's keyed on `applicationRegistrationId` at the instance level). - 22 of the 23 migrated entities removed from `WORKSPACE_SCOPED_EXEMPTIONS` entirely (zero remaining raw `@InjectRepository` sites). - `RoleTargetEntity` also removed; one call site in `user-workspace.service.ts` keeps a raw injection with an `eslint-disable` + reason because `softRemove(...)` is not on the wrapper API yet (the migration would require threading `workspaceId` through `deleteUserWorkspace`'s three callers). ### Still exempted (7 entities, follow-up PRs) | Entity | Why deferred | |---|---| | `ApplicationEntity` | ~50 sites with several cross-workspace lookups by id (auth, OAuth, file-storage, cleanup) | | `CalendarChannelEntity` / `MessageChannelEntity` | Use `.increment(...)` (not on wrapper) and `repository.manager.transaction(...)` — wrapper needs to grow `.increment` + the transaction sites need `withManager` or dual-inject | | `FieldMetadataEntity` / `ObjectMetadataEntity` | The metadata services `extends TypeOrmQueryService<X>` and `super(rawRepo)` — requires dual-inject or reworking the inheritance | | `KeyValuePairEntity` | Allows `workspaceId: IsNull()` for instance-level config; wrapper rejects null | | `UpgradeMigrationEntity` | Same — instance-level + cross-workspace ledger | ## Test plan - [x] `npx nx typecheck twenty-server` — clean - [x] `npx nx lint twenty-server` — clean (0/0) - [x] All 10 affected unit specs pass (115 tests) — api-key, agent-role, permissions, workspace-roles-permissions-cache, view-filter-group, workflow-version-step-operations, two-factor-authentication (service + resolver), user-workspace, file - [ ] Server integration tests in CI
2026-05-28 20:46:21 +02:00
parent 865ca697ca
commit f4ead89956
99 changed files with 1037 additions and 663 deletions
@@ -15,9 +15,16 @@ const createMockRepository = (): jest.Mocked<Repository<FakeEntity>> =>
  ({
    findOne: jest.fn().mockResolvedValue(null),
    findOneOrFail: jest.fn(),
+    findOneBy: jest.fn().mockResolvedValue(null),
    find: jest.fn().mockResolvedValue([]),
    count: jest.fn().mockResolvedValue(0),
+    findAndCount: jest.fn().mockResolvedValue([[], 0]),
+    exists: jest.fn().mockResolvedValue(false),
+    existsBy: jest.fn().mockResolvedValue(false),
+    maximum: jest.fn().mockResolvedValue(null),
    update: jest.fn(),
+    increment: jest.fn(),
+    decrement: jest.fn(),
    delete: jest.fn(),
    softDelete: jest.fn(),
    insert: jest.fn(),
@@ -45,15 +52,22 @@ describe('WorkspaceScopedRepository', () => {
        'findOneOrFail',
        () => scoped.findOneOrFail(undefined as never, { where: {} }),
      ],
+      ['findOneBy', () => scoped.findOneBy(undefined as never, {})],
      ['find', () => scoped.find(undefined as never)],
      ['count', () => scoped.count(undefined as never)],
+      ['findAndCount', () => scoped.findAndCount(undefined as never)],
+      ['exists', () => scoped.exists(undefined as never)],
+      ['existsBy', () => scoped.existsBy(undefined as never, {})],
      ['update', () => scoped.update(undefined as never, {}, {})],
+      ['increment', () => scoped.increment(undefined as never, {}, 'count', 1)],
+      ['decrement', () => scoped.decrement(undefined as never, {}, 'count', 1)],
      ['delete', () => scoped.delete(undefined as never, {})],
      ['softDelete', () => scoped.softDelete(undefined as never, {})],
      ['insert', () => scoped.insert(undefined as never, {})],
      ['upsert', () => scoped.upsert(undefined as never, {}, ['id'])],
      ['save', () => scoped.save(undefined as never, {})],
      ['saveMany', () => scoped.saveMany(undefined as never, [{}])],
+      ['maximum', () => scoped.maximum(undefined as never, 'id')],
    ])('%s throws when workspaceId is undefined', (_name, call) => {
      expect(call).toThrow(/workspaceId must be a non-empty string/);
    });
@@ -136,6 +150,28 @@ describe('WorkspaceScopedRepository', () => {
    });
  });

+  describe('findOneBy', () => {
+    it('merges workspaceId into where', async () => {
+      await scoped.findOneBy(WORKSPACE_ID, { id: 'a' });
+
+      expect(repository.findOneBy).toHaveBeenCalledWith({
+        id: 'a',
+        workspaceId: WORKSPACE_ID,
+      });
+    });
+
+    it('throws if the caller includes workspaceId in where', () => {
+      expect(() =>
+        scoped.findOneBy(WORKSPACE_ID, {
+          id: 'a',
+          workspaceId: OTHER_WORKSPACE_ID,
+        } as never),
+      ).toThrow(/do not include `workspaceId`/);
+
+      expect(repository.findOneBy).not.toHaveBeenCalled();
+    });
+  });
+
  describe('find', () => {
    it('adds workspaceId when no where is provided', async () => {
      await scoped.find(WORKSPACE_ID);
@@ -154,6 +190,53 @@ describe('WorkspaceScopedRepository', () => {
    });
  });

+  describe('findAndCount', () => {
+    it('merges workspaceId into where', async () => {
+      await scoped.findAndCount(WORKSPACE_ID, { where: { status: 'queued' } });
+
+      expect(repository.findAndCount).toHaveBeenCalledWith({
+        where: { status: 'queued', workspaceId: WORKSPACE_ID },
+      });
+    });
+
+    it('works without options', async () => {
+      await scoped.findAndCount(WORKSPACE_ID);
+
+      expect(repository.findAndCount).toHaveBeenCalledWith({
+        where: { workspaceId: WORKSPACE_ID },
+      });
+    });
+  });
+
+  describe('exists', () => {
+    it('merges workspaceId into where', async () => {
+      await scoped.exists(WORKSPACE_ID, { where: { status: 'queued' } });
+
+      expect(repository.exists).toHaveBeenCalledWith({
+        where: { status: 'queued', workspaceId: WORKSPACE_ID },
+      });
+    });
+
+    it('works without options', async () => {
+      await scoped.exists(WORKSPACE_ID);
+
+      expect(repository.exists).toHaveBeenCalledWith({
+        where: { workspaceId: WORKSPACE_ID },
+      });
+    });
+  });
+
+  describe('existsBy', () => {
+    it('merges workspaceId into where', async () => {
+      await scoped.existsBy(WORKSPACE_ID, { id: 'a' });
+
+      expect(repository.existsBy).toHaveBeenCalledWith({
+        id: 'a',
+        workspaceId: WORKSPACE_ID,
+      });
+    });
+  });
+
  describe('update', () => {
    it('merges workspaceId into the criteria, not the patch', async () => {
      await scoped.update(WORKSPACE_ID, { id: 'a' }, { status: 'completed' });
@@ -177,6 +260,41 @@ describe('WorkspaceScopedRepository', () => {
    });
  });

+  describe('increment and decrement', () => {
+    it('increment merges workspaceId into criteria', async () => {
+      await scoped.increment(WORKSPACE_ID, { id: 'a' }, 'count', 1);
+
+      expect(repository.increment).toHaveBeenCalledWith(
+        { id: 'a', workspaceId: WORKSPACE_ID },
+        'count',
+        1,
+      );
+    });
+
+    it('increment throws if the caller includes workspaceId in the criteria', () => {
+      expect(() =>
+        scoped.increment(
+          WORKSPACE_ID,
+          { id: 'a', workspaceId: OTHER_WORKSPACE_ID } as never,
+          'count',
+          1,
+        ),
+      ).toThrow(/do not include `workspaceId`/);
+
+      expect(repository.increment).not.toHaveBeenCalled();
+    });
+
+    it('decrement merges workspaceId into criteria', async () => {
+      await scoped.decrement(WORKSPACE_ID, { id: 'a' }, 'count', 1);
+
+      expect(repository.decrement).toHaveBeenCalledWith(
+        { id: 'a', workspaceId: WORKSPACE_ID },
+        'count',
+        1,
+      );
+    });
+  });
+
  describe('delete and softDelete', () => {
    it('delete merges workspaceId into criteria', async () => {
      await scoped.delete(WORKSPACE_ID, { id: 'a' });
@@ -319,6 +437,25 @@ describe('WorkspaceScopedRepository', () => {
    });
  });

+  describe('maximum', () => {
+    it('calls with scoped criteria when where is provided', async () => {
+      await scoped.maximum(WORKSPACE_ID, 'id', { status: 'queued' });
+
+      expect(repository.maximum).toHaveBeenCalledWith('id', {
+        status: 'queued',
+        workspaceId: WORKSPACE_ID,
+      });
+    });
+
+    it('calls with only workspaceId when no where is given', async () => {
+      await scoped.maximum(WORKSPACE_ID, 'id');
+
+      expect(repository.maximum).toHaveBeenCalledWith('id', {
+        workspaceId: WORKSPACE_ID,
+      });
+    });
+  });
+
  describe('createQueryBuilder', () => {
    it('returns the underlying QueryBuilder unchanged (escape hatch)', () => {
      scoped.createQueryBuilder('t');
@@ -39,6 +39,17 @@ export class WorkspaceScopedRepository<T extends WorkspaceScopedEntity> {
    });
  }

+  findOneBy(
+    workspaceId: string,
+    where: FindOptionsWhere<T>,
+  ): Promise<T | null> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.findOneBy(
+      this.mergeWorkspaceIdIntoCriteria(workspaceId, where),
+    );
+  }
+
  find(workspaceId: string, options?: FindManyOptions<T>): Promise<T[]> {
    this.assertWorkspaceId(workspaceId);

@@ -57,6 +68,51 @@ export class WorkspaceScopedRepository<T extends WorkspaceScopedEntity> {
    });
  }

+  findAndCount(
+    workspaceId: string,
+    options?: FindManyOptions<T>,
+  ): Promise<[T[], number]> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.findAndCount({
+      ...options,
+      where: this.mergeWorkspaceIdIntoWhere(workspaceId, options?.where),
+    });
+  }
+
+  exists(workspaceId: string, options?: FindManyOptions<T>): Promise<boolean> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.exists({
+      ...options,
+      where: this.mergeWorkspaceIdIntoWhere(workspaceId, options?.where),
+    });
+  }
+
+  existsBy(workspaceId: string, where: FindOptionsWhere<T>): Promise<boolean> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.existsBy(
+      this.mergeWorkspaceIdIntoCriteria(workspaceId, where),
+    );
+  }
+
+  maximum(
+    workspaceId: string,
+    columnName: string,
+    where?: FindOptionsWhere<T>,
+  ): Promise<number | null> {
+    this.assertWorkspaceId(workspaceId);
+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    return this.repository.maximum(
+      columnName as any,
+      where
+        ? this.mergeWorkspaceIdIntoCriteria(workspaceId, where)
+        : ({ workspaceId } as FindOptionsWhere<T>),
+    );
+  }
+
  update(
    workspaceId: string,
    criteria: FindOptionsWhere<T>,
@@ -70,6 +126,36 @@ export class WorkspaceScopedRepository<T extends WorkspaceScopedEntity> {
    );
  }

+  increment(
+    workspaceId: string,
+    criteria: FindOptionsWhere<T>,
+    propertyPath: string,
+    value: number | string,
+  ): Promise<UpdateResult> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.increment(
+      this.mergeWorkspaceIdIntoCriteria(workspaceId, criteria),
+      propertyPath,
+      value,
+    );
+  }
+
+  decrement(
+    workspaceId: string,
+    criteria: FindOptionsWhere<T>,
+    propertyPath: string,
+    value: number | string,
+  ): Promise<UpdateResult> {
+    this.assertWorkspaceId(workspaceId);
+
+    return this.repository.decrement(
+      this.mergeWorkspaceIdIntoCriteria(workspaceId, criteria),
+      propertyPath,
+      value,
+    );
+  }
+
  delete(
    workspaceId: string,
    criteria: FindOptionsWhere<T>,
@@ -92,6 +178,14 @@ export class WorkspaceScopedRepository<T extends WorkspaceScopedEntity> {
    );
  }

+  // softRemove / recover / remove are intentionally absent.
+  // TypeORM's entity-based methods use only the primary key in the WHERE
+  // clause — stamping workspaceId on the entity object does not add an
+  // AND workspace_id = ? guard to the SQL. A leaked entity id could
+  // therefore act on a row from a different workspace.
+  // Use softDelete / delete (criteria-based) instead — they always include
+  // workspaceId in the WHERE clause.
+
  insert(
    workspaceId: string,
    entity: QueryDeepPartialEntity<T> | QueryDeepPartialEntity<T>[],