From e6b0e2f799d0c6bc513e622ca19b9698f6f76db2 Mon Sep 17 00:00:00 2001 From: Kyle Johnson Date: Tue, 3 Dec 2024 13:18:10 +0000 Subject: [PATCH] feat: combine value + segment change requests for versioned environments (#4738) Co-authored-by: Matthew Elwell --- frontend/common/dispatcher/app-actions.js | 2 - .../common/providers/FeatureListProvider.js | 2 - frontend/common/services/useFeatureVersion.ts | 56 +++++++----- frontend/common/stores/feature-list-store.ts | 47 ++++------ frontend/web/components/modals/CreateFlag.js | 89 +++---------------- 5 files changed, 65 insertions(+), 131 deletions(-) diff --git a/frontend/common/dispatcher/app-actions.js b/frontend/common/dispatcher/app-actions.js index f284e64ec028..0eab0e1d1245 100644 --- a/frontend/common/dispatcher/app-actions.js +++ b/frontend/common/dispatcher/app-actions.js @@ -143,7 +143,6 @@ const AppActions = Object.assign({}, require('./base/_app-actions'), { segmentOverrides, changeRequest, commit, - mode, ) { Dispatcher.handleViewAction({ actionType: Actions.EDIT_ENVIRONMENT_FLAG_CHANGE_REQUEST, @@ -152,7 +151,6 @@ const AppActions = Object.assign({}, require('./base/_app-actions'), { environmentFlag, environmentId, flag, - mode, projectFlag, projectId, segmentOverrides, diff --git a/frontend/common/providers/FeatureListProvider.js b/frontend/common/providers/FeatureListProvider.js index 9d0edcce9782..3b8738bcf141 100644 --- a/frontend/common/providers/FeatureListProvider.js +++ b/frontend/common/providers/FeatureListProvider.js @@ -192,7 +192,6 @@ const FeatureListProvider = class extends React.Component { segmentOverrides, changeRequest, commit, - mode, ) => { AppActions.editFeatureMv( projectId, @@ -224,7 +223,6 @@ const FeatureListProvider = class extends React.Component { segmentOverrides, changeRequest, commit, - mode, ) }, ) diff --git a/frontend/common/services/useFeatureVersion.ts b/frontend/common/services/useFeatureVersion.ts index 9e81a84bf60b..8321873bf90c 100644 --- a/frontend/common/services/useFeatureVersion.ts +++ b/frontend/common/services/useFeatureVersion.ts @@ -43,35 +43,43 @@ export const getFeatureStateCrud = ( if (!oldFeatureStates) { return featureStates } - if (segments?.length) { - // filter out feature states that have no changes - const segmentDiffs = getSegmentDiff( - featureStates, - oldFeatureStates, - segments, - ) - return featureStates.filter((v) => { - const diff = segmentDiffs?.diffs?.find( - (diff) => v.feature_segment?.segment === diff.segment.id, + const segmentDiffs = segments?.length + ? getSegmentDiff( + featureStates.filter((v) => !!v.feature_segment), + oldFeatureStates.filter((v) => !!v.feature_segment), + segments, ) - return !!diff?.totalChanges - }) - } else { - // return nothing if feature state isn't different - const valueDiff = getFeatureStateDiff( - featureStates[0], - oldFeatureStates[0], + : null + const featureStateDiffs = featureStates.filter((v) => { + if (!v.feature_segment) return + const diff = segmentDiffs?.diffs?.find( + (diff) => v.feature_segment?.segment === diff.segment.id, ) - if (!valueDiff.totalChanges) { - const variationDiff = getVariationDiff( - featureStates[0], - oldFeatureStates[0], - ) - if (!variationDiff.totalChanges) return [] + return !!diff?.totalChanges + }) + const newValueFeatureState = featureStates.find((v) => !v.feature_segment)! + const oldValueFeatureState = oldFeatureStates.find( + (v) => !v.feature_segment, + )! + // return nothing if feature state isn't different + const valueDiff = getFeatureStateDiff( + oldValueFeatureState, + newValueFeatureState, + ) + if (!valueDiff.totalChanges) { + const variationDiff = getVariationDiff( + oldValueFeatureState, + newValueFeatureState, + ) + if (variationDiff.totalChanges) { + featureStateDiffs.push(newValueFeatureState) } - return featureStates + } else { + featureStateDiffs.push(newValueFeatureState) } + return featureStateDiffs } + const featureStatesToCreate = featureStates.filter( (v) => !v.id && !v.toRemove, ) diff --git a/frontend/common/stores/feature-list-store.ts b/frontend/common/stores/feature-list-store.ts index 2843884ef7e8..ebc4fa9ccc92 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -12,12 +12,12 @@ import { } from 'common/services/useProjectFlag' import OrganisationStore from './organisation-store' import { - ChangeRequest, - Environment, - FeatureState, - PagedResponse, - ProjectFlag, -} from 'common/types/responses' + ChangeRequest, + Environment, + FeatureState, + PagedResponse, + ProjectFlag, TypedFeatureState, +} from 'common/types/responses'; import Utils from 'common/utils/utils' import Actions from 'common/dispatcher/action-constants' import Project from 'common/project' @@ -466,14 +466,13 @@ const controller = { segmentOverrides: any, changeRequest: Req['createChangeRequest'], commit: boolean, - mode: 'VALUE' | 'SEGMENT', ) => { store.saving() try { API.trackEvent(Constants.events.EDIT_FEATURE) const env: Environment = ProjectStore.getEnvironment(environmentId) as any // Detect differences between change request and existing feature states - const res: { data: PagedResponse } = await getFeatureStates( + const res: { data: PagedResponse } = await getFeatureStates( getStore(), { environment: environmentFlag.environment, @@ -483,19 +482,15 @@ const controller = { forceRefetch: true, }, ) - let segments = null - if (mode === 'SEGMENT') { - const res = await getSegments(getStore(), { - include_feature_specific: true, - page_size: 1000, - projectId, - }) - segments = res.data.results - } - const oldFeatureStates = res.data.results.filter((v) => { - return mode === 'VALUE' ? !v.feature_segment : !!v.feature_segment + const segmentResult = await getSegments(getStore(), { + include_feature_specific: true, + page_size: 1000, + projectId, }) + const segments = segmentResult.data.results + const oldFeatureStates = res.data.results + let feature_states_to_create: | Req['createFeatureVersion']['feature_states_to_create'] | undefined = undefined, @@ -506,23 +501,20 @@ const controller = { | Req['createFeatureVersion']['segment_ids_to_delete_overrides'] | undefined = undefined if (env.use_v2_feature_versioning) { - let featureStates - if (mode === 'SEGMENT') { - featureStates = segmentOverrides?.map((override: any, i: number) => + const featureStates = (segmentOverrides || []) + .map((override: any, i: number) => convertSegmentOverrideToFeatureState(override, i, changeRequest), ) - } else { - featureStates = [ + .concat([ Object.assign({}, environmentFlag, { enabled: flag.default_enabled, feature_state_value: flag.initial_value, live_from: flag.live_from, }), - ] - } + ]) const version = getFeatureStateCrud( - featureStates.map((v) => ({ + featureStates.map((v: FeatureState) => ({ ...v, // endpoint returns object for feature_state_value rather than the value feature_state_value: Utils.valueToFeatureState( @@ -1082,7 +1074,6 @@ store.dispatcherIndex = Dispatcher.register(store, (payload) => { action.segmentOverrides, action.changeRequest, action.commit, - action.mode, ) break case Actions.EDIT_FEATURE: diff --git a/frontend/web/components/modals/CreateFlag.js b/frontend/web/components/modals/CreateFlag.js index 982f569d8a2c..b3749d5083f7 100644 --- a/frontend/web/components/modals/CreateFlag.js +++ b/frontend/web/components/modals/CreateFlag.js @@ -877,8 +877,9 @@ const CreateFlag = class extends Component { }, ) => { const saveFeatureValue = saveFeatureWithValidation((schedule) => { - this.setState({ valueChanged: false }) if ((is4Eyes || schedule) && !identity) { + this.setState({ segmentsChanged: false, valueChanged: false }) + openModal2( schedule ? 'New Scheduled Flag Update' @@ -940,7 +941,6 @@ const CreateFlag = class extends Component { title, }, !is4Eyes, - 'VALUE', ) }, ) @@ -948,6 +948,7 @@ const CreateFlag = class extends Component { />, ) } else { + this.setState({ valueChanged: false }) this.save(editFeatureValue, isSaving) } }) @@ -962,77 +963,7 @@ const CreateFlag = class extends Component { this.setState({ segmentsChanged: false }) if ((is4Eyes || schedule) && isVersioned && !identity) { - openModal2( - this.props.changeRequest - ? 'Update Change Request' - : schedule - ? 'Schedule Segment Overrides Update' - : 'New Change Request', - { - closeModal2() - this.save( - ( - projectId, - environmentId, - flag, - projectFlag, - environmentFlag, - segmentOverrides, - ) => { - createChangeRequest( - projectId, - environmentId, - flag, - projectFlag, - environmentFlag, - segmentOverrides, - { - approvals, - description, - featureStateId: - this.props.changeRequest && - this.props.changeRequest.feature_states[0] - .id, - id: - this.props.changeRequest && - this.props.changeRequest.id, - live_from, - multivariate_options: this.props - .multivariate_options - ? this.props.multivariate_options.map( - (v) => { - const matching = - this.state.multivariate_options.find( - (m) => - m.id === - v.multivariate_feature_option, - ) - return { - ...v, - percentage_allocation: - matching.default_percentage_allocation, - } - }, - ) - : this.state.multivariate_options, - title, - }, - !is4Eyes, - 'SEGMENT', - ) - }, - ) - }} - />, - ) + return saveFeatureValue() } else { this.save(editFeatureSegments, isSaving) } @@ -1129,7 +1060,11 @@ const CreateFlag = class extends Component {
{is4Eyes - ? 'This will create a change request for the environment' + ? `This will create a change request ${ + isVersioned + ? 'with any value and segment override changes ' + : '' + }for the environment` : 'This will update the feature value for the environment'}{' '} { @@ -1412,7 +1347,11 @@ const CreateFlag = class extends Component {

{is4Eyes && isVersioned - ? 'This will create a change request for the environment' + ? `This will create a change request ${ + isVersioned + ? 'with any value and segment override changes ' + : '' + }for the environment` : 'This will update the segment overrides for the environment'}{' '} {