From 8d1c22e898fceaa4f35c3e0a291283a8b414da07 Mon Sep 17 00:00:00 2001 From: Kyle Johnson Date: Tue, 8 Oct 2024 14:46:55 +0100 Subject: [PATCH] fix: diff check for versioned segment overrides and MV (#4656) --- frontend/common/services/useFeatureVersion.ts | 428 ++++++++++-------- frontend/common/stores/feature-list-store.ts | 21 +- frontend/common/types/responses.ts | 6 +- frontend/e2e/helpers.cafe.ts | 1 - frontend/web/components/WarningMessage.tsx | 3 + frontend/web/components/diff/DiffFeature.tsx | 11 +- frontend/web/components/diff/DiffSegments.tsx | 90 ++-- .../web/components/diff/DiffVariations.tsx | 53 ++- frontend/web/components/diff/diff-utils.ts | 36 +- frontend/web/components/modals/CreateFlag.js | 279 ++++++------ frontend/web/components/mv/VariationValue.js | 2 +- 11 files changed, 503 insertions(+), 427 deletions(-) diff --git a/frontend/common/services/useFeatureVersion.ts b/frontend/common/services/useFeatureVersion.ts index 041e68ec0650..6f0f87de2a34 100644 --- a/frontend/common/services/useFeatureVersion.ts +++ b/frontend/common/services/useFeatureVersion.ts @@ -1,78 +1,92 @@ import { FeatureState, - FeatureVersion, PagedResponse, + FeatureVersion, + PagedResponse, Res, Segment, -} from 'common/types/responses'; + TypedFeatureState, +} from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' import { getStore } from 'common/store' import { getVersionFeatureState } from './useVersionFeatureState' import transformCorePaging from 'common/transformCorePaging' import Utils from 'common/utils/utils' -import { getFeatureStateDiff, getSegmentDiff } from 'components/diff/diff-utils' -import { getSegments } from 'common/services/useSegment'; -import { getFeatureStates } from 'common/services/useFeatureState'; +import { + getFeatureStateDiff, + getSegmentDiff, + getVariationDiff, +} from 'components/diff/diff-utils' +import { getSegments } from './useSegment' +import { getFeatureStates } from './useFeatureState' -const transformFeatureStates = (featureStates: FeatureState[]) => - featureStates?.map((v) => ({ - ...v, - feature_state_value: Utils.valueToFeatureState(v.feature_state_value), - id: undefined, - multivariate_feature_state_values: v.multivariate_feature_state_values?.map( - (v) => ({ - ...v, - id: undefined, - }), - ), - })) +const transformFeatureStates = (featureStates: TypedFeatureState[]) => + featureStates?.map((v) => ({ + ...v, + feature_state_value: v.feature_state_value, + id: undefined, + multivariate_feature_state_values: v.multivariate_feature_state_values?.map( + (v) => ({ + ...v, + id: undefined, + }), + ), + })) export const getFeatureStateCrud = ( - featureStates: FeatureState[], - oldFeatureStates: FeatureState[], - segments?: Segment[] | null | undefined, + featureStates: TypedFeatureState[], + oldFeatureStates: TypedFeatureState[], + segments?: Segment[] | null | undefined, ) => { - const excludeNotChanged = (featureStates: FeatureState[]) => { + const excludeNotChanged = (featureStates: TypedFeatureState[]) => { if (!oldFeatureStates) { return featureStates } if (segments?.length) { // filter out feature states that have no changes const segmentDiffs = getSegmentDiff( - featureStates, - oldFeatureStates.map((v)=>({...v,feature_state_value:Utils.featureStateToValue(v.feature_state_value)})), - segments, + featureStates, + oldFeatureStates, + segments, ) return featureStates.filter((v) => { const diff = segmentDiffs?.diffs?.find( - (diff) => v.feature_segment?.segment === diff.segment.id, + (diff) => v.feature_segment?.segment === diff.segment.id, ) return !!diff?.totalChanges }) } else { // return nothing if feature state isn't different const valueDiff = getFeatureStateDiff( - featureStates[0], - oldFeatureStates[0], + featureStates[0], + oldFeatureStates[0], ) if (!valueDiff.totalChanges) { - return [] + const variationDiff = getVariationDiff( + featureStates[0], + oldFeatureStates[0], + ) + if (!variationDiff.totalChanges) return [] } return featureStates } } - const featureStatesToCreate: Req['createFeatureVersion']['feature_states_to_create'] = - featureStates.filter((v) => !v.id && !v.toRemove) - const featureStatesToUpdate: Req['createFeatureVersion']['feature_states_to_update'] = - excludeNotChanged(featureStates.filter((v) => !!v.id && !v.toRemove)) + const featureStatesToCreate = featureStates.filter( + (v) => !v.id && !v.toRemove, + ) + const featureStatesToUpdate = excludeNotChanged( + featureStates.filter((v) => !!v.id && !v.toRemove), + ) const segment_ids_to_delete_overrides: Req['createFeatureVersion']['segment_ids_to_delete_overrides'] = - featureStates - .filter((v) => !!v.id && !!v.toRemove && !!v.feature_segment) - .map((v) => v.feature_segment!.segment) + featureStates + .filter((v) => !!v.id && !!v.toRemove && !!v.feature_segment) + .map((v) => v.feature_segment!.segment) // Step 1: Create a new feature version - const feature_states_to_create = transformFeatureStates(featureStatesToCreate) - const feature_states_to_update = transformFeatureStates(featureStatesToUpdate) + const feature_states_to_create: Req['createFeatureVersion']['feature_states_to_create'] = + transformFeatureStates(featureStatesToCreate) + const feature_states_to_update: Req['createFeatureVersion']['feature_states_to_update'] = + transformFeatureStates(featureStatesToUpdate) return { feature_states_to_create, feature_states_to_update, @@ -80,208 +94,222 @@ export const getFeatureStateCrud = ( } } export const featureVersionService = service - .enhanceEndpoints({ addTagTypes: ['FeatureVersion'] }) - .injectEndpoints({ - endpoints: (builder) => ({ - createAndSetFeatureVersion: builder.mutation< - Res['featureVersion'], - Req['createAndSetFeatureVersion'] - >({ - invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], - queryFn: async (query: Req['createAndSetFeatureVersion']) => { - // todo: this will be removed when we combine saving value and segment overrides - const mode = query.featureStates.find((v)=>!v.feature_segment?.segment) ? 'VALUE' : 'SEGMENT' - const oldFeatureStates: { data: PagedResponse } = (await getFeatureStates( - getStore(), - { - environment: query.environmentId, - feature: query.featureId, - }, - { - forceRefetch: true - } - )) - let segments = mode === 'VALUE'? undefined: (await getSegments(getStore(), { - include_feature_specific: true, - page_size: 1000, - projectId: query.projectId, - })).data.results + .enhanceEndpoints({ addTagTypes: ['FeatureVersion'] }) + .injectEndpoints({ + endpoints: (builder) => ({ + createAndSetFeatureVersion: builder.mutation< + Res['featureVersion'], + Req['createAndSetFeatureVersion'] + >({ + invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], + queryFn: async (query: Req['createAndSetFeatureVersion']) => { + // todo: this will be removed when we combine saving value and segment overrides + const mode = query.featureStates.find( + (v) => !v.feature_segment?.segment, + ) + ? 'VALUE' + : 'SEGMENT' + const oldFeatureStates: { data: PagedResponse } = + await getFeatureStates( + getStore(), + { + environment: query.environmentId, + feature: query.featureId, + }, + { + forceRefetch: true, + }, + ) + const segments = + mode === 'VALUE' + ? undefined + : ( + await getSegments(getStore(), { + include_feature_specific: true, + page_size: 1000, + projectId: query.projectId, + }) + ).data.results + + const { + feature_states_to_create, + feature_states_to_update, + segment_ids_to_delete_overrides, + } = getFeatureStateCrud( + query.featureStates.map((v) => ({ + ...v, + feature_state_value: Utils.valueToFeatureState( + v.feature_state_value, + ), + })), + oldFeatureStates.data.results.filter((v) => { + if (mode === 'VALUE') { + return !v.feature_segment?.segment + } else { + return !!v.feature_segment?.segment + } + }), + segments, + ) + if ( + !feature_states_to_create.length && + !feature_states_to_update.length && + !segment_ids_to_delete_overrides.length + ) { + throw new Error('Feature contains no changes') + } - const { + const versionRes: { data: FeatureVersion } = + await createFeatureVersion(getStore(), { + environmentId: query.environmentId, + featureId: query.featureId, feature_states_to_create, feature_states_to_update, + live_from: query.liveFrom, + publish_immediately: !query.skipPublish, segment_ids_to_delete_overrides, - } = getFeatureStateCrud( - query.featureStates, - oldFeatureStates.data.results.filter((v)=>{ - if(mode === 'VALUE') { - return !v.feature_segment?.segment - } else { - return !!v.feature_segment?.segment - } - }), - segments, - ) - - if ( - !feature_states_to_create.length && - !feature_states_to_update.length && - !segment_ids_to_delete_overrides.length - ) { - throw new Error('Feature contains no changes') - } - - const versionRes: { data: FeatureVersion } = - await createFeatureVersion(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, - feature_states_to_create, - feature_states_to_update, - live_from: query.liveFrom, - publish_immediately: !query.skipPublish, - segment_ids_to_delete_overrides, - }) + }) - const currentFeatureStates: { data: FeatureState[] } = - await getVersionFeatureState(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, - sha: versionRes.data.uuid, - }) + const currentFeatureStates: { data: FeatureState[] } = + await getVersionFeatureState(getStore(), { + environmentId: query.environmentId, + featureId: query.featureId, + sha: versionRes.data.uuid, + }) - const res = currentFeatureStates.data + const res = currentFeatureStates.data - const ret = { - error: res.find((v) => !!v.error)?.error, - feature_states: res.map((item) => ({ - data: item, - version_sha: versionRes.data.uuid, - })), - feature_states_to_create, - feature_states_to_update, - segment_ids_to_delete_overrides, + const ret = { + error: res.find((v) => !!v.error)?.error, + feature_states: res.map((item) => ({ + data: item, version_sha: versionRes.data.uuid, - } + })), + feature_states_to_create, + feature_states_to_update, + segment_ids_to_delete_overrides, + version_sha: versionRes.data.uuid, + } - return { data: ret } as any - }, - }), - createFeatureVersion: builder.mutation< - Res['featureVersion'], - Req['createFeatureVersion'] - >({ - invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], - query: (query: Req['createFeatureVersion']) => ({ - body: query, - method: 'POST', - url: `environments/${query.environmentId}/features/${query.featureId}/versions/`, - }), + return { data: ret } as any + }, + }), + createFeatureVersion: builder.mutation< + Res['featureVersion'], + Req['createFeatureVersion'] + >({ + invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], + query: (query: Req['createFeatureVersion']) => ({ + body: query, + method: 'POST', + url: `environments/${query.environmentId}/features/${query.featureId}/versions/`, }), - getFeatureVersion: builder.query< - Res['featureVersion'], - Req['getFeatureVersion'] - >({ - providesTags: (res) => [{ id: res?.uuid, type: 'FeatureVersion' }], - query: (query: Req['getFeatureVersion']) => ({ - url: `environment-feature-versions/${query.uuid}/`, - }), + }), + getFeatureVersion: builder.query< + Res['featureVersion'], + Req['getFeatureVersion'] + >({ + providesTags: (res) => [{ id: res?.uuid, type: 'FeatureVersion' }], + query: (query: Req['getFeatureVersion']) => ({ + url: `environment-feature-versions/${query.uuid}/`, }), - getFeatureVersions: builder.query< - Res['featureVersions'], - Req['getFeatureVersions'] - >({ - providesTags: [{ id: 'LIST', type: 'FeatureVersion' }], - query: (query) => ({ - url: `environments/${query.environmentId}/features/${ - query.featureId - }/versions/?${Utils.toParam(query)}`, - }), - transformResponse: ( - baseQueryReturnValue: Res['featureVersions'], - meta, - req, - ) => { - return transformCorePaging(req, baseQueryReturnValue) - }, + }), + getFeatureVersions: builder.query< + Res['featureVersions'], + Req['getFeatureVersions'] + >({ + providesTags: [{ id: 'LIST', type: 'FeatureVersion' }], + query: (query) => ({ + url: `environments/${query.environmentId}/features/${ + query.featureId + }/versions/?${Utils.toParam(query)}`, }), - publishFeatureVersion: builder.mutation< - Res['featureVersion'], - Req['publishFeatureVersion'] - >({ - invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], - query: (query: Req['publishFeatureVersion']) => ({ - body: query, - method: 'POST', - url: `environments/${query.environmentId}/features/${query.featureId}/versions/${query.sha}/publish/`, - }), + transformResponse: ( + baseQueryReturnValue: Res['featureVersions'], + meta, + req, + ) => { + return transformCorePaging(req, baseQueryReturnValue) + }, + }), + publishFeatureVersion: builder.mutation< + Res['featureVersion'], + Req['publishFeatureVersion'] + >({ + invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], + query: (query: Req['publishFeatureVersion']) => ({ + body: query, + method: 'POST', + url: `environments/${query.environmentId}/features/${query.featureId}/versions/${query.sha}/publish/`, }), - // END OF ENDPOINTS }), - }) + // END OF ENDPOINTS + }), + }) export async function createFeatureVersion( - store: any, - data: Req['createFeatureVersion'], - options?: Parameters< - typeof featureVersionService.endpoints.createFeatureVersion.initiate - >[1], + store: any, + data: Req['createFeatureVersion'], + options?: Parameters< + typeof featureVersionService.endpoints.createFeatureVersion.initiate + >[1], ) { return store.dispatch( - featureVersionService.endpoints.createFeatureVersion.initiate( - data, - options, - ), + featureVersionService.endpoints.createFeatureVersion.initiate( + data, + options, + ), ) } export async function publishFeatureVersion( - store: any, - data: Req['publishFeatureVersion'], - options?: Parameters< - typeof featureVersionService.endpoints.publishFeatureVersion.initiate - >[1], + store: any, + data: Req['publishFeatureVersion'], + options?: Parameters< + typeof featureVersionService.endpoints.publishFeatureVersion.initiate + >[1], ) { return store.dispatch( - featureVersionService.endpoints.publishFeatureVersion.initiate( - data, - options, - ), + featureVersionService.endpoints.publishFeatureVersion.initiate( + data, + options, + ), ) } export async function createAndSetFeatureVersion( - store: any, - data: Req['createAndSetFeatureVersion'], - options?: Parameters< - typeof featureVersionService.endpoints.createAndSetFeatureVersion.initiate - >[1], + store: any, + data: Req['createAndSetFeatureVersion'], + options?: Parameters< + typeof featureVersionService.endpoints.createAndSetFeatureVersion.initiate + >[1], ) { return store.dispatch( - featureVersionService.endpoints.createAndSetFeatureVersion.initiate( - data, - options, - ), + featureVersionService.endpoints.createAndSetFeatureVersion.initiate( + data, + options, + ), ) } export async function getFeatureVersions( - store: any, - data: Req['getFeatureVersions'], - options?: Parameters< - typeof featureVersionService.endpoints.getFeatureVersions.initiate - >[1], + store: any, + data: Req['getFeatureVersions'], + options?: Parameters< + typeof featureVersionService.endpoints.getFeatureVersions.initiate + >[1], ) { return store.dispatch( - featureVersionService.endpoints.getFeatureVersions.initiate(data, options), + featureVersionService.endpoints.getFeatureVersions.initiate(data, options), ) } export async function getFeatureVersion( - store: any, - data: Req['getFeatureVersion'], - options?: Parameters< - typeof featureVersionService.endpoints.getFeatureVersion.initiate - >[1], + store: any, + data: Req['getFeatureVersion'], + options?: Parameters< + typeof featureVersionService.endpoints.getFeatureVersion.initiate + >[1], ) { return store.dispatch( - featureVersionService.endpoints.getFeatureVersion.initiate(data, options), + featureVersionService.endpoints.getFeatureVersion.initiate(data, options), ) } // END OF FUNCTION_EXPORTS diff --git a/frontend/common/stores/feature-list-store.ts b/frontend/common/stores/feature-list-store.ts index c7379c316f9b..be445d281df7 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -53,6 +53,8 @@ const convertSegmentOverrideToFeatureState = ( feature_state_value: override.value, id: override.id, live_from: changeRequest?.live_from, + multivariate_feature_state_values: + override.multivariate_options, toRemove: override.toRemove, } as Partial } @@ -113,7 +115,7 @@ const controller = { }) .then(() => Promise.all([ - data.get(`${Project.api}projects/${projectId}/features/`), + data.get(`${Project.api}projects/${projectId}/features?environment=${ProjectStore.getEnvironmentIdFromKey(environmentId)}`), ]).then(([features]) => { const environmentFeatures = features.results.map((v) => ({ ...v.environment_feature_state, @@ -526,16 +528,9 @@ const controller = { oldFeatureStates, segments, ) - const convertFeatureStateToValue = (v: any) => ({ - ...v, - feature_state_value: Utils.featureStateToValue(v.feature_state_value), - }) - feature_states_to_create = version.feature_states_to_create?.map( - convertFeatureStateToValue, - ) - feature_states_to_update = version.feature_states_to_update?.map( - convertFeatureStateToValue, - ) + + feature_states_to_create = version.feature_states_to_create + feature_states_to_update = version.feature_states_to_update segment_ids_to_delete_overrides = version.segment_ids_to_delete_overrides @@ -865,12 +860,12 @@ const controller = { ? page : `${Project.api}projects/${projectId}/features/?page=${ page || 1 - }&page_size=${pageSize || PAGE_SIZE}${filterUrl}` + }&environment=${environment}&page_size=${pageSize || PAGE_SIZE}${filterUrl}` if (store.search) { featuresEndpoint += `&search=${store.search}` } if (store.sort) { - featuresEndpoint += `&environment=${environment}&sort_field=${ + featuresEndpoint += `&sort_field=${ store.sort.sortBy }&sort_direction=${store.sort.sortOrder.toUpperCase()}` } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index b26f76c33d97..1ea21e07ec8a 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -390,6 +390,10 @@ export type FeatureState = { toRemove?: boolean } +export type TypedFeatureState = Omit & { + feature_state_value: FeatureStateValue +} + export type ProjectFlag = { created_date: string default_enabled: boolean @@ -502,7 +506,7 @@ export type FeatureConflict = { published_at: string is_environment_default: boolean } -export type FeatureStateWithConflict = FeatureState & { +export type FeatureStateWithConflict = TypedFeatureState & { conflict?: FeatureConflict } export type ChangeRequest = { diff --git a/frontend/e2e/helpers.cafe.ts b/frontend/e2e/helpers.cafe.ts index eb92aac91684..fb7465b432a6 100644 --- a/frontend/e2e/helpers.cafe.ts +++ b/frontend/e2e/helpers.cafe.ts @@ -362,7 +362,6 @@ export const editRemoteConfig = async ( } await Promise.all( mvs.map(async (v, i) => { - await setText(byId(`featureVariationValue${i}`), v.value) await setText(byId(`featureVariationWeight${v.value}`), `${v.weight}`) }), ) diff --git a/frontend/web/components/WarningMessage.tsx b/frontend/web/components/WarningMessage.tsx index af5e560a8892..aa5c4667d02a 100644 --- a/frontend/web/components/WarningMessage.tsx +++ b/frontend/web/components/WarningMessage.tsx @@ -14,6 +14,9 @@ const WarningMessage: FC = (props) => { const warningMessageClassName = `alert alert-warning ${ warningMessageClass || 'flex-1 align-items-center' }` + if(!props.warningMessage) { + return null + } return (
= ({ projectId, tabTheme, }) => { + const {data:projectFlag} = useGetProjectFlagQuery({project:projectId, id: featureId}) + const oldEnv = oldState?.find((v) => !v.feature_segment) const newEnv = newState?.find((v) => !v.feature_segment) const { data: feature } = useGetProjectFlagQuery({ @@ -65,7 +67,7 @@ const DiffFeature: FC = ({ const segmentDiffs = disableSegments ? { diffs: [], totalChanges: 0 } : getSegmentDiff(oldState, newState, segments?.results, conflicts) - const variationDiffs = getVariationDiff(oldEnv, newEnv, feature) + const variationDiffs = getVariationDiff(oldEnv, newEnv) const totalSegmentChanges = segmentDiffs?.totalChanges const totalVariationChanges = variationDiffs?.totalChanges useEffect(() => { @@ -105,11 +107,6 @@ const DiffFeature: FC = ({
} > - {!totalChanges && ( -
- No Changes Found -
- )} {!!valueConflict && (
= ({
} > - + )} {!!segmentDiffs?.diffs.length && ( diff --git a/frontend/web/components/diff/DiffSegments.tsx b/frontend/web/components/diff/DiffSegments.tsx index 98c56006651f..6f710cf90927 100644 --- a/frontend/web/components/diff/DiffSegments.tsx +++ b/frontend/web/components/diff/DiffSegments.tsx @@ -9,6 +9,7 @@ import Utils from 'common/utils/utils' import Icon from 'components/Icon' import Tooltip from 'components/Tooltip' import { Link } from 'react-router-dom' +import DiffVariations from './DiffVariations' type DiffSegment = { diff: TDiffSegment @@ -19,49 +20,56 @@ type DiffSegment = { const widths = [200, 80, 105] const DiffSegment: FC = ({ diff, environmentId, projectId }) => { return ( -
-
-
- -
- {!!diff.conflict && } - {diff.segment?.name} -
- {!!diff.conflict && ( - - View Change Request - - )} - - } - > - {diff.conflict - ? 'A change request was published since the creation of this one that also modified the value for this segment.' - : null} - +
+
+
+
+ +
+ {!!diff.conflict && } + {diff.segment?.name} +
+ {!!diff.conflict && ( + + View Change Request + + )} + + } + > + {diff.conflict + ? 'A change request was published since the creation of this one that also modified the value for this segment.' + : null} + +
+
+
+ +
+
+ +
+
+
-
- -
-
- -
-
- +
+ {!!diff.variationDiff?.diffs && ( + + )}
) diff --git a/frontend/web/components/diff/DiffVariations.tsx b/frontend/web/components/diff/DiffVariations.tsx index 89d944431e8e..f6147f4f96a5 100644 --- a/frontend/web/components/diff/DiffVariations.tsx +++ b/frontend/web/components/diff/DiffVariations.tsx @@ -2,13 +2,16 @@ import { TDiffVariation } from './diff-utils' import React, { FC } from 'react' import DiffString from './DiffString' import { DiffMethod } from 'react-diff-viewer-continued' +import { ProjectFlag } from 'common/types/responses'; +import Utils from 'common/utils/utils'; const widths = [120] type DiffVariationsType = { diffs: TDiffVariation[] | undefined + projectFlag: ProjectFlag | undefined } -const DiffVariations: FC = ({ diffs }) => { +const DiffVariations: FC = ({ diffs, projectFlag }) => { const tableHeader = (
Value
@@ -21,28 +24,32 @@ const DiffVariations: FC = ({ diffs }) => { return ( <> {tableHeader} - {diffs?.map((diff, i) => ( - -
- -
-
- -
- - ))} + {diffs?.map((diff, i) => { + const variation = projectFlag?.multivariate_options?.find((v)=>v.id === diff.variationOption) + const stringValue = variation?Utils.featureStateToValue(variation): '' + return ( + +
+ +
+
+ +
+ + ) + })} ) } diff --git a/frontend/web/components/diff/diff-utils.ts b/frontend/web/components/diff/diff-utils.ts index db368c6dfd7d..61ca6c0384fe 100644 --- a/frontend/web/components/diff/diff-utils.ts +++ b/frontend/web/components/diff/diff-utils.ts @@ -6,7 +6,7 @@ import { Segment, } from 'common/types/responses' import Utils from 'common/utils/utils' -import { sortBy } from 'lodash' +import { sortBy, uniq, uniqBy } from 'lodash' export function getFeatureStateDiff( oldFeatureState: FeatureState | undefined, newFeatureState: FeatureStateWithConflict | undefined, @@ -44,14 +44,17 @@ export type TDiffSegment = { oldValue: string conflict?: FeatureConflict totalChanges: number + variationDiff?: { + diffs: TDiffVariation[] + totalChanges: number + } created: boolean deleted: boolean } export type TDiffVariation = { hasChanged: boolean - newValue: string + variationOption: number newWeight: number - oldValue: string oldWeight: number } export type TDiffVariations = { @@ -97,6 +100,8 @@ export const getSegmentDiff = ( } } + const variationDiff = getVariationDiff(oldFeatureState, newFeatureState) + const oldEnabled = !!oldFeatureState?.enabled const oldPriority = oldFeatureState?.feature_segment ? oldFeatureState.feature_segment.priority + 1 @@ -126,6 +131,7 @@ export const getSegmentDiff = ( const segmentChanges = (enabledChanged ? 1 : 0) + (valueChanged ? 1 : 0) + + variationDiff.totalChanges + (priorityChanged ? 1 : 0) if (segmentChanges) { totalChanges += 1 @@ -143,6 +149,7 @@ export const getSegmentDiff = ( oldValue, segment, totalChanges: segmentChanges, + variationDiff, } as TDiffSegment }) return { @@ -155,31 +162,36 @@ export const getSegmentDiff = ( export const getVariationDiff = ( oldFeatureState: FeatureState | undefined, newFeatureState: FeatureState | undefined, - feature: ProjectFlag | undefined, ) => { let totalChanges = 0 - const diffs = feature?.multivariate_options?.map((variationOption) => { + const variationOptions = uniqBy( + oldFeatureState?.multivariate_feature_state_values || + [].concat(newFeatureState?.multivariate_feature_state_values || []), + (v) => v.multivariate_feature_option, + ) + const diffs = variationOptions.map((variationOption) => { const oldMV = oldFeatureState?.multivariate_feature_state_values?.find( - (v) => v.multivariate_feature_option === variationOption.id, + (v) => + v.multivariate_feature_option === + variationOption.multivariate_feature_option, ) const newMV = newFeatureState?.multivariate_feature_state_values?.find( - (v) => v.multivariate_feature_option === variationOption.id, + (v) => + v.multivariate_feature_option === + variationOption.multivariate_feature_option, ) - const oldValue = variationOption.string_value - const newValue = variationOption.string_value // todo: This would eventually be based on the old and new feature versions const oldWeight = oldMV?.percentage_allocation const newWeight = newMV?.percentage_allocation - const hasChanged = oldWeight !== newWeight || oldValue !== newValue + const hasChanged = oldWeight !== newWeight if (hasChanged) { totalChanges += 1 } return { hasChanged, - newValue, newWeight, - oldValue, oldWeight, + variationOption: variationOption.multivariate_feature_option, } as TDiffVariation }) diff --git a/frontend/web/components/modals/CreateFlag.js b/frontend/web/components/modals/CreateFlag.js index 6d968ea6b7b6..99dd33991c6f 100644 --- a/frontend/web/components/modals/CreateFlag.js +++ b/frontend/web/components/modals/CreateFlag.js @@ -45,6 +45,7 @@ import ExternalResourcesLinkTab from 'components/ExternalResourcesLinkTab' import { saveFeatureWithValidation } from 'components/saveFeatureWithValidation' import PlanBasedBanner from 'components/PlanBasedAccess' import FeatureHistory from 'components/FeatureHistory' +import WarningMessage from 'components/WarningMessage'; const CreateFlag = class extends Component { static displayName = 'CreateFlag' @@ -402,7 +403,17 @@ const CreateFlag = class extends Component { }, }) } - +parseError = (error)=>{ + const { projectFlag } = this.props + let featureError = error?.message || error?.name?.[0] || error + let featureWarning = '' + //Treat multivariate no changes as warnings + if(featureError?.includes?.("no changes") && projectFlag?.multivariate_options?.length) { + featureWarning = `Your feature contains no changes to its value, enabled state or environment weights. If you have adjusted any variation values this will have been saved for all environments.` + featureError = '' + } + return {featureError, featureWarning} +} drawChart = (data) => { return data?.length ? ( @@ -558,6 +569,7 @@ const CreateFlag = class extends Component { const metadataEnable = Utils.getPlansPermission('METADATA') && Utils.getFlagsmithHasFeature('enable_metadata') + try { if (!isEdit && name && regex) { regexValid = name.match(new RegExp(regex)) @@ -711,130 +723,134 @@ const CreateFlag = class extends Component { ) - const Value = (error, projectAdmin, createFeature, hideValue) => ( - <> - {!!isEdit && ( - - )} - {!isEdit && ( - - (this.input = e)} - data-test='featureID' - inputProps={{ - className: 'full-width', - maxLength: FEATURE_ID_MAXLENGTH, - name: 'featureID', - readOnly: isEdit, - }} - value={name} - onChange={(e) => { - const newName = Utils.safeParseEventValue(e).replace(/ /g, '_') - this.setState({ - name: caseSensitive ? newName.toLowerCase() : newName, - }) - }} - isValid={!!name && regexValid} - type='text' - title={ - <> - - - {isEdit ? 'ID / Name' : 'ID / Name*'} - - - - } - > - The ID that will be used by SDKs to retrieve the feature - value and enabled state. This cannot be edited once the - feature has been created. - - {!!regex && !isEdit && ( -
- {' '} - - {' '} - This must conform to the regular expression{' '} - 
{regex}
- -
- )} - - } - placeholder='E.g. header_size' - /> - - )} - - {identity && description && ( - - - this.setState({ description: Utils.safeParseEventValue(e) }) - } - type='text' - title={identity ? 'Description' : 'Description (optional)'} - placeholder='No description' - /> - - )} - {!hideValue && ( -
- { - this.setState({ identityVariations, valueChanged: true }) - }} - environmentFlag={this.props.environmentFlag} - projectFlag={projectFlag} - onValueChange={(e) => { - const initial_value = Utils.getTypedValue( - Utils.safeParseEventValue(e), - ) - this.setState({ initial_value, valueChanged: true }) - }} - onCheckedChange={(default_enabled) => - this.setState({ default_enabled }) - } - /> -
- )} - {!isEdit && - !identity && - Settings(projectAdmin, createFeature, featureContentType)} - - ) + const Value = (error, projectAdmin, createFeature, hideValue) => { + const {featureError, featureWarning}= this.parseError(error) + return ( + <> + {!!isEdit && ( + + )} + {!isEdit && ( + + (this.input = e)} + data-test='featureID' + inputProps={{ + className: 'full-width', + maxLength: FEATURE_ID_MAXLENGTH, + name: 'featureID', + readOnly: isEdit, + }} + value={name} + onChange={(e) => { + const newName = Utils.safeParseEventValue(e).replace(/ /g, '_') + this.setState({ + name: caseSensitive ? newName.toLowerCase() : newName, + }) + }} + isValid={!!name && regexValid} + type='text' + title={ + <> + + + {isEdit ? 'ID / Name' : 'ID / Name*'} + + + + } + > + The ID that will be used by SDKs to retrieve the feature + value and enabled state. This cannot be edited once the + feature has been created. + + {!!regex && !isEdit && ( +
+ {' '} + + {' '} + This must conform to the regular expression{' '} + 
{regex}
+ +
+ )} + + } + placeholder='E.g. header_size' + /> + + )} + + + {identity && description && ( + + + this.setState({ description: Utils.safeParseEventValue(e) }) + } + type='text' + title={identity ? 'Description' : 'Description (optional)'} + placeholder='No description' + /> + + )} + {!hideValue && ( +
+ { + this.setState({ identityVariations, valueChanged: true }) + }} + environmentFlag={this.props.environmentFlag} + projectFlag={projectFlag} + onValueChange={(e) => { + const initial_value = Utils.getTypedValue( + Utils.safeParseEventValue(e), + ) + this.setState({ initial_value, valueChanged: true }) + }} + onCheckedChange={(default_enabled) => + this.setState({ default_enabled }) + } + /> +
+ )} + {!isEdit && + !identity && + Settings(projectAdmin, createFeature, featureContentType)} + + ) + } return ( {({ project }) => ( @@ -1025,6 +1041,10 @@ const CreateFlag = class extends Component { project.total_features, project.max_features_allowed, ) + const {featureError, featureWarning}= this.parseError(error) + + + return ( +