[1/n]: Migrate deleteOne Rest API to use TwentyORM directly (#9784)

# This PR

- Addressing #3644 
- Migrates the `DELETE /rest/*` endpoint to use TwentyORM
- Factorizes common middleware logic into a common module

---------

Co-authored-by: martmull <martmull@hotmail.fr>

packages/twenty-e2e-testing/tests/authentication/fixture.ts

@@ -1,10 +1,10 @@
 import { test as base } from '../../lib/fixtures/screenshot';
-import { LoginPage } from '../../lib/pom/loginPage';
+import { ConfirmationModal } from '../../lib/pom/helper/confirmationModal';
 import { LeftMenu } from '../../lib/pom/leftMenu';
-import { SettingsPage } from '../../lib/pom/settingsPage';
+import { LoginPage } from '../../lib/pom/loginPage';
 import { MembersSection } from '../../lib/pom/settings/membersSection';
 import { ProfileSection } from '../../lib/pom/settings/profileSection';
-import { ConfirmationModal } from '../../lib/pom/helper/confirmationModal';
+import { SettingsPage } from '../../lib/pom/settingsPage';
 
 type Fixtures = {
   confirmationModal: ConfirmationModal;

packages/twenty-server/@types/jest.d.ts

@@ -5,13 +5,15 @@ declare module '@jest/types' {
     interface ConfigGlobals {
       APP_PORT: number;
       ACCESS_TOKEN: string;
+      EXPIRED_ACCESS_TOKEN: string;
     }
   }
 }
 
 declare global {
   const APP_PORT: number;
   const ACCESS_TOKEN: string;
+  const EXPIRED_ACCESS_TOKEN: string;
 }
 
 export {};

packages/twenty-server/jest-integration.config.ts

@@ -36,6 +36,8 @@ const jestConfig: JestConfigWithTsJest = {
     APP_PORT: 4000,
     ACCESS_TOKEN:
       'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC05ZTNiLTQ2ZDQtYTU1Ni04OGI5ZGRjMmIwMzQiLCJ3b3Jrc3BhY2VJZCI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsIndvcmtzcGFjZU1lbWJlcklkIjoiMjAyMDIwMjAtMDY4Ny00YzQxLWI3MDctZWQxYmZjYTk3MmE3IiwiaWF0IjoxNzI2NDkyNTAyLCJleHAiOjEzMjQ1MDE2NTAyfQ._ISjY_dlVWskeQ6wkE0-kOn641G_mee5GiqoZTQFIfE',
+    EXPIRED_ACCESS_TOKEN:
+      'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC05ZTNiLTQ2ZDQtYTU1Ni04OGI5ZGRjMmIwMzQiLCJ3b3Jrc3BhY2VJZCI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsIndvcmtzcGFjZU1lbWJlcklkIjoiMjAyMDIwMjAtMDY4Ny00YzQxLWI3MDctZWQxYmZjYTk3MmE3IiwiaWF0IjoxNzM4MzIzODc5LCJleHAiOjE3MzgzMjU2Nzl9.m73hHVpnw5uGNGrSuKxn6XtKEUK3Wqkp4HsQdYfZiHo',
   },
 };
 

packages/twenty-server/src/app.module.ts

@@ -21,14 +21,21 @@ import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphq
 import { RestApiModule } from 'src/engine/api/rest/rest-api.module';
 import { MessageQueueDriverType } from 'src/engine/core-modules/message-queue/interfaces';
 import { MessageQueueModule } from 'src/engine/core-modules/message-queue/message-queue.module';
+import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module';
+import { WorkspaceMetadataCacheModule } from 'src/engine/metadata-modules/workspace-metadata-cache/workspace-metadata-cache.module';
 import { GraphQLHydrateRequestFromTokenMiddleware } from 'src/engine/middlewares/graphql-hydrate-request-from-token.middleware';
+import { MiddlewareModule } from 'src/engine/middlewares/middleware.module';
+import { RestCoreMiddleware } from 'src/engine/middlewares/rest-core.middleware';
 import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module';
 import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module';
 import { ModulesModule } from 'src/modules/modules.module';
 
 import { CoreEngineModule } from './engine/core-modules/core-engine.module';
 import { I18nModule } from './engine/core-modules/i18n/i18n.module';
 
