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

feat: combine value + segment change requests for versioned environments #4738

Merged
merged 9 commits into from
Dec 3, 2024
2 changes: 0 additions & 2 deletions frontend/common/dispatcher/app-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -152,7 +151,6 @@ const AppActions = Object.assign({}, require('./base/_app-actions'), {
environmentFlag,
environmentId,
flag,
mode,
projectFlag,
projectId,
segmentOverrides,
Expand Down
2 changes: 0 additions & 2 deletions frontend/common/providers/FeatureListProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ const FeatureListProvider = class extends React.Component {
segmentOverrides,
changeRequest,
commit,
mode,
) => {
AppActions.editFeatureMv(
projectId,
Expand Down Expand Up @@ -224,7 +223,6 @@ const FeatureListProvider = class extends React.Component {
segmentOverrides,
changeRequest,
commit,
mode,
)
},
)
Expand Down
56 changes: 32 additions & 24 deletions frontend/common/services/useFeatureVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
47 changes: 19 additions & 28 deletions frontend/common/stores/feature-list-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -1082,7 +1074,6 @@ store.dispatcherIndex = Dispatcher.register(store, (payload) => {
action.segmentOverrides,
action.changeRequest,
action.commit,
action.mode,
)
break
case Actions.EDIT_FEATURE:
Expand Down
84 changes: 14 additions & 70 deletions frontend/web/components/modals/CreateFlag.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -940,14 +941,14 @@ const CreateFlag = class extends Component {
title,
},
!is4Eyes,
'VALUE',
)
},
)
}}
/>,
)
} else {
this.setState({ valueChanged: false })
this.save(editFeatureValue, isSaving)
}
})
Expand All @@ -961,72 +962,7 @@ const CreateFlag = class extends Component {
this.setState({ segmentsChanged: false })

if (is4Eyes && isVersioned && !identity) {
openModal2(
this.props.changeRequest
? 'Update Change Request'
: '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)
}
Expand Down Expand Up @@ -1122,7 +1058,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>
{
Expand Down Expand Up @@ -1405,7 +1345,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>
{
Expand Down
2 changes: 1 addition & 1 deletion frontend/web/components/pages/ChangeRequestPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ const ChangeRequestsPage = class extends Component {
>
Delete
</Button>
{!isVersioned && !changeRequest?.committedAt && (
kyle-ssg marked this conversation as resolved.
Show resolved Hide resolved
{!changeRequest?.committedAt && (
<Button
onClick={() =>
this.editChangeRequest(
Expand Down
Loading