From 0e53bafa6e2857f9cbb6d6b9170636f16e2a4217 Mon Sep 17 00:00:00 2001 From: Kyle Johnson Date: Tue, 10 Sep 2024 12:05:38 +0100 Subject: [PATCH] feat: Detect unchanged feature states when saving versions (#4609) --- frontend/common/services/useFeatureVersion.ts | 348 ++++++++++-------- frontend/common/stores/feature-list-store.ts | 5 + frontend/common/types/requests.ts | 1 + frontend/web/components/modals/CreateFlag.js | 2 +- frontend/web/project/api.js | 4 + 5 files changed, 205 insertions(+), 155 deletions(-) diff --git a/frontend/common/services/useFeatureVersion.ts b/frontend/common/services/useFeatureVersion.ts index 7f09d044218b..041e68ec0650 100644 --- a/frontend/common/services/useFeatureVersion.ts +++ b/frontend/common/services/useFeatureVersion.ts @@ -1,9 +1,9 @@ import { FeatureState, - FeatureVersion, + FeatureVersion, PagedResponse, Res, Segment, -} from 'common/types/responses' +} from 'common/types/responses'; import { Req } from 'common/types/requests' import { service } from 'common/service' import { getStore } from 'common/store' @@ -11,24 +11,26 @@ 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'; 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, - }), - ), - })) + 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, + }), + ), + })) export const getFeatureStateCrud = ( - featureStates: FeatureState[], - oldFeatureStates?: FeatureState[], - segments?: Segment[] | null | undefined, + featureStates: FeatureState[], + oldFeatureStates: FeatureState[], + segments?: Segment[] | null | undefined, ) => { const excludeNotChanged = (featureStates: FeatureState[]) => { if (!oldFeatureStates) { @@ -37,21 +39,21 @@ export const getFeatureStateCrud = ( if (segments?.length) { // filter out feature states that have no changes const segmentDiffs = getSegmentDiff( - featureStates, - oldFeatureStates, - segments, + featureStates, + oldFeatureStates.map((v)=>({...v,feature_state_value:Utils.featureStateToValue(v.feature_state_value)})), + 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 [] @@ -60,13 +62,13 @@ export const getFeatureStateCrud = ( } } const featureStatesToCreate: Req['createFeatureVersion']['feature_states_to_create'] = - featureStates.filter((v) => !v.id && !v.toRemove) + featureStates.filter((v) => !v.id && !v.toRemove) const featureStatesToUpdate: Req['createFeatureVersion']['feature_states_to_update'] = - excludeNotChanged(featureStates.filter((v) => !!v.id && !v.toRemove)) + 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) @@ -78,170 +80,208 @@ 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']) => { - const { - feature_states_to_create, - feature_states_to_update, - segment_ids_to_delete_overrides, - } = getFeatureStateCrud(query.featureStates) - const versionRes: { data: FeatureVersion } = - await createFeatureVersion(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, + .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 + + + const { 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 currentFeatureStates: { data: FeatureState[] } = - await getVersionFeatureState(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, - sha: versionRes.data.uuid, - }) + 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 res = currentFeatureStates.data + const currentFeatureStates: { data: FeatureState[] } = + await getVersionFeatureState(getStore(), { + environmentId: query.environmentId, + featureId: query.featureId, + sha: versionRes.data.uuid, + }) - const ret = { - error: res.find((v) => !!v.error)?.error, - feature_states: res.map((item) => ({ - data: item, + 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, 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 + }, }), - }), - getFeatureVersion: builder.query< - Res['featureVersion'], - Req['getFeatureVersion'] - >({ - providesTags: (res) => [{ id: res?.uuid, type: 'FeatureVersion' }], - query: (query: Req['getFeatureVersion']) => ({ - url: `environment-feature-versions/${query.uuid}/`, + 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/`, + }), }), - }), - 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)}`, + getFeatureVersion: builder.query< + Res['featureVersion'], + Req['getFeatureVersion'] + >({ + providesTags: (res) => [{ id: res?.uuid, type: 'FeatureVersion' }], + query: (query: Req['getFeatureVersion']) => ({ + url: `environment-feature-versions/${query.uuid}/`, + }), }), - 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/`, + 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) + }, + }), + 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 3192b79892f5..2939ffd91017 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -473,6 +473,9 @@ const controller = { environment: environmentFlag.environment, feature: projectFlag.id, }, + { + forceRefetch: true + } ) let segments = null if (mode === 'SEGMENT') { @@ -709,6 +712,7 @@ const controller = { return createAndSetFeatureVersion(getStore(), { environmentId: res, featureId: projectFlag.id, + projectId, featureStates, }).then((version) => { if (version.error) { @@ -764,6 +768,7 @@ const controller = { feature_state_value: flag.initial_value, }) return createAndSetFeatureVersion(getStore(), { + projectId, environmentId: res, featureId: projectFlag.id, featureStates: [data], diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index c4e5935b0d21..e84f9b4a4300 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -319,6 +319,7 @@ export type Req = { getGroupWithRole: { org_id: number; group_id: number } deleteGroupWithRole: { org_id: number; group_id: number; role_id: number } createAndSetFeatureVersion: { + projectId: number environmentId: number featureId: number skipPublish?: boolean diff --git a/frontend/web/components/modals/CreateFlag.js b/frontend/web/components/modals/CreateFlag.js index 8e938fe6d690..1d02c20cce8b 100644 --- a/frontend/web/components/modals/CreateFlag.js +++ b/frontend/web/components/modals/CreateFlag.js @@ -1069,7 +1069,7 @@ const CreateFlag = class extends Component { )} +
Environment Value{' '}
diff --git a/frontend/web/project/api.js b/frontend/web/project/api.js index 8ac745d78ec9..e2756788c0c1 100644 --- a/frontend/web/project/api.js +++ b/frontend/web/project/api.js @@ -30,6 +30,10 @@ global.API = { store.error = res.data store.goneABitWest() return + } else if (typeof res.text !== 'function') { + store.error = res + store.goneABitWest() + return } res