houseassassin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix view rest api in v2 (#15398)

Browse Source 
# Introduction
Initially wanted to reactive the test introduced in
https://github.com/twentyhq/twenty/pull/15393, that was failing because
of direct data source access removing all views ( even seeded one )
which was making the test fail

While doing so discovered a lot of issue with the rest API:
- Rest api wasn't consuming the v2 at all
- Rest api wasn't prepared to handle v2 exceptions
- Rest api did not handled unknown exceptions ( timeout )

Refactored the cleanup of each test to follow black box pattern and
avoid test leakage
This commit is contained in:
Paul Rastoin
2025-10-29 15:07:08 +01:00
committed by GitHub
parent 28c6edfa1f
commit c19a799973
34 changed files with 1281 additions and 577 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/twenty-server/src/engine/metadata-modules/view/controllers/view.controller.ts
+59 -6
View File
@@ -14,6 +14,8 @@ import {
import { type APP_LOCALES } from 'twenty-shared/translations';
import { isDefined } from 'twenty-shared/utils';
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service';
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
@@ -31,6 +33,7 @@ import {
ViewExceptionMessageKey,
} from 'src/engine/metadata-modules/view/exceptions/view.exception';
import { ViewRestApiExceptionFilter } from 'src/engine/metadata-modules/view/filters/view-rest-api-exception.filter';
import { ViewV2Service } from 'src/engine/metadata-modules/view/services/view-v2.service';
import { ViewService } from 'src/engine/metadata-modules/view/services/view.service';
import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service';
@@ -40,6 +43,8 @@ import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/works
export class ViewController {
constructor(
private readonly viewService: ViewService,
private readonly viewV2Service: ViewV2Service,
private readonly featureFlagService: FeatureFlagService,
private readonly workspaceMetadataCacheService: WorkspaceMetadataCacheService,
private readonly i18nService: I18nService,
) {}
@@ -98,10 +103,25 @@ export class ViewController {
@AuthWorkspace() workspace: WorkspaceEntity,
@RequestLocale() locale?: keyof typeof APP_LOCALES,
): Promise<ViewDTO> {
const view = await this.viewService.create({
...input,
workspaceId: workspace.id,
});
const isWorkspaceMigrationV2Enabled =
await this.featureFlagService.isFeatureEnabled(
FeatureFlagKey.IS_WORKSPACE_MIGRATION_V2_ENABLED,
workspace.id,
);
let view: ViewDTO;
if (isWorkspaceMigrationV2Enabled) {
view = await this.viewV2Service.createOne({
createViewInput: input,
workspaceId: workspace.id,
});
} else {
view = await this.viewService.create({
...input,
workspaceId: workspace.id,
});
}
const processedViews = await this.processViewsWithTemplates(
[view],
@@ -119,7 +139,25 @@ export class ViewController {
@RequestLocale() locale: keyof typeof APP_LOCALES | undefined,
@AuthWorkspace() workspace: WorkspaceEntity,
): Promise<ViewDTO> {
const updatedView = await this.viewService.update(id, workspace.id, input);
const isWorkspaceMigrationV2Enabled =
await this.featureFlagService.isFeatureEnabled(
FeatureFlagKey.IS_WORKSPACE_MIGRATION_V2_ENABLED,
workspace.id,
);
let updatedView: ViewDTO;
if (isWorkspaceMigrationV2Enabled) {
updatedView = await this.viewV2Service.updateOne({
updateViewInput: {
...input,
id,
},
workspaceId: workspace.id,
});
} else {
updatedView = await this.viewService.update(id, workspace.id, input);
}
const processedViews = await this.processViewsWithTemplates(
[updatedView],
@@ -135,7 +173,22 @@ export class ViewController {
@Param('id') id: string,
@AuthWorkspace() workspace: WorkspaceEntity,
): Promise<{ success: boolean }> {
const deletedView = await this.viewService.delete(id, workspace.id);
const isWorkspaceMigrationV2Enabled =
await this.featureFlagService.isFeatureEnabled(
FeatureFlagKey.IS_WORKSPACE_MIGRATION_V2_ENABLED,
workspace.id,
);
let deletedView: ViewDTO | null;
if (isWorkspaceMigrationV2Enabled) {
deletedView = await this.viewV2Service.deleteOne({
deleteViewInput: { id },
workspaceId: workspace.id,
});
} else {
deletedView = await this.viewService.delete(id, workspace.id);
}
return { success: isDefined(deletedView) };
}
packages/twenty-server/src/engine/metadata-modules/view/filters/view-rest-api-exception.filter.ts
+66 -22
View File
@@ -2,47 +2,91 @@ import {
type ArgumentsHost,
Catch,
type ExceptionFilter,
Injectable,
} from '@nestjs/common';
import { type Response } from 'express';
import { SOURCE_LOCALE } from 'twenty-shared/translations';
import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service';
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
import {
ViewException,
ViewExceptionCode,
} from 'src/engine/metadata-modules/view/exceptions/view.exception';
import { type CustomException } from 'src/utils/custom-exception';
import { WorkspaceMigrationBuilderExceptionV2 } from 'src/engine/workspace-manager/workspace-migration-v2/exceptions/workspace-migration-builder-exception-v2';
import { fromWorkspaceMigrationBuilderExceptionToMetadataValidationResponseError } from 'src/engine/workspace-manager/workspace-migration-v2/interceptors/utils/from-workspace-migration-builder-exception-to-metadata-validation-response-error.util';
import {
type CustomException,
UnknownException,
} from 'src/utils/custom-exception';
@Catch(ViewException)
@Injectable()
@Catch(ViewException, WorkspaceMigrationBuilderExceptionV2)
export class ViewRestApiExceptionFilter implements ExceptionFilter {
constructor(
private readonly httpExceptionHandlerService: HttpExceptionHandlerService,
private readonly i18nService: I18nService,
) {}
catch(exception: ViewException, host: ArgumentsHost) {
catch(
exception: ViewException | WorkspaceMigrationBuilderExceptionV2,
host: ArgumentsHost,
) {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();
switch (exception.code) {
case ViewExceptionCode.VIEW_NOT_FOUND:
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
404,
);
case ViewExceptionCode.INVALID_VIEW_DATA:
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
400,
);
default:
// TODO: change to 500 when we have input validation
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
400,
if (exception instanceof WorkspaceMigrationBuilderExceptionV2) {
const i18n = this.i18nService.getI18nInstance(SOURCE_LOCALE);
const { errors, summary } =
fromWorkspaceMigrationBuilderExceptionToMetadataValidationResponseError(
exception,
i18n,
);
return response.status(400).json({
statusCode: 400,
error: 'METADATA_VALIDATION_ERROR',
message: exception.message || 'Validation failed',
errors,
summary,
});
}
if (exception instanceof ViewException) {
switch (exception.code) {
case ViewExceptionCode.VIEW_NOT_FOUND:
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
404,
);
case ViewExceptionCode.INVALID_VIEW_DATA:
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
400,
);
default:
// TODO: change to 500 when we have input validation
return this.httpExceptionHandlerService.handleError(
exception as CustomException,
response,
400,
);
}
}
// Fallback for any other exception type
const unknownException = new UnknownException(
'Internal server error',
'INTERNAL_ERROR',
);
return this.httpExceptionHandlerService.handleError(
unknownException as CustomException,
response,
500,
);
}
}