+// TODO: Remove this middleware when all the rest endpoints are migrated to TwentyORM
+const MIGRATED_REST_METHODS = [RequestMethod.DELETE];
+
 @Module({
   imports: [
     SentryModule.forRoot(),
@@ -52,6 +59,9 @@ import { I18nModule } from './engine/core-modules/i18n/i18n.module';
     CoreGraphQLApiModule,
     MetadataGraphQLApiModule,
     RestApiModule,
+    DataSourceModule,
+    MiddlewareModule,
+    WorkspaceMetadataCacheModule,
     // I18n module for translations
     I18nModule,
     // Conditional modules
@@ -89,5 +99,9 @@ export class AppModule {
     consumer
       .apply(GraphQLHydrateRequestFromTokenMiddleware)
       .forRoutes({ path: 'metadata', method: RequestMethod.ALL });
+
+    for (const method of MIGRATED_REST_METHODS) {
+      consumer.apply(RestCoreMiddleware).forRoutes({ path: 'rest/*', method });
+    }
   }
 }

packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts

@@ -7,20 +7,26 @@ import {
   Put,
   Req,
   Res,
+  UseFilters,
   UseGuards,
 } from '@nestjs/common';
 
 import { Request, Response } from 'express';
 
+import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service';
 import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service';
 import { cleanGraphQLResponse } from 'src/engine/api/rest/utils/clean-graphql-response.utils';
 import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard';
 import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
+import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter';
 
 @Controller('rest/*')
 @UseGuards(JwtAuthGuard, WorkspaceAuthGuard)
 export class RestApiCoreController {
-  constructor(private readonly restApiCoreService: RestApiCoreService) {}
+  constructor(
+    private readonly restApiCoreService: RestApiCoreService,
+    private readonly restApiCoreServiceV2: RestApiCoreServiceV2,
+  ) {}
 
   @Post('/duplicates')
   async handleApiFindDuplicates(@Req() request: Request, @Res() res: Response) {
@@ -37,10 +43,13 @@ export class RestApiCoreController {
   }
 
   @Delete()
+  // We should move this exception filter to RestApiCoreController class level
+  // when all endpoints are migrated to v2
+  @UseFilters(RestApiExceptionFilter)
   async handleApiDelete(@Req() request: Request, @Res() res: Response) {
-    const result = await this.restApiCoreService.delete(request);
+    const result = await this.restApiCoreServiceV2.delete(request);
 
-    res.status(200).send(cleanGraphQLResponse(result.data.data));
+    res.status(200).send(result);
   }
 
   @Post()

packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts

@@ -0,0 +1,73 @@
+import { BadRequestException, Injectable } from '@nestjs/common';
+
+import { Request } from 'express';
+import { capitalize } from 'twenty-shared';
+
+import { CoreQueryBuilderFactory } from 'src/engine/api/rest/core/query-builder/core-query-builder.factory';
+import { parseCorePath } from 'src/engine/api/rest/core/query-builder/utils/path-parsers/parse-core-path.utils';
+import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
+
+@Injectable()
+export class RestApiCoreServiceV2 {
+  constructor(
+    private readonly coreQueryBuilderFactory: CoreQueryBuilderFactory,
+    private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
+  ) {}
+
+  async delete(request: Request) {
+    const { workspace } = request;
+    const { object: parsedObject, id: recordId } = parseCorePath(request);
+
+    const objectMetadata = await this.coreQueryBuilderFactory.getObjectMetadata(
+      request,
+      parsedObject,
+    );
+
+    if (!objectMetadata) {
+      throw new BadRequestException('Object metadata not found');
+    }
+
+    if (!recordId) {
+      throw new BadRequestException('Record ID not found');
+    }
+
+    const objectMetadataNameSingular =
+      objectMetadata.objectMetadataItem.nameSingular;
+
+    if (!workspace?.id) {
+      throw new BadRequestException('Workspace not found');
+    }
+
+    const repository =
+      await this.twentyORMGlobalManager.getRepositoryForWorkspace(
+        workspace.id,
+        objectMetadataNameSingular,
+      );
+
+    const recordToDelete = await repository.findOneOrFail({
+      where: {
+        id: recordId,
+      },
+    });
+
+    await repository.delete(recordId);
+
+    return this.formatResult('delete', objectMetadataNameSingular, {
+      id: recordToDelete.id,
+    });
+  }
+
+  private formatResult<T>(
+    operation: 'delete' | 'create' | 'update' | 'find',
+    objectNameSingular: string,
+    data: T,
+  ) {
+    const result = {
+      data: {
+        [operation + capitalize(objectNameSingular)]: data,
+      },
+    };
+
+    return result;
+  }
+}

packages/twenty-server/src/engine/api/rest/rest-api-exception.filter.ts

@@ -0,0 +1,24 @@
+import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common';
+
+import { Response } from 'express';
+
+import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service';
+import { CustomException } from 'src/utils/custom-exception';
+
+@Catch()
+export class RestApiExceptionFilter implements ExceptionFilter {
+  constructor(
+    private readonly httpExceptionHandlerService: HttpExceptionHandlerService,
+  ) {}
+
+  catch(exception: unknown, host: ArgumentsHost) {
+    const ctx = host.switchToHttp();
+    const response = ctx.getResponse<Response>();
+
+    return this.httpExceptionHandlerService.handleError(
+      exception as CustomException,
+      response,
+      400,
+    );
+  }
+}

packages/twenty-server/src/engine/api/rest/rest-api.module.ts

@@ -4,6 +4,7 @@ import { Module } from '@nestjs/common';
 import { RestApiCoreBatchController } from 'src/engine/api/rest/core/controllers/rest-api-core-batch.controller';
 import { RestApiCoreController } from 'src/engine/api/rest/core/controllers/rest-api-core.controller';
 import { CoreQueryBuilderModule } from 'src/engine/api/rest/core/query-builder/core-query-builder.module';
+import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service';
 import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service';
 import { EndingBeforeInputFactory } from 'src/engine/api/rest/input-factories/ending-before-input.factory';
 import { LimitInputFactory } from 'src/engine/api/rest/input-factories/limit-input.factory';
@@ -13,6 +14,7 @@ import { RestApiMetadataController } from 'src/engine/api/rest/metadata/rest-api
 import { RestApiMetadataService } from 'src/engine/api/rest/metadata/rest-api-metadata.service';
 import { RestApiService } from 'src/engine/api/rest/rest-api.service';
 import { AuthModule } from 'src/engine/core-modules/auth/auth.module';
+import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module';
 import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module';
 
 @Module({
@@ -22,6 +24,7 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/
     WorkspaceCacheStorageModule,
     AuthModule,
     HttpModule,
+    TwentyORMModule,
   ],
   controllers: [
     RestApiMetadataController,
@@ -31,6 +34,7 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/
   providers: [
     RestApiMetadataService,
     RestApiCoreService,
+    RestApiCoreServiceV2,
     RestApiService,
     StartingAfterInputFactory,
     EndingBeforeInputFactory,

packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts

@@ -14,8 +14,10 @@ export const handleException = (
   exceptionHandlerService: ExceptionHandlerService,
   user?: ExceptionHandlerUser,
   workspace?: ExceptionHandlerWorkspace,
-): void => {
+): CustomException => {
   exceptionHandlerService.captureExceptions([exception], { user, workspace });
+
+  return exception;
 };
 
 interface RequestAndParams {
@@ -45,7 +47,12 @@ export class HttpExceptionHandlerService {
     if (params?.userId) user = { ...user, id: params.userId };
 
     handleException(exception, this.exceptionHandlerService, user, workspace);
+    const statusCode = errorCode || 500;
 
-    return response.status(errorCode || 500).send(exception.message);
+    return response.status(statusCode).send({
+      statusCode,
+      error: exception.code || 'Bad Request',
+      messages: [exception?.message],
+    });
   };
 }

packages/twenty-server/src/engine/middlewares/constants/default-error-message.constant.ts

@@ -0,0 +1 @@
+export const INTERNAL_SERVER_ERROR = 'Internal server error';

packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts

@@ -0,0 +1,19 @@
+export const EXCLUDED_MIDDLEWARE_OPERATIONS = [
+  'GetClientConfig',
+  'GetWorkspaceFromInviteHash',
+  'Track',
+  'CheckUserExists',
+  'GetLoginTokenFromCredentials',
+  'GetAuthTokensFromLoginToken',
+  'GetLoginTokenFromEmailVerificationToken',
+  'ResendEmailVerificationToken',
+  'SignUp',
+  'RenewToken',
+  'EmailPasswordResetLink',
+  'ValidatePasswordResetToken',
+  'UpdatePasswordViaResetToken',
+  'IntrospectionQuery',
+  'ExchangeAuthorizationCode',
+  'GetAuthorizationUrl',
+  'GetPublicWorkspaceDataBySubdomain',
+] as const;