From 5ac05e2cbc12b9469d836bdb152eb2c2bf1f3788 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 4 Feb 2026 18:21:16 +0100 Subject: [PATCH 01/16] feat: calculate-has-metadata-required-based-on-all-entities --- .../metadata/AddMetadataToEntity.tsx | 22 ++++++++++++++-- .../components/modals/create-feature/index.js | 25 ++++++++++++------- .../create-feature/tabs/FeatureSettings.tsx | 2 +- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index f460b07af5d4..a1f484906217 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -104,6 +104,24 @@ const AddMetadataToEntity: FC = ({ const [metadataChanged, setMetadataChanged] = useState(false) + // Compute hasMetadataRequired reactively when state changes + useEffect(() => { + if (!metadataFieldsAssociatedtoEntity) return + + const totalRequired = metadataFieldsAssociatedtoEntity.filter( + (m) => m.isRequiredFor, + ).length + const totalFilledRequired = metadataFieldsAssociatedtoEntity.filter( + (m) => m.field_value && m.field_value !== '' && m.isRequiredFor, + ).length + + // hasMetadataRequired = true means "there are unfilled required fields" + setHasMetadataRequired?.( + totalRequired > 0 && totalFilledRequired < totalRequired, + ) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [metadataFieldsAssociatedtoEntity]) + const mergeMetadataEntityWithMetadataField = ( metadata: Metadata[], // Metadata array metadataField: CustomMetadataField[], // Custom metadata field array @@ -158,11 +176,11 @@ const AddMetadataToEntity: FC = ({ }) // Determine if isRequiredFor should be true or false based on is_required_for array const isRequiredFor = !!matchingItem?.is_required_for.length - setHasMetadataRequired?.(isRequiredFor) + // Return the metadata field with additional metadata model field information including isRequiredFor return { ...meta, - isRequiredFor: isRequiredFor || false, + isRequiredFor, metadataModelFieldId: matchingItem ? matchingItem.id : null, } }) diff --git a/frontend/web/components/modals/create-feature/index.js b/frontend/web/components/modals/create-feature/index.js index ea838aef32c5..53bce83bd7e6 100644 --- a/frontend/web/components/modals/create-feature/index.js +++ b/frontend/web/components/modals/create-feature/index.js @@ -829,9 +829,6 @@ const Index = class extends Component { > {({ permission: projectAdmin }) => { this.state.skipSaveProjectFeature = !createFeature - const _hasMetadataRequired = - this.state.hasMetadataRequired && - !projectFlag.metadata?.length return (
@@ -1706,11 +1703,15 @@ const Index = class extends Component { }} onHasMetadataRequiredChange={( hasMetadataRequired, - ) => + ) => { + console.log( + 'hasMetadataRequired', + hasMetadataRequired, + ) this.setState({ hasMetadataRequired, }) - } + }} /> {isSaving @@ -1820,11 +1821,15 @@ const Index = class extends Component { } onHasMetadataRequiredChange={( hasMetadataRequired, - ) => + ) => { + console.log( + 'hasMetadataRequired', + hasMetadataRequired, + ) this.setState({ hasMetadataRequired, }) - } + }} featureError={ this.parseError(error).featureError } @@ -1842,7 +1847,9 @@ const Index = class extends Component { featureLimitPercentage={ this.state.featureLimitAlert.percentage } - hasMetadataRequired={_hasMetadataRequired} + hasMetadataRequired={ + this.state.hasMetadataRequired + } />
)} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx index 456f0e068fd4..28d4b1ffa273 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx @@ -75,7 +75,7 @@ const FeatureSettings: FC = ({ )} {metadataEnable && featureContentType?.id && !identity && ( <> - + {/* */} Date: Wed, 4 Feb 2026 18:21:16 +0100 Subject: [PATCH 02/16] fix: calculate-has-metadata-required-based-on-all-entities --- .../metadata/AddMetadataToEntity.tsx | 20 +++++++++++++++++-- .../components/modals/create-feature/index.js | 17 +++++++++------- .../create-feature/tabs/FeatureSettings.tsx | 2 +- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index f460b07af5d4..b62262fe3572 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -104,6 +104,22 @@ const AddMetadataToEntity: FC = ({ const [metadataChanged, setMetadataChanged] = useState(false) + useEffect(() => { + if (!metadataFieldsAssociatedtoEntity) return + + const totalRequired = metadataFieldsAssociatedtoEntity.filter( + (m) => m.isRequiredFor, + ).length + const totalFilledRequired = metadataFieldsAssociatedtoEntity.filter( + (m) => m.field_value && m.field_value !== '' && m.isRequiredFor, + ).length + + setHasMetadataRequired?.( + totalRequired > 0 && totalFilledRequired < totalRequired, + ) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [metadataFieldsAssociatedtoEntity]) + const mergeMetadataEntityWithMetadataField = ( metadata: Metadata[], // Metadata array metadataField: CustomMetadataField[], // Custom metadata field array @@ -158,11 +174,11 @@ const AddMetadataToEntity: FC = ({ }) // Determine if isRequiredFor should be true or false based on is_required_for array const isRequiredFor = !!matchingItem?.is_required_for.length - setHasMetadataRequired?.(isRequiredFor) + // Return the metadata field with additional metadata model field information including isRequiredFor return { ...meta, - isRequiredFor: isRequiredFor || false, + isRequiredFor, metadataModelFieldId: matchingItem ? matchingItem.id : null, } }) diff --git a/frontend/web/components/modals/create-feature/index.js b/frontend/web/components/modals/create-feature/index.js index ea838aef32c5..ccdf8758557e 100644 --- a/frontend/web/components/modals/create-feature/index.js +++ b/frontend/web/components/modals/create-feature/index.js @@ -829,9 +829,6 @@ const Index = class extends Component { > {({ permission: projectAdmin }) => { this.state.skipSaveProjectFeature = !createFeature - const _hasMetadataRequired = - this.state.hasMetadataRequired && - !projectFlag.metadata?.length return (
@@ -1739,7 +1736,7 @@ const Index = class extends Component { isSaving || !projectFlag.name || invalid || - _hasMetadataRequired + this.state.hasMetadataRequired } > {isSaving @@ -1820,11 +1817,15 @@ const Index = class extends Component { } onHasMetadataRequiredChange={( hasMetadataRequired, - ) => + ) => { + console.log( + 'hasMetadataRequired', + hasMetadataRequired, + ) this.setState({ hasMetadataRequired, }) - } + }} featureError={ this.parseError(error).featureError } @@ -1842,7 +1843,9 @@ const Index = class extends Component { featureLimitPercentage={ this.state.featureLimitAlert.percentage } - hasMetadataRequired={_hasMetadataRequired} + hasMetadataRequired={ + this.state.hasMetadataRequired + } />
)} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx index 456f0e068fd4..28d4b1ffa273 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx @@ -75,7 +75,7 @@ const FeatureSettings: FC = ({ )} {metadataEnable && featureContentType?.id && !identity && ( <> - + {/* */} Date: Thu, 5 Feb 2026 16:24:54 +0100 Subject: [PATCH 03/16] feat: refactor-metadata-with-hooks --- .../common/hooks/useEntityMetadataFields.ts | 159 ++++++ .../__tests__/metadataValidation.test.ts | 90 ++++ frontend/common/utils/metadataValidation.ts | 29 + .../metadata/AddMetadataToEntity.tsx | 496 ++++++------------ .../create-feature/tabs/FeatureSettings.tsx | 2 +- 5 files changed, 438 insertions(+), 338 deletions(-) create mode 100644 frontend/common/hooks/useEntityMetadataFields.ts create mode 100644 frontend/common/utils/__tests__/metadataValidation.test.ts create mode 100644 frontend/common/utils/metadataValidation.ts diff --git a/frontend/common/hooks/useEntityMetadataFields.ts b/frontend/common/hooks/useEntityMetadataFields.ts new file mode 100644 index 000000000000..4848a12d282c --- /dev/null +++ b/frontend/common/hooks/useEntityMetadataFields.ts @@ -0,0 +1,159 @@ +import { useMemo } from 'react' +import { sortBy } from 'lodash' +import { useGetMetadataModelFieldListQuery } from 'common/services/useMetadataModelField' +import { useGetMetadataFieldListQuery } from 'common/services/useMetadataField' +import { useGetSegmentQuery } from 'common/services/useSegment' +import { useGetEnvironmentQuery } from 'common/services/useEnvironment' +import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' +import { MetadataField, Metadata } from 'common/types/responses' + +export type CustomMetadataField = MetadataField & { + metadataModelFieldId: number | string | null + isRequiredFor: boolean + model_field?: string | number + hasValue?: boolean + field_value?: string +} + +type UseEntityMetadataFieldsParams = { + organisationId: number + projectId: number + entityContentType: number + entityType: 'feature' | 'segment' | 'environment' + entityId?: number +} + +type UseEntityMetadataFieldsResult = { + metadataFields: CustomMetadataField[] + isLoading: boolean +} + +/** + * Merges field definitions with existing entity values. + * This takes the list of metadata field definitions and enriches them + * with any existing values from the entity. + */ +function mergeFieldDefinitionsWithValues( + fieldDefinitions: CustomMetadataField[], + existingValues: Metadata[], +): CustomMetadataField[] { + return fieldDefinitions.map((field) => { + const existingValue = existingValues.find( + (v) => v.model_field === field.metadataModelFieldId, + ) + return { + ...field, + field_value: existingValue?.field_value ?? '', + hasValue: !!existingValue, + } + }) +} + +/** + * Hook that fetches and merges metadata fields for an entity. + * + * This encapsulates the complex data fetching and merging logic that was + * previously spread across multiple useEffects in AddMetadataToEntity. + */ +export function useEntityMetadataFields({ + entityContentType, + entityId, + entityType, + organisationId, + projectId, +}: UseEntityMetadataFieldsParams): UseEntityMetadataFieldsResult { + // Fetch all metadata field definitions for the organisation + const { data: metadataFieldList, isLoading: metadataFieldListLoading } = + useGetMetadataFieldListQuery({ + organisation: organisationId, + }) + + // Fetch all model field mappings + const { data: metadataModelFieldList, isLoading: metadataModelFieldLoading } = + useGetMetadataModelFieldListQuery({ + organisation_id: organisationId, + }) + + // Fetch entity-specific data based on type + const { data: projectFeatureData, isLoading: projectFeatureLoading } = + useGetProjectFlagQuery( + { id: entityId!, project: projectId }, + { skip: entityType !== 'feature' || !entityId }, + ) + + const { data: segmentData, isLoading: segmentLoading } = useGetSegmentQuery( + { id: entityId!, projectId }, + { skip: entityType !== 'segment' || !entityId || !projectId }, + ) + + const { data: envData, isLoading: envLoading } = useGetEnvironmentQuery( + { id: entityId! }, + { skip: entityType !== 'environment' || !entityId }, + ) + + // Compute the merged metadata fields + const metadataFields = useMemo(() => { + if (!metadataFieldList || !metadataModelFieldList) { + return [] + } + + // Filter metadata fields that apply to this content type + const fieldsForContentType: CustomMetadataField[] = + metadataFieldList.results + .filter((meta) => + metadataModelFieldList.results.some( + (item) => + item.field === meta.id && item.content_type === entityContentType, + ), + ) + .map((meta) => { + const matchingItem = metadataModelFieldList.results.find( + (item) => + item.field === meta.id && item.content_type === entityContentType, + ) + return { + ...meta, + isRequiredFor: !!matchingItem?.is_required_for.length, + metadataModelFieldId: matchingItem ? matchingItem.id : null, + } + }) + + // Get existing values from the entity + let existingValues: Metadata[] = [] + if (entityType === 'feature' && projectFeatureData?.metadata) { + existingValues = projectFeatureData.metadata + } else if (entityType === 'segment' && segmentData?.metadata) { + existingValues = segmentData.metadata + } else if (entityType === 'environment' && envData?.metadata) { + existingValues = envData.metadata + } + + // Merge field definitions with existing values + const mergedMetadata = mergeFieldDefinitionsWithValues( + fieldsForContentType, + existingValues, + ) + + return sortBy(mergedMetadata, (m) => (m.isRequiredFor ? -1 : 1)) + }, [ + metadataFieldList, + metadataModelFieldList, + entityContentType, + entityType, + projectFeatureData, + segmentData, + envData, + ]) + + const isLoading = + metadataFieldListLoading || + metadataModelFieldLoading || + (entityType === 'feature' && entityId && projectFeatureLoading) || + (entityType === 'segment' && entityId && segmentLoading) || + (entityType === 'environment' && entityId && envLoading) + + return { + isLoading: !!isLoading, + metadataFields, + } +} diff --git a/frontend/common/utils/__tests__/metadataValidation.test.ts b/frontend/common/utils/__tests__/metadataValidation.test.ts new file mode 100644 index 000000000000..0f2d110734d5 --- /dev/null +++ b/frontend/common/utils/__tests__/metadataValidation.test.ts @@ -0,0 +1,90 @@ +import { getGlobalMetadataValidationState } from 'common/utils/metadataValidation' +import { CustomMetadataField } from 'common/hooks/useEntityMetadataFields' + +const createField = ( + partialField: Partial = {}, +): CustomMetadataField => ({ + description: 'A test field', + field_value: '', + hasValue: false, + id: 1, + isRequiredFor: false, + metadataModelFieldId: 1, + name: 'Test Field', + organisation: 1, + type: 'str', + ...partialField, +}) + +describe('getMetadataValidationState', () => { + it('returns all zeros for empty fields array', () => { + const result = getGlobalMetadataValidationState([]) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 0, + totalRequired: 0, + }) + }) + + it('returns hasUnfilledRequired false when no required fields', () => { + const fields = [ + createField({ id: 1, isRequiredFor: false }), + createField({ id: 2, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 0, + totalRequired: 0, + }) + }) + + it('returns hasUnfilledRequired false when required field is filled', () => { + const fields = [ + createField({ field_value: 'some value', id: 1, isRequiredFor: true }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 1, + totalRequired: 1, + }) + }) + + it('returns hasUnfilledRequired true when some required fields are unfilled', () => { + const fields = [ + createField({ field_value: 'filled', id: 1, isRequiredFor: true }), + createField({ field_value: '', id: 2, isRequiredFor: true }), + createField({ field_value: '', id: 3, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: true, + totalFilledRequired: 1, + totalRequired: 2, + }) + }) + + it('returns hasUnfilledRequired false when all required fields are filled', () => { + const fields = [ + createField({ field_value: 'filled', id: 1, isRequiredFor: true }), + createField({ field_value: 'also filled', id: 2, isRequiredFor: true }), + createField({ field_value: '', id: 3, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 2, + totalRequired: 2, + }) + }) +}) diff --git a/frontend/common/utils/metadataValidation.ts b/frontend/common/utils/metadataValidation.ts new file mode 100644 index 000000000000..e84dc3522c4d --- /dev/null +++ b/frontend/common/utils/metadataValidation.ts @@ -0,0 +1,29 @@ +import { useMemo } from 'react' +import { CustomMetadataField } from 'common/hooks/useEntityMetadataFields' + +export type MetadataValidationState = { + hasUnfilledRequired: boolean + totalRequired: number + totalFilledRequired: number +} + +export function getGlobalMetadataValidationState( + fields: CustomMetadataField[], +): MetadataValidationState { + const totalRequired = fields.filter((f) => f.isRequiredFor).length + const totalFilledRequired = fields.filter( + (f) => f.isRequiredFor && f.field_value && f.field_value !== '', + ).length + + return { + hasUnfilledRequired: + totalRequired > 0 && totalFilledRequired < totalRequired, + totalFilledRequired, + totalRequired, + } +} +export function useGlobalMetadataValidation( + fields: CustomMetadataField[], +): MetadataValidationState { + return useMemo(() => getGlobalMetadataValidationState(fields), [fields]) +} diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index b62262fe3572..c78222de0d6b 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -1,43 +1,57 @@ -import React, { FC, useEffect, useState } from 'react' +import React, { FC, useCallback, useEffect, useState } from 'react' import PanelSearch from 'components/PanelSearch' import Button from 'components/base/forms/Button' -import { useGetMetadataModelFieldListQuery } from 'common/services/useMetadataModelField' -import { useGetMetadataFieldListQuery } from 'common/services/useMetadataField' -import { useGetSegmentQuery } from 'common/services/useSegment' -import { - useGetEnvironmentQuery, - useUpdateEnvironmentMutation, -} from 'common/services/useEnvironment' -import { MetadataField, Metadata } from 'common/types/responses' +import { useUpdateEnvironmentMutation } from 'common/services/useEnvironment' +import { Metadata } from 'common/types/responses' import Utils from 'common/utils/utils' -import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' -import { sortBy } from 'lodash' import Switch from 'components/Switch' import InputGroup from 'components/base/forms/InputGroup' +import { + useEntityMetadataFields, + CustomMetadataField, +} from 'common/hooks/useEntityMetadataFields' +import { useGlobalMetadataValidation } from 'common/utils/metadataValidation' -export type CustomMetadataField = MetadataField & { - metadataModelFieldId: number | string | null - isRequiredFor: boolean - model_field?: string | number - metadataEntity?: boolean - field_value?: string -} - -type CustomMetadata = (Metadata & CustomMetadataField) | null +export type { CustomMetadataField } -type AddMetadataToEntityType = { +type AddMetadataToEntityProps = { isCloningEnvironment?: boolean - organisationId: string - projectId: string | number + organisationId: number + projectId: number entityContentType: number - entityId: string + entityId?: number entity: string envName?: string - onChange?: (m: CustomMetadataField[]) => void + onChange?: (metadata: Metadata[]) => void setHasMetadataRequired?: (b: boolean) => void } -const AddMetadataToEntity: FC = ({ +function formatMetadataToApi(fields: CustomMetadataField[]): Metadata[] { + return fields + .filter((f) => f.hasValue) + .map(({ field_value, metadataModelFieldId }) => ({ + field_value: field_value ?? '', + model_field: metadataModelFieldId as number, + })) +} + +type MetadataErrorResponse = { + data?: { + metadata?: Array<{ + non_field_errors?: string[] + [key: string]: unknown + }> + } +} + +function getMetadataErrors(error: MetadataErrorResponse): string { + const metadataErrors = error?.data?.metadata + if (!metadataErrors) return '' + + return metadataErrors.flatMap((m) => m.non_field_errors ?? []).join('\n') +} + +const AddMetadataToEntity: FC = ({ entity, entityContentType, entityId, @@ -48,364 +62,172 @@ const AddMetadataToEntity: FC = ({ projectId, setHasMetadataRequired, }) => { - const { data: metadataFieldList, isSuccess: metadataFieldListLoaded } = - useGetMetadataFieldListQuery({ - organisation: organisationId, - }) - const { - data: metadataModelFieldList, - isSuccess: metadataModelFieldListLoaded, - } = useGetMetadataModelFieldListQuery({ - organisation_id: organisationId, + const { isLoading, metadataFields: initialFields } = useEntityMetadataFields({ + entityContentType, + entityId: entityId, + entityType: entity as 'feature' | 'segment' | 'environment', + organisationId, + projectId, }) - const { data: projectFeatureData, isSuccess: projectFeatureDataLoaded } = - useGetProjectFlagQuery( - { - id: entityId, - project: projectId, - }, - { skip: entity !== 'feature' || !entityId }, - ) - - const { data: segmentData, isSuccess: segmentDataLoaded } = - useGetSegmentQuery( - { - id: `${entityId}`, - projectId: `${projectId}`, - }, - { skip: entity !== 'segment' || !entityId }, - ) - - const { data: envData, isSuccess: envDataLoaded } = useGetEnvironmentQuery( - { id: entityId }, - { skip: entity !== 'environment' || !entityId }, + const [metadataFields, setMetadataFields] = useState( + [], ) + const [hasChanges, setHasChanges] = useState(false) - const [updateEnvironment] = useUpdateEnvironmentMutation() - - const [ - metadataFieldsAssociatedtoEntity, - setMetadataFieldsAssociatedtoEntity, - ] = useState() + const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (metadataFieldsAssociatedtoEntity?.length && metadataChanged) { - const metadataParsed = metadataFieldsAssociatedtoEntity - .filter((m) => m.metadataEntity) - .map((i) => { - const { metadataModelFieldId, ...rest } = i - return { model_field: metadataModelFieldId, ...rest } - }) - onChange?.(metadataParsed as CustomMetadataField[]) + if (initialFields.length > 0 && metadataFields.length === 0) { + setMetadataFields(initialFields) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [metadataFieldsAssociatedtoEntity]) - - const [metadataChanged, setMetadataChanged] = useState(false) + }, [initialFields, metadataFields.length]) useEffect(() => { - if (!metadataFieldsAssociatedtoEntity) return - - const totalRequired = metadataFieldsAssociatedtoEntity.filter( - (m) => m.isRequiredFor, - ).length - const totalFilledRequired = metadataFieldsAssociatedtoEntity.filter( - (m) => m.field_value && m.field_value !== '' && m.isRequiredFor, - ).length - - setHasMetadataRequired?.( - totalRequired > 0 && totalFilledRequired < totalRequired, - ) + setHasMetadataRequired?.(hasUnfilledRequired) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [metadataFieldsAssociatedtoEntity]) + }, [hasUnfilledRequired]) + + const handleFieldChange = useCallback( + (fieldId: number, newValue: string) => { + setMetadataFields((prev) => { + const updatedMetadataFields = prev.map((field) => + field.id === fieldId + ? { ...field, field_value: newValue, hasValue: !!newValue } + : field, + ) - const mergeMetadataEntityWithMetadataField = ( - metadata: Metadata[], // Metadata array - metadataField: CustomMetadataField[], // Custom metadata field array - ) => { - // Create a map of metadata fields using metadataModelFieldId as key - const map = new Map( - metadataField.map((item) => [item.metadataModelFieldId, item]), - ) + // Propagate the change to upstream parents + if (entity !== 'environment' || isCloningEnvironment) { + const formattedMetadata = formatMetadataToApi(updatedMetadataFields) + onChange?.(formattedMetadata) + } + + return updatedMetadataFields + }) + setHasChanges(true) + }, + [entity, isCloningEnvironment, onChange], + ) - // Merge metadata fields with metadata entities - return metadataField.map((item) => { - const mergedItem = { - ...item, // Spread the properties of the metadata field - ...(map.get(item.model_field!) || {}), // Get the corresponding metadata field from the map - ...(metadata.find((m) => m.model_field === item.metadataModelFieldId) || - {}), // Find the corresponding metadata entity - } + const [updateEnvironment] = useUpdateEnvironmentMutation() - // Determine if metadata entity exists - mergedItem.metadataEntity = - mergedItem.metadataModelFieldId !== undefined && - mergedItem.model_field !== undefined + const handleEnvironmentSave = async () => { + if (!envName || !entityId) return - return mergedItem // Return the merged item + const result = await updateEnvironment({ + body: { + metadata: formatMetadataToApi(metadataFields), + name: envName, + project: projectId, + }, + id: entityId, }) - } - useEffect(() => { - if ( - metadataFieldList && - metadataFieldListLoaded && - metadataModelFieldList && - metadataModelFieldListLoaded - ) { - // Filter metadata fields based on the provided content type - const metadataForContentType = metadataFieldList.results - // Filter metadata fields that have corresponding entries in the metadata model field list - .filter((meta) => { - return metadataModelFieldList.results.some((item) => { - return ( - item.field === meta.id && item.content_type === entityContentType - ) - }) - }) - // Map each filtered metadata field to include additional information from the metadata model field list - .map((meta) => { - // Find the matching item in the metadata model field list - const matchingItem = metadataModelFieldList.results.find((item) => { - return ( - item.field === meta.id && item.content_type === entityContentType - ) - }) - // Determine if isRequiredFor should be true or false based on is_required_for array - const isRequiredFor = !!matchingItem?.is_required_for.length - - // Return the metadata field with additional metadata model field information including isRequiredFor - return { - ...meta, - isRequiredFor, - metadataModelFieldId: matchingItem ? matchingItem.id : null, - } - }) - if (projectFeatureData?.metadata && projectFeatureDataLoaded) { - const mergedFeatureEntity = mergeMetadataEntityWithMetadataField( - projectFeatureData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedFeatureEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else if (segmentData?.metadata && segmentDataLoaded) { - const mergedSegmentEntity = mergeMetadataEntityWithMetadataField( - segmentData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedSegmentEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else if (envData?.metadata && envDataLoaded) { - const mergedEnvEntity = mergeMetadataEntityWithMetadataField( - envData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedEnvEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else { - const sortedArray = sortBy(metadataForContentType, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } + if ('error' in result) { + toast(getMetadataErrors(result.error as MetadataErrorResponse), 'danger') + } else { + toast('Environment Field Updated') + setHasChanges(false) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - metadataFieldList, - metadataFieldListLoaded, - metadataModelFieldList, - metadataModelFieldListLoaded, - projectFeatureDataLoaded, - projectFeatureData, - entityId, - envData, - envDataLoaded, - segmentData, - segmentDataLoaded, - ]) - - const getMetadataErrors = (error: any) => { - const nonFieldErrors = - error?.data?.metadata?.map( - (metadata: any) => metadata?.non_field_errors, - ) || [] - const fieldErrors = - error?.data?.metadata?.map((metadata: any) => metadata) || [] - - const allErrors = [...nonFieldErrors, ...fieldErrors] - - return allErrors.join('\n') } return ( - <> - - - Field - Value - - } - items={metadataFieldsAssociatedtoEntity} - renderRow={(m) => { - return ( - { - setMetadataFieldsAssociatedtoEntity((prevState) => - prevState?.map((metadata) => { - if (metadata.id === m?.id) { - return { - ...metadata, - field_value: m?.field_value, - metadataEntity: !!m?.field_value, - } - } - return metadata - }), - ) - setMetadataChanged(true) - }} - /> - ) - }} - renderNoResults={ - - No custom fields configured for {entity}s. Add custom fields in - your{' '} - - Organisation Settings - - . - - } - /> - {entity === 'environment' && !isCloningEnvironment && ( -
- -
+ + + Field + Value + + } + items={metadataFields} + renderRow={(field: CustomMetadataField) => ( + )} - - + renderNoResults={ + + No custom fields configured for {entity}s. Add custom fields in your{' '} + + Organisation Settings + + . + + } + /> + {entity === 'environment' && !isCloningEnvironment && ( +
+ +
+ )} +
) } -type MetadataRowType = { +type MetadataRowProps = { metadata: CustomMetadataField - getMetadataValue?: (metadata: CustomMetadata) => void - entity: string + onFieldChange: (fieldId: number, value: string) => void } -const MetadataRow: FC = ({ - entity, - getMetadataValue, - metadata, -}) => { - const [metadataValueChanged, setMetadataValueChanged] = - useState(false) - const metadataValue = - metadata?.type === 'bool' - ? metadata?.field_value === 'true' - : metadata?.field_value || '' - const handleChange = (newMetadataValue: string | boolean) => { - setMetadataValueChanged(false) - const updatedMetadataObject = { ...metadata } - updatedMetadataObject.field_value = - metadata?.type === 'bool' ? `${!newMetadataValue}` : `${newMetadataValue}` - getMetadataValue?.(updatedMetadataObject as CustomMetadata) - } +const MetadataRow: FC = ({ metadata, onFieldChange }) => { + const displayValue = + metadata.type === 'bool' + ? metadata.field_value === 'true' + : metadata.field_value || '' - const isRequiredForAndCorrectType = - metadata?.isRequiredFor && - Utils.validateMetadataType(metadata?.type, metadataValue) - const isNotRequiredAndCorrectType = - !!metadataValue && Utils.validateMetadataType(metadata?.type, metadataValue) - const isEmptyAuthorized = !metadataValue && !metadata?.isRequiredFor + const handleChange = (newValue: string | boolean) => { + const stringValue = metadata.type === 'bool' ? `${newValue}` : `${newValue}` + onFieldChange(metadata.id, stringValue) + } + const isEmpty = !displayValue || displayValue === '' + const isValidType = Utils.validateMetadataType(metadata.type, displayValue) + const isValid = isEmpty ? !metadata.isRequiredFor : isValidType return ( - {metadataValueChanged && entity !== 'segment' && ( -
{'*'}
- )} - {`${metadata?.name} ${ - metadata?.isRequiredFor ? '*' : '' - }`} - {metadata?.type === 'bool' ? ( + {metadata.name} + {metadata.type === 'bool' ? ( { - setMetadataValueChanged(true) - handleChange(!metadataValue) + const currentBool = + displayValue === true || displayValue === 'true' + handleChange(!currentBool) }} /> ) : ( { - setMetadataValueChanged(true) handleChange(Utils.safeParseEventValue(e)) }} type='text' diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx index 28d4b1ffa273..456f0e068fd4 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureSettings.tsx @@ -75,7 +75,7 @@ const FeatureSettings: FC = ({ )} {metadataEnable && featureContentType?.id && !identity && ( <> - {/* */} + Date: Thu, 5 Feb 2026 17:26:00 +0100 Subject: [PATCH 04/16] feat: consolidated-requests-and-merged-strategy-in-rtk --- .../common/hooks/useEntityMetadataFields.ts | 159 ------------------ frontend/common/services/useMetadataField.ts | 111 ++++++++++++ frontend/common/types/metadata-field.ts | 9 + .../__tests__/mergeMetadataFields.test.ts | 103 ++++++++++++ .../__tests__/metadataValidation.test.ts | 2 +- frontend/common/utils/mergeMetadataFields.ts | 60 +++++++ frontend/common/utils/metadataValidation.ts | 2 +- .../metadata/AddMetadataToEntity.tsx | 32 ++-- .../pages/CreateEnvironmentPage.tsx | 74 ++++---- 9 files changed, 342 insertions(+), 210 deletions(-) delete mode 100644 frontend/common/hooks/useEntityMetadataFields.ts create mode 100644 frontend/common/types/metadata-field.ts create mode 100644 frontend/common/utils/__tests__/mergeMetadataFields.test.ts create mode 100644 frontend/common/utils/mergeMetadataFields.ts diff --git a/frontend/common/hooks/useEntityMetadataFields.ts b/frontend/common/hooks/useEntityMetadataFields.ts deleted file mode 100644 index 4848a12d282c..000000000000 --- a/frontend/common/hooks/useEntityMetadataFields.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { useMemo } from 'react' -import { sortBy } from 'lodash' -import { useGetMetadataModelFieldListQuery } from 'common/services/useMetadataModelField' -import { useGetMetadataFieldListQuery } from 'common/services/useMetadataField' -import { useGetSegmentQuery } from 'common/services/useSegment' -import { useGetEnvironmentQuery } from 'common/services/useEnvironment' -import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' -import { MetadataField, Metadata } from 'common/types/responses' - -export type CustomMetadataField = MetadataField & { - metadataModelFieldId: number | string | null - isRequiredFor: boolean - model_field?: string | number - hasValue?: boolean - field_value?: string -} - -type UseEntityMetadataFieldsParams = { - organisationId: number - projectId: number - entityContentType: number - entityType: 'feature' | 'segment' | 'environment' - entityId?: number -} - -type UseEntityMetadataFieldsResult = { - metadataFields: CustomMetadataField[] - isLoading: boolean -} - -/** - * Merges field definitions with existing entity values. - * This takes the list of metadata field definitions and enriches them - * with any existing values from the entity. - */ -function mergeFieldDefinitionsWithValues( - fieldDefinitions: CustomMetadataField[], - existingValues: Metadata[], -): CustomMetadataField[] { - return fieldDefinitions.map((field) => { - const existingValue = existingValues.find( - (v) => v.model_field === field.metadataModelFieldId, - ) - return { - ...field, - field_value: existingValue?.field_value ?? '', - hasValue: !!existingValue, - } - }) -} - -/** - * Hook that fetches and merges metadata fields for an entity. - * - * This encapsulates the complex data fetching and merging logic that was - * previously spread across multiple useEffects in AddMetadataToEntity. - */ -export function useEntityMetadataFields({ - entityContentType, - entityId, - entityType, - organisationId, - projectId, -}: UseEntityMetadataFieldsParams): UseEntityMetadataFieldsResult { - // Fetch all metadata field definitions for the organisation - const { data: metadataFieldList, isLoading: metadataFieldListLoading } = - useGetMetadataFieldListQuery({ - organisation: organisationId, - }) - - // Fetch all model field mappings - const { data: metadataModelFieldList, isLoading: metadataModelFieldLoading } = - useGetMetadataModelFieldListQuery({ - organisation_id: organisationId, - }) - - // Fetch entity-specific data based on type - const { data: projectFeatureData, isLoading: projectFeatureLoading } = - useGetProjectFlagQuery( - { id: entityId!, project: projectId }, - { skip: entityType !== 'feature' || !entityId }, - ) - - const { data: segmentData, isLoading: segmentLoading } = useGetSegmentQuery( - { id: entityId!, projectId }, - { skip: entityType !== 'segment' || !entityId || !projectId }, - ) - - const { data: envData, isLoading: envLoading } = useGetEnvironmentQuery( - { id: entityId! }, - { skip: entityType !== 'environment' || !entityId }, - ) - - // Compute the merged metadata fields - const metadataFields = useMemo(() => { - if (!metadataFieldList || !metadataModelFieldList) { - return [] - } - - // Filter metadata fields that apply to this content type - const fieldsForContentType: CustomMetadataField[] = - metadataFieldList.results - .filter((meta) => - metadataModelFieldList.results.some( - (item) => - item.field === meta.id && item.content_type === entityContentType, - ), - ) - .map((meta) => { - const matchingItem = metadataModelFieldList.results.find( - (item) => - item.field === meta.id && item.content_type === entityContentType, - ) - return { - ...meta, - isRequiredFor: !!matchingItem?.is_required_for.length, - metadataModelFieldId: matchingItem ? matchingItem.id : null, - } - }) - - // Get existing values from the entity - let existingValues: Metadata[] = [] - if (entityType === 'feature' && projectFeatureData?.metadata) { - existingValues = projectFeatureData.metadata - } else if (entityType === 'segment' && segmentData?.metadata) { - existingValues = segmentData.metadata - } else if (entityType === 'environment' && envData?.metadata) { - existingValues = envData.metadata - } - - // Merge field definitions with existing values - const mergedMetadata = mergeFieldDefinitionsWithValues( - fieldsForContentType, - existingValues, - ) - - return sortBy(mergedMetadata, (m) => (m.isRequiredFor ? -1 : 1)) - }, [ - metadataFieldList, - metadataModelFieldList, - entityContentType, - entityType, - projectFeatureData, - segmentData, - envData, - ]) - - const isLoading = - metadataFieldListLoading || - metadataModelFieldLoading || - (entityType === 'feature' && entityId && projectFeatureLoading) || - (entityType === 'segment' && entityId && segmentLoading) || - (entityType === 'environment' && entityId && envLoading) - - return { - isLoading: !!isLoading, - metadataFields, - } -} diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index e33711b000bd..c089ee1494a2 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -2,6 +2,44 @@ import { Res } from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' import Utils from 'common/utils/utils' +import { CustomMetadataField } from 'common/types/metadata-field' +import { + Environment, + MetadataField, + MetadataModelField, + PagedResponse, + ProjectFlag, + Segment, +} from 'common/types/responses' +import { mergeMetadataFields } from 'common/utils/mergeMetadataFields' + +type EntityType = 'feature' | 'segment' | 'environment' + +type EntityMetadataParams = { + organisationId: number + projectId: number + entityContentType: number + entityType: EntityType + entityId?: number +} + +type EntityData = ProjectFlag | Segment | Environment + +function getEntityUrl(params: EntityMetadataParams): string | null { + const { entityId, entityType, projectId } = params + if (!entityId) return null + + switch (entityType) { + case 'feature': + return `projects/${projectId}/features/${entityId}/` + case 'segment': + return `projects/${projectId}/segments/${entityId}/` + case 'environment': + return `environments/${entityId}/` + default: + return null + } +} export const metadataService = service .enhanceEndpoints({ addTagTypes: ['Metadata'] }) @@ -28,6 +66,67 @@ export const metadataService = service url: `metadata/fields/${query.id}/`, }), }), + getEntityMetadataFields: builder.query< + CustomMetadataField[], + EntityMetadataParams + >({ + providesTags: (_res, _err, arg) => [ + { + id: `${arg.entityType}-${arg.entityId ?? 'new'}-${ + arg.entityContentType + }`, + type: 'Metadata', + }, + ], + queryFn: async (arg, _api, _extraOptions, baseQuery) => { + const entityUrl = getEntityUrl(arg) + + // Build queries to run in parallel + const queries: Promise<{ data?: unknown; error?: unknown }>[] = [ + baseQuery({ + url: `metadata/fields/?${Utils.toParam({ + organisation: arg.organisationId, + })}`, + }), + baseQuery({ + url: `organisations/${arg.organisationId}/metadata-model-fields/`, + }), + ] + + // Only fetch entity data if we have an entityId + if (entityUrl) { + queries.push(baseQuery({ url: entityUrl })) + } + + // Fetch all in parallel + const results = await Promise.all(queries) + + const [fieldsRes, modelFieldsRes, entityRes] = results + + // Handle errors + if (fieldsRes.error) { + return { error: fieldsRes.error as Res['metadataList'] } + } + if (modelFieldsRes.error) { + return { + error: modelFieldsRes.error as Res['metadataModelFieldList'], + } + } + if (entityRes?.error) { + return { error: entityRes.error as EntityData } + } + + // Merge and return + const mergedMetadata = mergeMetadataFields( + fieldsRes.data as PagedResponse, + modelFieldsRes.data as PagedResponse, + entityRes?.data as EntityData | null, + arg.entityContentType, + ) + + return { data: mergedMetadata } + }, + }), getMetadataField: builder.query< Res['metadataField'], Req['getMetadataField'] @@ -119,11 +218,23 @@ export async function updateMetadata( metadataService.endpoints.updateMetadataField.initiate(data, options), ) } +export async function getEntityMetadataFields( + store: any, + data: EntityMetadataParams, + options?: Parameters< + typeof metadataService.endpoints.getEntityMetadataFields.initiate + >[1], +) { + return store.dispatch( + metadataService.endpoints.getEntityMetadataFields.initiate(data, options), + ) +} // END OF FUNCTION_EXPORTS export const { useCreateMetadataFieldMutation, useDeleteMetadataFieldMutation, + useGetEntityMetadataFieldsQuery, useGetMetadataFieldListQuery, useGetMetadataFieldQuery, useUpdateMetadataFieldMutation, diff --git a/frontend/common/types/metadata-field.ts b/frontend/common/types/metadata-field.ts new file mode 100644 index 000000000000..a71dbf84088c --- /dev/null +++ b/frontend/common/types/metadata-field.ts @@ -0,0 +1,9 @@ +import { MetadataField } from './responses' + +export type CustomMetadataField = MetadataField & { + metadataModelFieldId: number | string | null + isRequiredFor: boolean + model_field?: string | number + hasValue?: boolean + field_value?: string +} diff --git a/frontend/common/utils/__tests__/mergeMetadataFields.test.ts b/frontend/common/utils/__tests__/mergeMetadataFields.test.ts new file mode 100644 index 000000000000..1d23d15eab30 --- /dev/null +++ b/frontend/common/utils/__tests__/mergeMetadataFields.test.ts @@ -0,0 +1,103 @@ +import { mergeMetadataFields } from 'common/utils/mergeMetadataFields' +import { + MetadataField, + MetadataModelField, + PagedResponse, +} from 'common/types/responses' + +const createFieldList = ( + fields: Partial[], +): PagedResponse => ({ + results: fields.map((f, idx) => ({ + description: 'Test description', + id: idx + 1, + name: `Field ${idx + 1}`, + organisation: 1, + type: 'str', + ...f, + })) as MetadataField[], +}) + +const createModelFieldList = ( + modelFields: Partial[], +): PagedResponse => ({ + results: modelFields.map((mf, idx) => ({ + content_type: 100, + field: idx + 1, + id: `${idx + 10}`, + is_required_for: [], + ...mf, + })) as MetadataModelField[], +}) + +describe('mergeMetadataFields', () => { + it('merges field definitions with existing values', () => { + const fieldList = createFieldList([{ id: 1, name: 'Field 1' }]) + const modelFieldList = createModelFieldList([ + { content_type: 100, field: 1, id: '10', is_required_for: [] }, + ]) + const entityData = { + metadata: [{ field_value: 'existing value', model_field: '10' }], + } + + const result = mergeMetadataFields( + fieldList, + modelFieldList, + entityData, + 100, + ) + + expect(result).toHaveLength(1) + expect(result[0].field_value).toBe('existing value') + expect(result[0].hasValue).toBe(true) + }) + + it('sets empty field_value when no existing value or null entity', () => { + const fieldList = createFieldList([{ id: 1, name: 'Field 1' }]) + const modelFieldList = createModelFieldList([ + { content_type: 100, field: 1, id: '10', is_required_for: [] }, + ]) + + const result = mergeMetadataFields(fieldList, modelFieldList, null, 100) + + expect(result[0].field_value).toBe('') + expect(result[0].hasValue).toBe(false) + }) + + it('marks field as required when is_required_for has entries', () => { + const fieldList = createFieldList([{ id: 1, name: 'Required Field' }]) + const modelFieldList = createModelFieldList([ + { + content_type: 100, + field: 1, + id: '10', + is_required_for: [{ content_type: 50 }], + }, + ]) + + const result = mergeMetadataFields(fieldList, modelFieldList, null, 100) + + expect(result[0].isRequiredFor).toBe(true) + }) + + it('sorts required fields first', () => { + const fieldList = createFieldList([ + { id: 1, name: 'Optional' }, + { id: 2, name: 'Required' }, + ]) + const modelFieldList = createModelFieldList([ + { content_type: 100, field: 1, id: '10', is_required_for: [] }, + { + content_type: 100, + field: 2, + id: '11', + is_required_for: [{ content_type: 50 }], + }, + ]) + + const result = mergeMetadataFields(fieldList, modelFieldList, null, 100) + + expect(result[0].name).toBe('Required') + expect(result[1].name).toBe('Optional') + }) +}) diff --git a/frontend/common/utils/__tests__/metadataValidation.test.ts b/frontend/common/utils/__tests__/metadataValidation.test.ts index 0f2d110734d5..3e2300f15a02 100644 --- a/frontend/common/utils/__tests__/metadataValidation.test.ts +++ b/frontend/common/utils/__tests__/metadataValidation.test.ts @@ -1,5 +1,5 @@ import { getGlobalMetadataValidationState } from 'common/utils/metadataValidation' -import { CustomMetadataField } from 'common/hooks/useEntityMetadataFields' +import { CustomMetadataField } from 'common/types/metadata-field' const createField = ( partialField: Partial = {}, diff --git a/frontend/common/utils/mergeMetadataFields.ts b/frontend/common/utils/mergeMetadataFields.ts new file mode 100644 index 000000000000..fc9cf4ac5891 --- /dev/null +++ b/frontend/common/utils/mergeMetadataFields.ts @@ -0,0 +1,60 @@ +import { sortBy } from 'lodash' +import { + Metadata, + MetadataField, + MetadataModelField, + PagedResponse, +} from 'common/types/responses' +import { CustomMetadataField } from 'common/types/metadata-field' + +type EntityWithMetadata = { + metadata?: Metadata[] +} + +/** + * Merges metadata field definitions with model field mappings and existing entity values. + */ +export function mergeMetadataFields( + fieldList: PagedResponse, + modelFieldList: PagedResponse, + entityData: EntityWithMetadata | null, + entityContentType: number, +): CustomMetadataField[] { + // Filter fields that apply to this content type + const fieldsForContentType: CustomMetadataField[] = fieldList.results + .filter((meta) => + modelFieldList.results.some( + (item) => + item.field === meta.id && item.content_type === entityContentType, + ), + ) + .map((meta) => { + const matchingItem = modelFieldList.results.find( + (item) => + item.field === meta.id && item.content_type === entityContentType, + ) + return { + ...meta, + isRequiredFor: !!matchingItem?.is_required_for.length, + metadataModelFieldId: matchingItem ? matchingItem.id : null, + } + }) + + // Get existing values from the entity + const existingValues: Metadata[] = entityData?.metadata ?? [] + + // Merge field definitions with existing values + const mergedMetadata = fieldsForContentType.map((field) => { + const existingValue = existingValues.find( + (v) => v.model_field === field.metadataModelFieldId, + ) + return { + ...field, + field_value: existingValue?.field_value ?? '', + hasValue: !!existingValue, + } + }) + + // Sort required fields first + return sortBy(mergedMetadata, (m) => (m.isRequiredFor ? -1 : 1)) +} diff --git a/frontend/common/utils/metadataValidation.ts b/frontend/common/utils/metadataValidation.ts index e84dc3522c4d..d66e9c4be57c 100644 --- a/frontend/common/utils/metadataValidation.ts +++ b/frontend/common/utils/metadataValidation.ts @@ -1,5 +1,5 @@ import { useMemo } from 'react' -import { CustomMetadataField } from 'common/hooks/useEntityMetadataFields' +import { CustomMetadataField } from 'common/types/metadata-field' export type MetadataValidationState = { hasUnfilledRequired: boolean diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index c78222de0d6b..47a8cc63ff07 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -6,14 +6,10 @@ import { Metadata } from 'common/types/responses' import Utils from 'common/utils/utils' import Switch from 'components/Switch' import InputGroup from 'components/base/forms/InputGroup' -import { - useEntityMetadataFields, - CustomMetadataField, -} from 'common/hooks/useEntityMetadataFields' +import { useGetEntityMetadataFieldsQuery } from 'common/services/useMetadataField' +import { CustomMetadataField } from 'common/types/metadata-field' import { useGlobalMetadataValidation } from 'common/utils/metadataValidation' -export type { CustomMetadataField } - type AddMetadataToEntityProps = { isCloningEnvironment?: boolean organisationId: number @@ -62,13 +58,14 @@ const AddMetadataToEntity: FC = ({ projectId, setHasMetadataRequired, }) => { - const { isLoading, metadataFields: initialFields } = useEntityMetadataFields({ - entityContentType, - entityId: entityId, - entityType: entity as 'feature' | 'segment' | 'environment', - organisationId, - projectId, - }) + const { data: initialFields = [], isLoading } = + useGetEntityMetadataFieldsQuery({ + entityContentType, + entityId, + entityType: entity as 'feature' | 'segment' | 'environment', + organisationId, + projectId, + }) const [metadataFields, setMetadataFields] = useState( [], @@ -170,7 +167,9 @@ const AddMetadataToEntity: FC = ({ @@ -209,31 +238,8 @@ const CreateEnvironmentPage: React.FC = () => { )} - ) : ( -
-

- Check your project permissions -

-

- Although you have been invited to this project, you are not - invited to any environments yet! -

-

- Contact your project administrator asking them to either: -

    -
  • - Invite you to an environment (e.g. develop) by visiting{' '} - Environment settings -
  • -
  • - Grant permissions to create an environment under{' '} - Project settings. -
  • -
-

-
) - } + }} ) From 6f8c33f35c9fa73eafb94fe6614e13cfbe0b10b0 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 16 Feb 2026 10:31:36 +0100 Subject: [PATCH 05/16] feat: resolved-review-comments --- frontend/common/services/useMetadataField.ts | 1 - frontend/common/utils/__tests__/metadataValidation.test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index c089ee1494a2..0114669454a2 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -27,7 +27,6 @@ type EntityData = ProjectFlag | Segment | Environment function getEntityUrl(params: EntityMetadataParams): string | null { const { entityId, entityType, projectId } = params - if (!entityId) return null switch (entityType) { case 'feature': diff --git a/frontend/common/utils/__tests__/metadataValidation.test.ts b/frontend/common/utils/__tests__/metadataValidation.test.ts index 3e2300f15a02..860bd86df82c 100644 --- a/frontend/common/utils/__tests__/metadataValidation.test.ts +++ b/frontend/common/utils/__tests__/metadataValidation.test.ts @@ -16,7 +16,7 @@ const createField = ( ...partialField, }) -describe('getMetadataValidationState', () => { +describe('getGlobalMetadataValidationState', () => { it('returns all zeros for empty fields array', () => { const result = getGlobalMetadataValidationState([]) From 2c6cf8b7e7b45b913c486f068b9e48b186c5960b Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 17 Feb 2026 15:42:52 +0100 Subject: [PATCH 06/16] fix: skip-feature-metadata-fetch-when-creating-feature --- frontend/common/services/useMetadataField.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index 0114669454a2..7730df4db555 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -93,7 +93,7 @@ export const metadataService = service ] // Only fetch entity data if we have an entityId - if (entityUrl) { + if (arg.entityId && entityUrl) { queries.push(baseQuery({ url: entityUrl })) } From 463d388d16417f1f65745d26b78cd96d3e2a94ef Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 17 Feb 2026 16:19:08 +0100 Subject: [PATCH 07/16] fix: fixed-parsing-error --- frontend/web/components/modals/create-feature/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frontend/web/components/modals/create-feature/index.js b/frontend/web/components/modals/create-feature/index.js index fe52a7a90323..242a25baf2bc 100644 --- a/frontend/web/components/modals/create-feature/index.js +++ b/frontend/web/components/modals/create-feature/index.js @@ -498,7 +498,11 @@ const Index = class extends Component { } parseError = (error) => { const { projectFlag } = this.props - let featureError = error?.message || error?.name?.[0] || error + let featureError = + error?.metadata?.flatMap((m) => m.non_field_errors ?? []).join('\n') || + error?.message || + error?.name?.[0] || + error let featureWarning = '' //Treat multivariate no changes as warnings if ( From 0503ad32263676efa7777702360cb262bc7ed7b0 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 17 Feb 2026 17:53:32 +0100 Subject: [PATCH 08/16] feat: invalidate-tags-on-delete-metadata --- frontend/common/services/useMetadataField.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index 7730df4db555..8304b6836c6a 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -48,7 +48,7 @@ export const metadataService = service Res['metadataField'], Req['createMetadataField'] >({ - invalidatesTags: [{ id: 'LIST', type: 'Metadata' }], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['createMetadataField']) => ({ body: query.body, method: 'POST', @@ -59,7 +59,7 @@ export const metadataService = service Res['metadataField'], Req['deleteMetadataField'] >({ - invalidatesTags: [{ id: 'LIST', type: 'Metadata' }], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['deleteMetadataField']) => ({ method: 'DELETE', url: `metadata/fields/${query.id}/`, @@ -148,10 +148,7 @@ export const metadataService = service Res['metadataField'], Req['updateMetadataField'] >({ - invalidatesTags: (res) => [ - { id: 'LIST', type: 'Metadata' }, - { id: res?.id, type: 'Metadata' }, - ], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['updateMetadataField']) => ({ body: query.body, method: 'PUT', From 3cc20aa19a3f656179c2dad294eb6a3fcd38836a Mon Sep 17 00:00:00 2001 From: wood Date: Thu, 19 Feb 2026 15:49:04 +0100 Subject: [PATCH 09/16] feat: fixed-use-effect-on-entity-id-switch --- frontend/web/components/metadata/AddMetadataToEntity.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index 47a8cc63ff07..e477050ad12f 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -75,10 +75,10 @@ const AddMetadataToEntity: FC = ({ const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (initialFields.length > 0 && metadataFields.length === 0) { + if (initialFields.length > 0) { setMetadataFields(initialFields) } - }, [initialFields, metadataFields.length]) + }, [initialFields]) useEffect(() => { setHasMetadataRequired?.(hasUnfilledRequired) From 77bbdc079c4d85ff6f4ac058c3ddfc416a1571a8 Mon Sep 17 00:00:00 2001 From: wood Date: Thu, 19 Feb 2026 15:52:09 +0100 Subject: [PATCH 10/16] feat: cleaned-up-boolean-no-op --- frontend/web/components/metadata/AddMetadataToEntity.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index e477050ad12f..010cc83430ee 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -192,8 +192,7 @@ const MetadataRow: FC = ({ metadata, onFieldChange }) => { : metadata.field_value || '' const handleChange = (newValue: string | boolean) => { - const stringValue = metadata.type === 'bool' ? `${newValue}` : `${newValue}` - onFieldChange(metadata.id, stringValue) + onFieldChange(metadata.id, `${newValue}`) } const isEmpty = !displayValue || displayValue === '' const isValidType = Utils.validateMetadataType(metadata.type, displayValue) From e2ad26e776cdf24b64156bafa62f08fe20fa8e12 Mon Sep 17 00:00:00 2001 From: wood Date: Thu, 19 Feb 2026 17:50:02 +0100 Subject: [PATCH 11/16] feat: only-sync-local-state-without-edit --- frontend/web/components/metadata/AddMetadataToEntity.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index 010cc83430ee..2858b4341bf6 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -75,10 +75,10 @@ const AddMetadataToEntity: FC = ({ const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (initialFields.length > 0) { + if (initialFields.length > 0 && !hasChanges) { setMetadataFields(initialFields) } - }, [initialFields]) + }, [initialFields, hasChanges]) useEffect(() => { setHasMetadataRequired?.(hasUnfilledRequired) From 129ca9cadf3aae7e4e30800b2746eb670e2290f8 Mon Sep 17 00:00:00 2001 From: wood Date: Mon, 23 Feb 2026 14:04:55 +0100 Subject: [PATCH 12/16] fix: env-custom-fields-reverting-local-value-on-change --- frontend/common/services/useMetadataField.ts | 4 ++++ .../components/metadata/AddMetadataToEntity.tsx | 15 +++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index 8304b6836c6a..f169a5361df5 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -28,6 +28,10 @@ type EntityData = ProjectFlag | Segment | Environment function getEntityUrl(params: EntityMetadataParams): string | null { const { entityId, entityType, projectId } = params + if (!entityId) { + return null + } + switch (entityType) { case 'feature': return `projects/${projectId}/features/${entityId}/` diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index 2858b4341bf6..35fc1ab7f2a0 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -6,7 +6,11 @@ import { Metadata } from 'common/types/responses' import Utils from 'common/utils/utils' import Switch from 'components/Switch' import InputGroup from 'components/base/forms/InputGroup' -import { useGetEntityMetadataFieldsQuery } from 'common/services/useMetadataField' +import { + metadataService, + useGetEntityMetadataFieldsQuery, +} from 'common/services/useMetadataField' +import { getStore } from 'common/store' import { CustomMetadataField } from 'common/types/metadata-field' import { useGlobalMetadataValidation } from 'common/utils/metadataValidation' @@ -75,10 +79,11 @@ const AddMetadataToEntity: FC = ({ const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (initialFields.length > 0 && !hasChanges) { + if (initialFields.length > 0) { setMetadataFields(initialFields) + setHasChanges(false) } - }, [initialFields, hasChanges]) + }, [initialFields]) useEffect(() => { setHasMetadataRequired?.(hasUnfilledRequired) @@ -125,7 +130,9 @@ const AddMetadataToEntity: FC = ({ toast(getMetadataErrors(result.error as MetadataErrorResponse), 'danger') } else { toast('Environment Field Updated') - setHasChanges(false) + getStore().dispatch( + metadataService.util.invalidateTags([{ type: 'Metadata' }]), + ) } } From 937aecb18bc6b9d8476bf666be5eba7a13ec3fbc Mon Sep 17 00:00:00 2001 From: wood Date: Mon, 23 Feb 2026 15:19:09 +0100 Subject: [PATCH 13/16] fix: stabilize-empty-reference --- .../web/components/metadata/AddMetadataToEntity.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index 35fc1ab7f2a0..c74fc74acf46 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -14,6 +14,8 @@ import { getStore } from 'common/store' import { CustomMetadataField } from 'common/types/metadata-field' import { useGlobalMetadataValidation } from 'common/utils/metadataValidation' +const EMPTY_FIELDS: CustomMetadataField[] = [] + type AddMetadataToEntityProps = { isCloningEnvironment?: boolean organisationId: number @@ -62,7 +64,7 @@ const AddMetadataToEntity: FC = ({ projectId, setHasMetadataRequired, }) => { - const { data: initialFields = [], isLoading } = + const { data: initialFields = EMPTY_FIELDS, isLoading } = useGetEntityMetadataFieldsQuery({ entityContentType, entityId, @@ -79,10 +81,8 @@ const AddMetadataToEntity: FC = ({ const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (initialFields.length > 0) { - setMetadataFields(initialFields) - setHasChanges(false) - } + setMetadataFields(initialFields) + setHasChanges(false) }, [initialFields]) useEffect(() => { From 5f502cd08fccd38e77e59aee5ed7f3f6dc6fb3dd Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 24 Feb 2026 08:33:09 +0100 Subject: [PATCH 14/16] fix: invalidate-cache-on-mounting --- .../components/metadata/AddMetadataToEntity.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index c74fc74acf46..6596c6a0c8c1 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -65,13 +65,16 @@ const AddMetadataToEntity: FC = ({ setHasMetadataRequired, }) => { const { data: initialFields = EMPTY_FIELDS, isLoading } = - useGetEntityMetadataFieldsQuery({ - entityContentType, - entityId, - entityType: entity as 'feature' | 'segment' | 'environment', - organisationId, - projectId, - }) + useGetEntityMetadataFieldsQuery( + { + entityContentType, + entityId, + entityType: entity as 'feature' | 'segment' | 'environment', + organisationId, + projectId, + }, + { refetchOnMountOrArgChange: true }, + ) const [metadataFields, setMetadataFields] = useState( [], From a68c1a63744e180efb791c7b7275774510f8dc7f Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 24 Feb 2026 15:55:26 +0100 Subject: [PATCH 15/16] fix: fixed-empty-toas-if-message-error --- frontend/web/components/metadata/AddMetadataToEntity.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index 6596c6a0c8c1..b657dc81dfb9 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -130,7 +130,10 @@ const AddMetadataToEntity: FC = ({ }) if ('error' in result) { - toast(getMetadataErrors(result.error as MetadataErrorResponse), 'danger') + const errorMessage = getMetadataErrors( + result.error as MetadataErrorResponse, + ) + toast(errorMessage || 'Failed to update custom fields', 'danger') } else { toast('Environment Field Updated') getStore().dispatch( From ca9f6817f5fc7b07aa2a1b9dd8a19e135775ead4 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 24 Feb 2026 17:37:42 +0100 Subject: [PATCH 16/16] fix: undefined-create-segment-id-blocking-metadata --- frontend/web/components/modals/CreateSegment.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/web/components/modals/CreateSegment.tsx b/frontend/web/components/modals/CreateSegment.tsx index 642b1444a6ec..3022a9ff92c9 100644 --- a/frontend/web/components/modals/CreateSegment.tsx +++ b/frontend/web/components/modals/CreateSegment.tsx @@ -466,7 +466,7 @@ const CreateSegment: FC = ({ {