feat: combine value + segment change requests for versioned environme…

…nts (#4738) Co-authored-by: Matthew Elwell <matthew.elwell@flagsmith.com>
Flagsmith · Dec 3, 2024 · e6b0e2f · e6b0e2f
1 parent bb7849c
commit e6b0e2f
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 131 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<FeatureState> } = await getFeatureStates(
+      const res: { data: PagedResponse<TypedFeatureState> } = 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
@@ -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,14 +941,14 @@ const CreateFlag = class extends Component {
                                 title,
                               },
                               !is4Eyes,
-                              'VALUE',
                             )
                           },
                         )
                       }}
                     />,
                   )
                 } 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',
-                      <ChangeRequestModal
-                        showAssignees={is4Eyes}
-                        changeRequest={this.props.changeRequest}
-                        onSave={({
-                          approvals,
-                          description,
-                          live_from,
-                          title,
-                        }) => {
-                          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 {
                                     <ModalHR className='mt-4' />
                                     <div className='text-right mt-4 mb-3 fs-small lh-sm modal-caption'>
                                       {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'}{' '}
                                       <strong>
                                         {
@@ -1412,7 +1347,11 @@ const CreateFlag = class extends Component {
                                             <div>
                                               <p className='text-right mt-4 fs-small lh-sm modal-caption'>
                                                 {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'}{' '}
                                                 <strong>
                                                   {