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(flags): update payloads object when removing variants #25580

Merged
merged 12 commits into from
Oct 17, 2024
2 changes: 1 addition & 1 deletion frontend/src/scenes/feature-flags/FeatureFlag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ function UsageTab({ featureFlag }: { id: string; featureFlag: FeatureFlagType })
) {
enrichUsageDashboard()
}
}, [dashboard])
}, [dashboard, hasEnrichedAnalytics, enrichUsageDashboard])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

satisfying ESLint


const propertyFilter: AnyPropertyFilter[] = [
{
Expand Down
35 changes: 27 additions & 8 deletions frontend/src/scenes/feature-flags/featureFlagLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
FilterType,
InsightModel,
InsightType,
JsonType,
MultivariateFlagOptions,
MultivariateFlagVariant,
NewEarlyAccessFeatureType,
Expand Down Expand Up @@ -132,9 +133,11 @@ export const variantKeyToIndexFeatureFlagPayloads = (flag: FeatureFlagType): Fea
return flag
}

const newPayloads = {}
const newPayloads: Record<number, JsonType> = {}
flag.filters.multivariate?.variants.forEach((variant, index) => {
newPayloads[index] = flag.filters.payloads?.[variant.key]
if (flag.filters.payloads?.[variant.key] !== undefined) {
newPayloads[index] = flag.filters.payloads[variant.key]
}
Comment on lines +136 to +140
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appeasing the compiler

})
return {
...flag,
Expand All @@ -147,11 +150,10 @@ export const variantKeyToIndexFeatureFlagPayloads = (flag: FeatureFlagType): Fea

const indexToVariantKeyFeatureFlagPayloads = (flag: Partial<FeatureFlagType>): Partial<FeatureFlagType> => {
if (flag.filters?.multivariate) {
const newPayloads = {}
flag.filters?.multivariate?.variants.forEach(({ key }, index) => {
const payload = flag.filters?.payloads?.[index]
if (payload) {
newPayloads[key] = payload
const newPayloads: Record<string, JsonType> = {}
flag.filters.multivariate.variants.forEach(({ key }, index) => {
if (flag.filters?.payloads?.[index] !== undefined) {
newPayloads[key] = flag.filters.payloads[index]
Comment on lines +153 to +156
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appeasing the compiler

}
})
return {
Expand Down Expand Up @@ -316,6 +318,22 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
}
const variants = [...(state.filters.multivariate?.variants || [])]
variants.splice(index, 1)

const currentPayloads = { ...state.filters.payloads }
const newPayloads: Record<number, any> = {}

// TRICKY: In addition to modifying the variant array, we also need to shift the payload indices
// because the variant array is being modified and we need to make sure that the payloads object
// stays in sync with the variant array.
Object.keys(currentPayloads).forEach((key) => {
const payloadIndex = parseInt(key)
if (payloadIndex > index) {
newPayloads[payloadIndex - 1] = currentPayloads[payloadIndex]
} else if (payloadIndex < index) {
newPayloads[payloadIndex] = currentPayloads[payloadIndex]
}
})
Comment on lines +321 to +335
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the meat of the change that fixes the bug.


return {
...state,
filters: {
Expand All @@ -324,6 +342,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
...state.filters.multivariate,
variants,
},
payloads: newPayloads,
},
}
},
Expand Down Expand Up @@ -636,7 +655,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
createScheduledChange: async () => {
const { scheduledChangeOperation, scheduleDateMarker, currentTeamId, schedulePayload } = values

const fields = {
const fields: Record<ScheduledChangeOperationType, keyof ScheduleFlagPayload> = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appeasing the compiler

[ScheduledChangeOperationType.UpdateStatus]: 'active',
[ScheduledChangeOperationType.AddReleaseCondition]: 'filters',
}
Expand Down
Loading