Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Oct 8, 2024
Merged
428 changes: 228 additions & 200 deletions frontend/common/services/useFeatureVersion.ts

Large diffs are not rendered by default.

21 changes: 8 additions & 13 deletions frontend/common/stores/feature-list-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()}`
}
Expand Down
6 changes: 5 additions & 1 deletion frontend/common/types/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion frontend/web/components/FeatureVersion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const FeatureVersion: FC<VersionDiffType> = ({
environmentId,
featureId: `${featureId}`,
uuid: oldUUID,
})
})``
const { data: newVersion } = useGetFeatureVersionQuery({
environmentId,
featureId: `${featureId}`,
Expand Down
3 changes: 3 additions & 0 deletions frontend/web/components/WarningMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
11 changes: 4 additions & 7 deletions frontend/web/components/diff/DiffFeature.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -182,7 +179,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
</div>
}
>
<DiffVariations diffs={variationDiffs.diffs} />
<DiffVariations projectFlag={projectFlag} diffs={variationDiffs.diffs} />
</TabItem>
)}
{!!segmentDiffs?.diffs.length && (
Expand Down
90 changes: 49 additions & 41 deletions frontend/web/components/diff/DiffSegments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>
)
Expand Down
53 changes: 30 additions & 23 deletions frontend/web/components/diff/DiffVariations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
)
})}
</>
)
}
Expand Down
Loading
Loading