Skip to content

Commit e056337

Browse files
abdulrahmancodescursoragentcharlesBochet
authored
Fix unclear metadata validation errors (#20234)
https://github.com/user-attachments/assets/8f8f1122-3de1-4a9b-8bb4-a3c8d31e47ae --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Charles Bochet <charles@twenty.com>
1 parent a03c264 commit e056337

70 files changed

Lines changed: 935 additions & 771 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
graphQLErrorCodesToFilter,
3333
shouldCaptureException,
3434
} from 'src/engine/utils/global-exception-handler.util';
35+
import { translateUserFriendlyMessageDescriptors } from 'src/engine/core-modules/i18n/utils/translate-user-friendly-message-descriptors.util';
3536

3637
const DEFAULT_EVENT_ID_KEY = 'exceptionEventId';
3738
const SCHEMA_VERSION_HEADER = 'x-schema-version';
@@ -223,13 +224,15 @@ export const useGraphQLErrorHandlerHook = <
223224
error instanceof BaseGraphQLError
224225
? {
225226
...error,
226-
extensions: {
227-
...error.extensions,
228-
userFriendlyMessage: i18n._(
229-
error.extensions.userFriendlyMessage ??
227+
extensions: translateUserFriendlyMessageDescriptors(
228+
{
229+
...error.extensions,
230+
userFriendlyMessage:
231+
error.extensions.userFriendlyMessage ??
230232
defaultErrorMessage,
231-
),
232-
},
233+
},
234+
i18n,
235+
),
233236
}
234237
: generateGraphQLErrorFromError(error, i18n);
235238

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { type I18n, type MessageDescriptor } from '@lingui/core';
2+
3+
import { translateUserFriendlyMessageDescriptors } from 'src/engine/core-modules/i18n/utils/translate-user-friendly-message-descriptors.util';
4+
5+
const buildDescriptor = (id: string): MessageDescriptor => ({
6+
id,
7+
message: id,
8+
});
9+
10+
const i18nMock = {
11+
_: (descriptor: MessageDescriptor) => `tr:${descriptor.id}`,
12+
} as unknown as I18n;
13+
14+
describe('translateUserFriendlyMessageDescriptors', () => {
15+
it('translates a top-level userFriendlyMessage descriptor to a string', () => {
16+
const result = translateUserFriendlyMessageDescriptors(
17+
{ userFriendlyMessage: buildDescriptor('top-level') },
18+
i18nMock,
19+
);
20+
21+
expect(result).toEqual({ userFriendlyMessage: 'tr:top-level' });
22+
});
23+
24+
it('leaves a userFriendlyMessage value untouched when it is already a string', () => {
25+
const result = translateUserFriendlyMessageDescriptors(
26+
{ userFriendlyMessage: 'already a string' },
27+
i18nMock,
28+
);
29+
30+
expect(result).toEqual({ userFriendlyMessage: 'already a string' });
31+
});
32+
33+
it('does not translate descriptors stored under other keys', () => {
34+
const descriptor = buildDescriptor('not-user-friendly');
35+
36+
const result = translateUserFriendlyMessageDescriptors(
37+
{ someOtherKey: descriptor },
38+
i18nMock,
39+
);
40+
41+
expect(result).toEqual({
42+
someOtherKey: { id: 'not-user-friendly', message: 'not-user-friendly' },
43+
});
44+
});
45+
46+
it('translates userFriendlyMessage descriptors nested inside arrays of objects', () => {
47+
const result = translateUserFriendlyMessageDescriptors(
48+
{
49+
errors: [
50+
{ code: 'A', userFriendlyMessage: buildDescriptor('first') },
51+
{ code: 'B', userFriendlyMessage: buildDescriptor('second') },
52+
],
53+
},
54+
i18nMock,
55+
);
56+
57+
expect(result).toEqual({
58+
errors: [
59+
{ code: 'A', userFriendlyMessage: 'tr:first' },
60+
{ code: 'B', userFriendlyMessage: 'tr:second' },
61+
],
62+
});
63+
});
64+
65+
it('preserves primitives, null and undefined values found along the way', () => {
66+
const result = translateUserFriendlyMessageDescriptors(
67+
{
68+
statusCode: 400,
69+
message: 'Validation failed',
70+
eventId: null,
71+
traceId: undefined,
72+
nested: { count: 0, flag: false },
73+
},
74+
i18nMock,
75+
);
76+
77+
expect(result).toEqual({
78+
statusCode: 400,
79+
message: 'Validation failed',
80+
eventId: null,
81+
traceId: undefined,
82+
nested: { count: 0, flag: false },
83+
});
84+
});
85+
86+
it('handles a realistic metadata-validation payload with both top-level and per-error descriptors', () => {
87+
const result = translateUserFriendlyMessageDescriptors(
88+
{
89+
userFriendlyMessage: buildDescriptor('Field value is required'),
90+
summary: { totalErrors: 1, viewGroup: 1 },
91+
errors: {
92+
viewGroup: [
93+
{
94+
status: 'fail',
95+
type: 'create',
96+
metadataName: 'viewGroup',
97+
errors: [
98+
{
99+
code: 'INVALID_VIEW_DATA',
100+
message: 'Field value is required',
101+
userFriendlyMessage: buildDescriptor(
102+
'Field value is required',
103+
),
104+
},
105+
],
106+
},
107+
],
108+
},
109+
},
110+
i18nMock,
111+
);
112+
113+
expect(result).toEqual({
114+
userFriendlyMessage: 'tr:Field value is required',
115+
summary: { totalErrors: 1, viewGroup: 1 },
116+
errors: {
117+
viewGroup: [
118+
{
119+
status: 'fail',
120+
type: 'create',
121+
metadataName: 'viewGroup',
122+
errors: [
123+
{
124+
code: 'INVALID_VIEW_DATA',
125+
message: 'Field value is required',
126+
userFriendlyMessage: 'tr:Field value is required',
127+
},
128+
],
129+
},
130+
],
131+
},
132+
});
133+
});
134+
135+
it('does not mutate the input payload', () => {
136+
const descriptor = buildDescriptor('mutation-check');
137+
const input = { userFriendlyMessage: descriptor, nested: { value: 1 } };
138+
139+
const result = translateUserFriendlyMessageDescriptors(input, i18nMock);
140+
141+
expect(input.userFriendlyMessage).toBe(descriptor);
142+
expect(result).not.toBe(input);
143+
expect(result).toEqual({
144+
userFriendlyMessage: 'tr:mutation-check',
145+
nested: { value: 1 },
146+
});
147+
});
148+
});
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { type I18n, type MessageDescriptor } from '@lingui/core';
2+
import { isArray, isObject, isString } from '@sniptt/guards';
3+
4+
const USER_FRIENDLY_MESSAGE_KEY = 'userFriendlyMessage';
5+
6+
const isMessageDescriptor = (value: unknown): value is MessageDescriptor =>
7+
isObject(value) && 'id' in value && isString(value.id);
8+
9+
const translateValueRecursively = (
10+
value: unknown,
11+
i18n: I18n,
12+
parentKey?: string,
13+
): unknown => {
14+
if (parentKey === USER_FRIENDLY_MESSAGE_KEY && isMessageDescriptor(value)) {
15+
return i18n._(value);
16+
}
17+
18+
if (isArray(value)) {
19+
return value.map((item) =>
20+
translateValueRecursively(item, i18n, parentKey),
21+
);
22+
}
23+
24+
if (isObject(value)) {
25+
return Object.fromEntries(
26+
Object.entries(value).map(([key, nestedValue]) => [
27+
key,
28+
translateValueRecursively(nestedValue, i18n, key),
29+
]),
30+
);
31+
}
32+
33+
return value;
34+
};
35+
36+
export const translateUserFriendlyMessageDescriptors = (
37+
payload: object,
38+
i18n: I18n,
39+
): Record<string, unknown> =>
40+
translateValueRecursively(payload, i18n) as Record<string, unknown>;

packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ export class FieldMetadataResolver {
9595
async createOneField(
9696
@Args('input') input: CreateOneFieldMetadataInput,
9797
@AuthWorkspace() { id: workspaceId }: WorkspaceEntity,
98-
@Context() context: I18nContext,
9998
) {
10099
try {
101100
const flatFieldMetadata = await this.fieldMetadataService.createOneField({
@@ -105,10 +104,7 @@ export class FieldMetadataResolver {
105104

106105
return fromFlatFieldMetadataToFieldMetadataDto(flatFieldMetadata);
107106
} catch (error) {
108-
return fieldMetadataGraphqlApiExceptionHandler(
109-
error,
110-
this.i18nService.getI18nInstance(context.req.locale),
111-
);
107+
return fieldMetadataGraphqlApiExceptionHandler(error);
112108
}
113109
}
114110

@@ -117,7 +113,6 @@ export class FieldMetadataResolver {
117113
async updateOneField(
118114
@Args('input') input: UpdateOneFieldMetadataInput,
119115
@AuthWorkspace() { id: workspaceId }: WorkspaceEntity,
120-
@Context() context: I18nContext,
121116
) {
122117
try {
123118
const flatFieldMetadata = await this.fieldMetadataService.updateOneField({
@@ -127,10 +122,7 @@ export class FieldMetadataResolver {
127122

128123
return fromFlatFieldMetadataToFieldMetadataDto(flatFieldMetadata);
129124
} catch (error) {
130-
fieldMetadataGraphqlApiExceptionHandler(
131-
error,
132-
this.i18nService.getI18nInstance(context.req.locale),
133-
);
125+
fieldMetadataGraphqlApiExceptionHandler(error);
134126
}
135127
}
136128

@@ -139,7 +131,6 @@ export class FieldMetadataResolver {
139131
async deleteOneField(
140132
@Args('input') deleteOneFieldInput: DeleteOneFieldInput,
141133
@AuthWorkspace() { id: workspaceId }: WorkspaceEntity,
142-
@Context() context: I18nContext,
143134
) {
144135
if (!isDefined(workspaceId)) {
145136
throw new ForbiddenError('Could not retrieve workspace ID');
@@ -153,10 +144,7 @@ export class FieldMetadataResolver {
153144

154145
return fromFlatFieldMetadataToFieldMetadataDto(flatFieldMetadata);
155146
} catch (error) {
156-
fieldMetadataGraphqlApiExceptionHandler(
157-
error,
158-
this.i18nService.getI18nInstance(context.req.locale),
159-
);
147+
fieldMetadataGraphqlApiExceptionHandler(error);
160148
}
161149
}
162150

@@ -168,7 +156,7 @@ export class FieldMetadataResolver {
168156
id: fieldMetadataId,
169157
objectMetadataId,
170158
}: Pick<FieldMetadataDTO, 'id' | 'objectMetadataId'>,
171-
@Context() context: { loaders: IDataloaders } & I18nContext,
159+
@Context() context: { loaders: IDataloaders },
172160
): Promise<RelationDTO | null> {
173161
try {
174162
return await context.loaders.relationLoader.load({
@@ -177,10 +165,7 @@ export class FieldMetadataResolver {
177165
workspaceId: workspace.id,
178166
});
179167
} catch (error) {
180-
return fieldMetadataGraphqlApiExceptionHandler(
181-
error,
182-
this.i18nService.getI18nInstance(context.req.locale),
183-
);
168+
return fieldMetadataGraphqlApiExceptionHandler(error);
184169
}
185170
}
186171

@@ -192,7 +177,7 @@ export class FieldMetadataResolver {
192177
id: fieldMetadataId,
193178
objectMetadataId,
194179
}: Pick<FieldMetadataDTO, 'id' | 'objectMetadataId'>,
195-
@Context() context: { loaders: IDataloaders } & I18nContext,
180+
@Context() context: { loaders: IDataloaders },
196181
): Promise<RelationDTO[] | null> {
197182
try {
198183
return await context.loaders.morphRelationLoader.load({
@@ -201,10 +186,7 @@ export class FieldMetadataResolver {
201186
workspaceId: workspace.id,
202187
});
203188
} catch (error) {
204-
return fieldMetadataGraphqlApiExceptionHandler(
205-
error,
206-
this.i18nService.getI18nInstance(context.req.locale),
207-
);
189+
return fieldMetadataGraphqlApiExceptionHandler(error);
208190
}
209191
}
210192
}

packages/twenty-server/src/engine/metadata-modules/field-metadata/interceptors/field-metadata-graphql-api-exception.interceptor.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,19 @@ import {
44
Injectable,
55
type NestInterceptor,
66
} from '@nestjs/common';
7-
import { GqlExecutionContext } from '@nestjs/graphql';
87

98
import { type Observable, catchError } from 'rxjs';
10-
import { SOURCE_LOCALE } from 'twenty-shared/translations';
119

12-
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
1310
import { fieldMetadataGraphqlApiExceptionHandler } from 'src/engine/metadata-modules/field-metadata/utils/field-metadata-graphql-api-exception-handler.util';
1411

1512
@Injectable()
1613
export class FieldMetadataGraphqlApiExceptionInterceptor
1714
implements NestInterceptor
1815
{
19-
constructor(private readonly i18nService: I18nService) {}
20-
2116
// oxlint-disable-next-line @typescripttypescript/no-explicit-any
22-
intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
23-
const gqlContext = GqlExecutionContext.create(context);
24-
const ctx = gqlContext.getContext();
25-
const locale = ctx.req?.locale ?? SOURCE_LOCALE;
26-
const i18n = this.i18nService.getI18nInstance(locale);
27-
17+
intercept(_context: ExecutionContext, next: CallHandler): Observable<any> {
2818
return next
2919
.handle()
30-
.pipe(
31-
catchError((err) => fieldMetadataGraphqlApiExceptionHandler(err, i18n)),
32-
);
20+
.pipe(catchError((err) => fieldMetadataGraphqlApiExceptionHandler(err)));
3321
}
3422
}

packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/field-metadata-graphql-api-exception-handler.util.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { type I18n } from '@lingui/core';
21
import { assertUnreachable } from 'twenty-shared/utils';
32

43
import {
@@ -13,14 +12,11 @@ import {
1312
} from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
1413
import { InvalidMetadataException } from 'src/engine/metadata-modules/utils/exceptions/invalid-metadata.exception';
1514
import { WorkspaceMigrationBuilderException } from 'src/engine/workspace-manager/workspace-migration/exceptions/workspace-migration-builder-exception';
16-
import { workspaceMigrationBuilderExceptionFormatter } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-builder-exception-formatter';
15+
import { workspaceMigrationBuilderGraphqlApiExceptionHandler } from 'src/engine/workspace-manager/workspace-migration/interceptors/utils/workspace-migration-builder-graphql-api-exception-handler.util';
1716

18-
export const fieldMetadataGraphqlApiExceptionHandler = (
19-
error: Error,
20-
i18n: I18n,
21-
) => {
17+
export const fieldMetadataGraphqlApiExceptionHandler = (error: Error) => {
2218
if (error instanceof WorkspaceMigrationBuilderException) {
23-
workspaceMigrationBuilderExceptionFormatter(error, i18n);
19+
workspaceMigrationBuilderGraphqlApiExceptionHandler(error);
2420
}
2521

2622
if (error instanceof InvalidMetadataException) {

0 commit comments

Comments
 (0)