Skip to content

Commit b67f520

Browse files
Zaimwa9Zaimwa9
authored
fix: validate-metadata-required-for-on-all-entities (#6658)
1 parent 064b4bb commit b67f520

10 files changed

Lines changed: 633 additions & 370 deletions

File tree

‎frontend/common/services/useMetadataField.ts‎

Lines changed: 117 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,47 @@ import { Res } from 'common/types/responses'
22
import { Req } from 'common/types/requests'
33
import { service } from 'common/service'
44
import Utils from 'common/utils/utils'
5+
import { CustomMetadataField } from 'common/types/metadata-field'
6+
import {
7+
Environment,
8+
MetadataField,
9+
MetadataModelField,
10+
PagedResponse,
11+
ProjectFlag,
12+
Segment,
13+
} from 'common/types/responses'
14+
import { mergeMetadataFields } from 'common/utils/mergeMetadataFields'
15+
16+
type EntityType = 'feature' | 'segment' | 'environment'
17+
18+
type EntityMetadataParams = {
19+
organisationId: number
20+
projectId: number
21+
entityContentType: number
22+
entityType: EntityType
23+
entityId?: number
24+
}
25+
26+
type EntityData = ProjectFlag | Segment | Environment
27+
28+
function getEntityUrl(params: EntityMetadataParams): string | null {
29+
const { entityId, entityType, projectId } = params
30+
31+
if (!entityId) {
32+
return null
33+
}
34+
35+
switch (entityType) {
36+
case 'feature':
37+
return `projects/${projectId}/features/${entityId}/`
38+
case 'segment':
39+
return `projects/${projectId}/segments/${entityId}/`
40+
case 'environment':
41+
return `environments/${entityId}/`
42+
default:
43+
return null
44+
}
45+
}
546

647
export const metadataService = service
748
.enhanceEndpoints({ addTagTypes: ['Metadata'] })
@@ -11,7 +52,7 @@ export const metadataService = service
1152
Res['metadataField'],
1253
Req['createMetadataField']
1354
>({
14-
invalidatesTags: [{ id: 'LIST', type: 'Metadata' }],
55+
invalidatesTags: [{ type: 'Metadata' }],
1556
query: (query: Req['createMetadataField']) => ({
1657
body: query.body,
1758
method: 'POST',
@@ -22,12 +63,73 @@ export const metadataService = service
2263
Res['metadataField'],
2364
Req['deleteMetadataField']
2465
>({
25-
invalidatesTags: [{ id: 'LIST', type: 'Metadata' }],
66+
invalidatesTags: [{ type: 'Metadata' }],
2667
query: (query: Req['deleteMetadataField']) => ({
2768
method: 'DELETE',
2869
url: `metadata/fields/${query.id}/`,
2970
}),
3071
}),
72+
getEntityMetadataFields: builder.query<
73+
CustomMetadataField[],
74+
EntityMetadataParams
75+
>({
76+
providesTags: (_res, _err, arg) => [
77+
{
78+
id: `${arg.entityType}-${arg.entityId ?? 'new'}-${
79+
arg.entityContentType
80+
}`,
81+
type: 'Metadata',
82+
},
83+
],
84+
queryFn: async (arg, _api, _extraOptions, baseQuery) => {
85+
const entityUrl = getEntityUrl(arg)
86+
87+
// Build queries to run in parallel
88+
const queries: Promise<{ data?: unknown; error?: unknown }>[] = [
89+
baseQuery({
90+
url: `metadata/fields/?${Utils.toParam({
91+
organisation: arg.organisationId,
92+
})}`,
93+
}),
94+
baseQuery({
95+
url: `organisations/${arg.organisationId}/metadata-model-fields/`,
96+
}),
97+
]
98+
99+
// Only fetch entity data if we have an entityId
100+
if (arg.entityId && entityUrl) {
101+
queries.push(baseQuery({ url: entityUrl }))
102+
}
103+
104+
// Fetch all in parallel
105+
const results = await Promise.all(queries)
106+
107+
const [fieldsRes, modelFieldsRes, entityRes] = results
108+
109+
// Handle errors
110+
if (fieldsRes.error) {
111+
return { error: fieldsRes.error as Res['metadataList'] }
112+
}
113+
if (modelFieldsRes.error) {
114+
return {
115+
error: modelFieldsRes.error as Res['metadataModelFieldList'],
116+
}
117+
}
118+
if (entityRes?.error) {
119+
return { error: entityRes.error as EntityData }
120+
}
121+
122+
// Merge and return
123+
const mergedMetadata = mergeMetadataFields(
124+
fieldsRes.data as PagedResponse<MetadataField>,
125+
modelFieldsRes.data as PagedResponse<MetadataModelField>,
126+
entityRes?.data as EntityData | null,
127+
arg.entityContentType,
128+
)
129+
130+
return { data: mergedMetadata }
131+
},
132+
}),
31133
getMetadataField: builder.query<
32134
Res['metadataField'],
33135
Req['getMetadataField']
@@ -50,10 +152,7 @@ export const metadataService = service
50152
Res['metadataField'],
51153
Req['updateMetadataField']
52154
>({
53-
invalidatesTags: (res) => [
54-
{ id: 'LIST', type: 'Metadata' },
55-
{ id: res?.id, type: 'Metadata' },
56-
],
155+
invalidatesTags: [{ type: 'Metadata' }],
57156
query: (query: Req['updateMetadataField']) => ({
58157
body: query.body,
59158
method: 'PUT',
@@ -119,11 +218,23 @@ export async function updateMetadata(
119218
metadataService.endpoints.updateMetadataField.initiate(data, options),
120219
)
121220
}
221+
export async function getEntityMetadataFields(
222+
store: any,
223+
data: EntityMetadataParams,
224+
options?: Parameters<
225+
typeof metadataService.endpoints.getEntityMetadataFields.initiate
226+
>[1],
227+
) {
228+
return store.dispatch(
229+
metadataService.endpoints.getEntityMetadataFields.initiate(data, options),
230+
)
231+
}
122232
// END OF FUNCTION_EXPORTS
123233

124234
export const {
125235
useCreateMetadataFieldMutation,
126236
useDeleteMetadataFieldMutation,
237+
useGetEntityMetadataFieldsQuery,
127238
useGetMetadataFieldListQuery,
128239
useGetMetadataFieldQuery,
129240
useUpdateMetadataFieldMutation,

‎frontend/common/types/metadata-field.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { MetadataField } from './responses'
2+
3+
export type CustomMetadataField = MetadataField & {
4+
metadataModelFieldId: number | string | null
5+
isRequiredFor: boolean
6+
model_field?: string | number
7+
hasValue?: boolean
8+
field_value?: string
9+
}

‎frontend/common/utils/__tests__/mergeMetadataFields.test.ts‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { mergeMetadataFields } from 'common/utils/mergeMetadataFields'
2+
import {
3+
MetadataField,
4+
MetadataModelField,
5+
PagedResponse,
6+
} from 'common/types/responses'
7+
8+
const createFieldList = (
9+
fields: Partial<MetadataField>[],
10+
): PagedResponse<MetadataField> => ({
11+
results: fields.map((f, idx) => ({
12+
description: 'Test description',
13+
id: idx + 1,
14+
name: `Field ${idx + 1}`,
15+
organisation: 1,
16+
type: 'str',
17+
...f,
18+
})) as MetadataField[],
19+
})
20+
21+
const createModelFieldList = (
22+
modelFields: Partial<MetadataModelField>[],
23+
): PagedResponse<MetadataModelField> => ({
24+
results: modelFields.map((mf, idx) => ({
25+
content_type: 100,
26+
field: idx + 1,
27+
id: `${idx + 10}`,
28+
is_required_for: [],
29+
...mf,
30+
})) as MetadataModelField[],
31+
})
32+
33+
describe('mergeMetadataFields', () => {
34+
it('merges field definitions with existing values', () => {
35+
const fieldList = createFieldList([{ id: 1, name: 'Field 1' }])
36+
const modelFieldList = createModelFieldList([
37+
{ content_type: 100, field: 1, id: '10', is_required_for: [] },
38+
])
39+
const entityData = {
40+
metadata: [{ field_value: 'existing value', model_field: '10' }],
41+
}
42+
43+
const result = mergeMetadataFields(
44+
fieldList,
45+
modelFieldList,
46+
entityData,
47+
100,
48+
)
49+
50+
expect(result).toHaveLength(1)
51+
expect(result[0].field_value).toBe('existing value')
52+
expect(result[0].hasValue).toBe(true)
53+
})
54+
55+
it('sets empty field_value when no existing value or null entity', () => {
56+
const fieldList = createFieldList([{ id: 1, name: 'Field 1' }])
57+
const modelFieldList = createModelFieldList([
58+
{ content_type: 100, field: 1, id: '10', is_required_for: [] },
59+
])
60+
61+
const result = mergeMetadataFields(fieldList, modelFieldList, null, 100)
62+
63+
expect(result[0].field_value).toBe('')
64+
expect(result[0].hasValue).toBe(false)
65+
})
66+
67+
it('marks field as required when is_required_for has entries', () => {
68+
const fieldList = createFieldList([{ id: 1, name: 'Required Field' }])
69+
const modelFieldList = createModelFieldList([
70+
{
71+
content_type: 100,
72+
field: 1,
73+
id: '10',
74+
is_required_for: [{ content_type: 50 }],
75+
},
76+
])
77+
78+
const result = mergeMetadataFields(fieldList, modelFieldList, null, 100)
79+
80+
expect(result[0].isRequiredFor).toBe(true)
81+
})
82+
83+
it('sorts required fields first', () => {
84+
const fieldList = createFieldList([
85+
{ id: 1, name: 'Optional' },
86+
{ id: 2, name: 'Required' },
87+
])
88+
const modelFieldList = createModelFieldList([
89+
{ content_type: 100, field: 1, id: '10', is_required_for: [] },
90+
{
91+
content_type: 100,
92+
field: 2,
93+
id: '11',
94+
is_required_for: [{ content_type: 50 }],
95+
},
96+
])
97+
98+
const result = mergeMetadataFields(fieldList, modelFieldList, null, 100)
99+
100+
expect(result[0].name).toBe('Required')
101+
expect(result[1].name).toBe('Optional')
102+
})
103+
})

‎frontend/common/utils/__tests__/metadataValidation.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { getGlobalMetadataValidationState } from 'common/utils/metadataValidation'
2+
import { CustomMetadataField } from 'common/types/metadata-field'
3+
4+
const createField = (
5+
partialField: Partial<CustomMetadataField> = {},
6+
): CustomMetadataField => ({
7+
description: 'A test field',
8+
field_value: '',
9+
hasValue: false,
10+
id: 1,
11+
isRequiredFor: false,
12+
metadataModelFieldId: 1,
13+
name: 'Test Field',
14+
organisation: 1,
15+
type: 'str',
16+
...partialField,
17+
})
18+
19+
describe('getGlobalMetadataValidationState', () => {
20+
it('returns all zeros for empty fields array', () => {
21+
const result = getGlobalMetadataValidationState([])
22+
23+
expect(result).toEqual({
24+
hasUnfilledRequired: false,
25+
totalFilledRequired: 0,
26+
totalRequired: 0,
27+
})
28+
})
29+
30+
it('returns hasUnfilledRequired false when no required fields', () => {
31+
const fields = [
32+
createField({ id: 1, isRequiredFor: false }),
33+
createField({ id: 2, isRequiredFor: false }),
34+
]
35+
36+
const result = getGlobalMetadataValidationState(fields)
37+
38+
expect(result).toEqual({
39+
hasUnfilledRequired: false,
40+
totalFilledRequired: 0,
41+
totalRequired: 0,
42+
})
43+
})
44+
45+
it('returns hasUnfilledRequired false when required field is filled', () => {
46+
const fields = [
47+
createField({ field_value: 'some value', id: 1, isRequiredFor: true }),
48+
]
49+
50+
const result = getGlobalMetadataValidationState(fields)
51+
52+
expect(result).toEqual({
53+
hasUnfilledRequired: false,
54+
totalFilledRequired: 1,
55+
totalRequired: 1,
56+
})
57+
})
58+
59+
it('returns hasUnfilledRequired true when some required fields are unfilled', () => {
60+
const fields = [
61+
createField({ field_value: 'filled', id: 1, isRequiredFor: true }),
62+
createField({ field_value: '', id: 2, isRequiredFor: true }),
63+
createField({ field_value: '', id: 3, isRequiredFor: false }),
64+
]
65+
66+
const result = getGlobalMetadataValidationState(fields)
67+
68+
expect(result).toEqual({
69+
hasUnfilledRequired: true,
70+
totalFilledRequired: 1,
71+
totalRequired: 2,
72+
})
73+
})
74+
75+
it('returns hasUnfilledRequired false when all required fields are filled', () => {
76+
const fields = [
77+
createField({ field_value: 'filled', id: 1, isRequiredFor: true }),
78+
createField({ field_value: 'also filled', id: 2, isRequiredFor: true }),
79+
createField({ field_value: '', id: 3, isRequiredFor: false }),
80+
]
81+
82+
const result = getGlobalMetadataValidationState(fields)
83+
84+
expect(result).toEqual({
85+
hasUnfilledRequired: false,
86+
totalFilledRequired: 2,
87+
totalRequired: 2,
88+
})
89+
})
90+
})

0 commit comments

Comments
 (0)