fix: diff check for versioned segment overrides and MV (#4656)

Flagsmith · Oct 8, 2024 · 8d1c22e · 8d1c22e
1 parent 8e6b241
commit 8d1c22e
Show file tree

Hide file tree

Showing 11 changed files with 503 additions and 427 deletions.
diff --git a/frontend/common/services/useFeatureVersion.ts b/frontend/common/services/useFeatureVersion.ts
diff --git 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<FeatureState>
 }
@@ -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
@@ -390,6 +390,10 @@ export type FeatureState = {
   toRemove?: boolean
 }
 
+export type TypedFeatureState = Omit<FeatureState, 'feature_state_value'> & {
+  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
@@ -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
@@ -14,6 +14,9 @@ const WarningMessage: FC<WarningMessageType> = (props) => {
   const warningMessageClassName = `alert alert-warning ${
     warningMessageClass || 'flex-1 align-items-center'
   }`
+  if(!props.warningMessage) {
+    return null
+  }
   return (
     <div
       className={warningMessageClassName}

diff --git a/frontend/web/components/diff/DiffFeature.tsx b/frontend/web/components/diff/DiffFeature.tsx
@@ -45,6 +45,8 @@ const DiffFeature: FC<FeatureDiffType> = ({
   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<FeatureDiffType> = ({
   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<FeatureDiffType> = ({
                 </div>
               }
             >
-              {!totalChanges && (
-                <div className='mb-3 text-center fw-semibold'>
-                  No Changes Found
-                </div>
-              )}
               {!!valueConflict && (
                 <div className='mt-4'>
                   <WarningMessage
@@ -182,7 +179,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
                   </div>
                 }
               >
-                <DiffVariations diffs={variationDiffs.diffs} />
+                <DiffVariations projectFlag={projectFlag} diffs={variationDiffs.diffs} />
               </TabItem>
             )}
             {!!segmentDiffs?.diffs.length && (

diff --git 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<DiffSegment> = ({ diff, environmentId, projectId }) => {
   return (
-    <div className={'flex-row list-item list-item-sm'}>
-      <div style={{ width: widths[0] }} className='table-column'>
-        <div>
-          <Tooltip
-            title={
-              <>
-                <div className='d-flex fw-semibold gap-2 align-items-center'>
-                  {!!diff.conflict && <Icon name='warning' width={16} />}
-                  {diff.segment?.name}
-                </div>
-                {!!diff.conflict && (
-                  <Link
-                    to={`/project/${projectId}/environment/${environmentId}/change-requests/${diff.conflict.original_cr_id}`}
-                  >
-                    View Change Request
-                  </Link>
-                )}
-              </>
-            }
-          >
-            {diff.conflict
-              ? 'A change request was published since the creation of this one that also modified the value for this segment.'
-              : null}
-          </Tooltip>
+    <div>
+      <div className={'flex-row list-item list-item-sm'}>
+        <div style={{ width: widths[0] }} className='table-column'>
+          <div>
+            <Tooltip
+              title={
+                <>
+                  <div className='d-flex fw-semibold gap-2 align-items-center'>
+                    {!!diff.conflict && <Icon name='warning' width={16} />}
+                    {diff.segment?.name}
+                  </div>
+                  {!!diff.conflict && (
+                    <Link
+                      to={`/project/${projectId}/environment/${environmentId}/change-requests/${diff.conflict.original_cr_id}`}
+                    >
+                      View Change Request
+                    </Link>
+                  )}
+                </>
+              }
+            >
+              {diff.conflict
+                ? 'A change request was published since the creation of this one that also modified the value for this segment.'
+                : null}
+            </Tooltip>
+          </div>
+        </div>
+        <div style={{ width: widths[1] }} className='table-column text-center'>
+          <DiffString
+            oldValue={diff.created ? diff.newPriority : diff.oldPriority}
+            newValue={diff.deleted ? diff.oldPriority : diff.newPriority}
+          />
+        </div>
+        <div className='table-column flex-1 overflow-hidden'>
+          <DiffString
+            oldValue={diff.created ? diff.newValue : diff.oldValue}
+            newValue={diff.deleted ? diff.oldValue : diff.newValue}
+          />
+        </div>
+        <div style={{ width: widths[2] }} className='table-column'>
+          <DiffEnabled
+            oldValue={diff.created ? diff.newEnabled : diff.oldEnabled}
+            newValue={diff.deleted ? diff.oldEnabled : diff.newEnabled}
+          />
         </div>
       </div>
-      <div style={{ width: widths[1] }} className='table-column text-center'>
-        <DiffString
-          oldValue={diff.created ? diff.newPriority : diff.oldPriority}
-          newValue={diff.deleted ? diff.oldPriority : diff.newPriority}
-        />
-      </div>
-      <div className='table-column flex-1 overflow-hidden'>
-        <DiffString
-          oldValue={diff.created ? diff.newValue : diff.oldValue}
-          newValue={diff.deleted ? diff.oldValue : diff.newValue}
-        />
-      </div>
-      <div style={{ width: widths[2] }} className='table-column'>
-        <DiffEnabled
-          oldValue={diff.created ? diff.newEnabled : diff.oldEnabled}
-          newValue={diff.deleted ? diff.oldEnabled : diff.newEnabled}
-        />
+      <div className='px-3'>
+        {!!diff.variationDiff?.diffs && (
+          <DiffVariations diffs={diff.variationDiff.diffs} />
+        )}
       </div>
     </div>
   )

diff --git 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<DiffVariationsType> = ({ diffs }) => {
+const DiffVariations: FC<DiffVariationsType> = ({ diffs, projectFlag }) => {
   const tableHeader = (
     <Row className='table-header mt-4'>
       <div className='table-column flex-fill'>Value</div>
@@ -21,28 +24,32 @@ const DiffVariations: FC<DiffVariationsType> = ({ diffs }) => {
   return (
     <>
       {tableHeader}
-      {diffs?.map((diff, i) => (
-        <Row key={i} className='list-item list-item-sm'>
-          <div className='table-column flex-fill'>
-            <DiffString
-              data-test={`version-variation-${i}-value`}
-              oldValue={diff.oldValue}
-              newValue={diff.newValue}
-            />
-          </div>
-          <div
-            className='table-column text-center'
-            style={{ width: widths[0] }}
-          >
-            <DiffString
-              data-test={`version-variation-${i}-weight`}
-              compareMethod={DiffMethod.WORDS}
-              oldValue={`${diff.oldWeight || 0}%`}
-              newValue={`${diff.newWeight || 0}%`}
-            />
-          </div>
-        </Row>
-      ))}
+      {diffs?.map((diff, i) => {
+          const variation = projectFlag?.multivariate_options?.find((v)=>v.id === diff.variationOption)
+          const stringValue = variation?Utils.featureStateToValue(variation): ''
+          return (
+              <Row key={i} className='list-item list-item-sm'>
+                  <div className='table-column flex-fill'>
+                      <DiffString
+                          oldValue={stringValue}
+                          newValue={stringValue}
+                          data-test={`version-variation-${i}-value`}
+                      />
+                  </div>
+                  <div
+                      className='table-column text-center'
+                      style={{ width: widths[0] }}
+                  >
+                      <DiffString
+                          data-test={`version-variation-${i}-weight`}
+                          compareMethod={DiffMethod.WORDS}
+                          oldValue={`${diff.oldWeight || 0}%`}
+                          newValue={`${diff.newWeight || 0}%`}
+                      />
+                  </div>
+              </Row>
+          )
+      })}
     </>
   )
 }