diff --git a/frontend/common/services/useChangeRequest.ts b/frontend/common/services/useChangeRequest.ts index 595b2816df78..9c3b9696c638 100644 --- a/frontend/common/services/useChangeRequest.ts +++ b/frontend/common/services/useChangeRequest.ts @@ -1,7 +1,14 @@ -import { Res } from 'common/types/responses' +import { + ChangeRequest, + ChangeSet, + FeatureState, + Res, +} from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' import Utils from 'common/utils/utils' +import sortBy from 'lodash/sortBy' +import moment from 'moment' export const changeRequestService = service .enhanceEndpoints({ addTagTypes: ['ChangeRequest'] }) @@ -45,3 +52,87 @@ const { data, isLoading } = useGetChangeRequestsQuery({ id: 2 }, {}) //get hook const [createChangeRequests, { isLoading, data, isSuccess }] = useCreateChangeRequestsMutation() //create hook changeRequestService.endpoints.getChangeRequests.select({id: 2})(store.getState()) //access data from any function */ +export function parseChangeSet(changeSet: ChangeSet) { + const parsedChangeSet: { + feature_states_to_update: FeatureState[] + feature_states_to_create: FeatureState[] + segment_ids_to_delete_overrides: number[] + } = { + feature_states_to_create: changeSet.feature_states_to_create, + feature_states_to_update: changeSet.feature_states_to_update, + segment_ids_to_delete_overrides: changeSet.segment_ids_to_delete_overrides, + } + + return { + ...parsedChangeSet, + feature_states_to_create: Array.isArray( + parsedChangeSet.feature_states_to_create, + ) + ? parsedChangeSet.feature_states_to_create + : [], + feature_states_to_update: Array.isArray( + parsedChangeSet.feature_states_to_update, + ) + ? parsedChangeSet.feature_states_to_update + : [], + segment_ids_to_delete_overrides: Array.isArray( + parsedChangeSet.segment_ids_to_delete_overrides, + ) + ? parsedChangeSet.segment_ids_to_delete_overrides + : [], + } +} + +export function mergeChangeSets( + changeSets: ChangeSet[] | undefined, + featureStates: FeatureState[] | undefined, + conflicts: ChangeRequest['conflicts'] | undefined, +) { + let mergedFeatureStates = (featureStates || []).concat([]) + + const safeChangeSets = changeSets || [] + safeChangeSets.forEach((changeSet) => { + const parsedChangeSet = parseChangeSet(changeSet) + const toUpsert = (parsedChangeSet.feature_states_to_create || []).concat( + parsedChangeSet.feature_states_to_update, + ) + + toUpsert.forEach((v) => { + // Remove the existing feature state if it exists + mergedFeatureStates = mergedFeatureStates.filter((mergedFeatureState) => { + return ( + mergedFeatureState.feature_segment?.segment !== + v.feature_segment?.segment + ) + }) + + // Add the new or updated feature state + mergedFeatureStates = mergedFeatureStates.concat([v]) + }) + + // Remove any to delete segment overrides + mergedFeatureStates = mergedFeatureStates.filter( + (v) => + !v.feature_segment?.segment || + !parsedChangeSet.segment_ids_to_delete_overrides?.includes( + v.feature_segment?.segment, + ), + ) + }) + + return mergedFeatureStates.map((featureState) => { + const conflict = sortBy( + conflicts, + //prioritise newly published conflicts as we show those when diffing change requests + (conflict) => -moment(conflict.published_at).valueOf(), + )?.find( + (conflict) => + (conflict.segment_id || null) === + (featureState.feature_segment?.segment || null), + ) + return { + ...featureState, + conflict: conflict, + } + }) +} diff --git a/frontend/common/services/useFeatureState.ts b/frontend/common/services/useFeatureState.ts index dfa1cbcfe115..1b52753a373f 100644 --- a/frontend/common/services/useFeatureState.ts +++ b/frontend/common/services/useFeatureState.ts @@ -4,9 +4,8 @@ import { service } from 'common/service' import Utils from 'common/utils/utils' import { getFeatureSegment } from './useFeatureSegment' import { getStore } from 'common/store' -const _data = require('../data/base/_data') export const addFeatureSegmentsToFeatureStates = async (v) => { - if (!v.feature_segment) { + if (typeof v.feature_segment !== 'number') { return v } const featureSegmentData = await getFeatureSegment(getStore(), { diff --git a/frontend/common/services/useFeatureVersion.ts b/frontend/common/services/useFeatureVersion.ts index 91fa9449d740..7f09d044218b 100644 --- a/frontend/common/services/useFeatureVersion.ts +++ b/frontend/common/services/useFeatureVersion.ts @@ -1,17 +1,82 @@ -import { FeatureState, FeatureVersion, Res } from 'common/types/responses' +import { + FeatureState, + FeatureVersion, + Res, + Segment, +} from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' import { getStore } from 'common/store' -import { - createVersionFeatureState, - getVersionFeatureState, - updateVersionFeatureState, -} from './useVersionFeatureState' -import { deleteFeatureSegment } from './useFeatureSegment' +import { getVersionFeatureState } from './useVersionFeatureState' import transformCorePaging from 'common/transformCorePaging' import Utils from 'common/utils/utils' -import { updateSegmentPriorities } from './useSegmentPriority' +import { getFeatureStateDiff, getSegmentDiff } from 'components/diff/diff-utils' + +const transformFeatureStates = (featureStates: FeatureState[]) => + featureStates?.map((v) => ({ + ...v, + feature_state_value: Utils.valueToFeatureState(v.feature_state_value), + id: undefined, + multivariate_feature_state_values: v.multivariate_feature_state_values?.map( + (v) => ({ + ...v, + id: undefined, + }), + ), + })) +export const getFeatureStateCrud = ( + featureStates: FeatureState[], + oldFeatureStates?: FeatureState[], + segments?: Segment[] | null | undefined, +) => { + const excludeNotChanged = (featureStates: FeatureState[]) => { + 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, + ) + return !!diff?.totalChanges + }) + } else { + // return nothing if feature state isn't different + const valueDiff = getFeatureStateDiff( + featureStates[0], + oldFeatureStates[0], + ) + if (!valueDiff.totalChanges) { + return [] + } + return featureStates + } + } + const featureStatesToCreate: Req['createFeatureVersion']['feature_states_to_create'] = + featureStates.filter((v) => !v.id && !v.toRemove) + const featureStatesToUpdate: Req['createFeatureVersion']['feature_states_to_update'] = + excludeNotChanged(featureStates.filter((v) => !!v.id && !v.toRemove)) + const segment_ids_to_delete_overrides: Req['createFeatureVersion']['segment_ids_to_delete_overrides'] = + featureStates + .filter((v) => !!v.id && !!v.toRemove && !!v.feature_segment) + .map((v) => v.feature_segment!.segment) + + // Step 1: Create a new feature version + const feature_states_to_create = transformFeatureStates(featureStatesToCreate) + const feature_states_to_update = transformFeatureStates(featureStatesToUpdate) + return { + feature_states_to_create, + feature_states_to_update, + segment_ids_to_delete_overrides, + } +} export const featureVersionService = service .enhanceEndpoints({ addTagTypes: ['FeatureVersion'] }) .injectEndpoints({ @@ -22,15 +87,22 @@ export const featureVersionService = service >({ invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], queryFn: async (query: Req['createAndSetFeatureVersion']) => { - // Step 1: Create a new feature version + const { + feature_states_to_create, + feature_states_to_update, + segment_ids_to_delete_overrides, + } = getFeatureStateCrud(query.featureStates) const versionRes: { data: FeatureVersion } = await createFeatureVersion(getStore(), { environmentId: query.environmentId, featureId: query.featureId, - liveFrom: query.liveFrom, + feature_states_to_create, + feature_states_to_update, + live_from: query.liveFrom, + publish_immediately: !query.skipPublish, + segment_ids_to_delete_overrides, }) - // Step 2: Get the feature states for the live version const currentFeatureStates: { data: FeatureState[] } = await getVersionFeatureState(getStore(), { environmentId: query.environmentId, @@ -38,121 +110,20 @@ export const featureVersionService = service sha: versionRes.data.uuid, }) - // Step 3: update, create or delete feature states from the new version - const res: { data: FeatureState }[] = ( - await Promise.all( - query.featureStates.map((featureState) => { - const matchingVersionState = currentFeatureStates.data.find( - (feature) => { - return ( - feature.feature_segment?.segment === - featureState.feature_segment?.segment - ) - }, - ) - // Matching feature state exists, meaning we need to either modify or delete it - if (matchingVersionState) { - //Feature state is marked as to remove, delete it from the current version - if ( - featureState.toRemove && - matchingVersionState.feature_segment - ) { - return deleteFeatureSegment(getStore(), { - id: matchingVersionState.feature_segment.id, - }) - } - //Feature state is not marked as remove, so we update it - const multivariate_feature_state_values = - featureState.multivariate_feature_state_values - ? featureState.multivariate_feature_state_values?.map( - (featureStateValue) => { - const newId = - matchingVersionState?.multivariate_feature_state_values?.find( - (v) => { - return ( - v.multivariate_feature_option === - featureStateValue.multivariate_feature_option - ) - }, - ) - - return { - ...featureStateValue, - id: newId!.id, - } - }, - ) - : [] - - return updateVersionFeatureState(getStore(), { - environmentId: query.environmentId, - featureId: matchingVersionState.feature, - featureState: { - ...featureState, - feature_segment: matchingVersionState?.feature_segment - ? { - ...(matchingVersionState.feature_segment as any), - priority: featureState.feature_segment!.priority, - } - : undefined, - id: matchingVersionState.id, - multivariate_feature_state_values, - uuid: matchingVersionState.uuid, - }, - id: matchingVersionState.id, - sha: versionRes.data.uuid, - uuid: matchingVersionState.uuid, - }) - } - // Matching feature state does not exist, meaning we need to create it - else { - return createVersionFeatureState(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, - featureState, - sha: versionRes.data.uuid, - }) - } - }), - ) - ).filter((v) => !!v?.data) - - //Step 4: Update feature segment priorities before saving feature states - const prioritiesToUpdate = query.featureStates - .filter((v) => !v.toRemove && !!v.feature_segment) - .map((v) => { - const matchingFeatureSegment = res?.find( - (currentFeatureState) => - v.feature_segment?.segment === - currentFeatureState.data.feature_segment?.segment, - ) - return { - id: matchingFeatureSegment!.data.feature_segment!.id!, - priority: v.feature_segment!.priority, - } - }) - if (prioritiesToUpdate.length) { - await updateSegmentPriorities(getStore(), prioritiesToUpdate) - } + const res = currentFeatureStates.data const ret = { error: res.find((v) => !!v.error)?.error, feature_states: res.map((item) => ({ - ...item, + data: item, version_sha: versionRes.data.uuid, })), + feature_states_to_create, + feature_states_to_update, + segment_ids_to_delete_overrides, version_sha: versionRes.data.uuid, } - // Step 5: Publish the feature version - if (!query.skipPublish) { - await publishFeatureVersion(getStore(), { - environmentId: query.environmentId, - featureId: query.featureId, - sha: versionRes.data.uuid, - }) - } - return { data: ret } as any }, }), @@ -162,7 +133,7 @@ export const featureVersionService = service >({ invalidatesTags: [{ id: 'LIST', type: 'FeatureVersion' }], query: (query: Req['createFeatureVersion']) => ({ - body: { live_from: query.liveFrom }, + body: query, method: 'POST', url: `environments/${query.environmentId}/features/${query.featureId}/versions/`, }), diff --git a/frontend/common/stores/change-requests-store.js b/frontend/common/stores/change-requests-store.js index b9c4aaf31b8d..235ecc0d1f58 100644 --- a/frontend/common/stores/change-requests-store.js +++ b/frontend/common/stores/change-requests-store.js @@ -8,23 +8,12 @@ const { const PAGE_SIZE = 20 const transformChangeRequest = async (changeRequest) => { - const res = changeRequest?.environment_feature_versions?.length - ? { - ...changeRequest, - feature_states: flatten( - changeRequest.environment_feature_versions.map( - (featureVersion) => featureVersion.feature_states, - ), - ), - } - : changeRequest - const feature_states = await Promise.all( - res.feature_states.map(addFeatureSegmentsToFeatureStates), + changeRequest.feature_states.map(addFeatureSegmentsToFeatureStates), ) return { - ...res, + ...changeRequest, feature_states, } } @@ -60,13 +49,13 @@ const controller = { .get(`${Project.api}features/workflows/change-requests/${id}/`) .then(async (apiResponse) => { const res = await transformChangeRequest(apiResponse) + const feature = + res.feature_states[0]?.feature || res.change_sets[0]?.feature return Promise.all([ data.get( - `${Project.api}environments/${environmentId}/featurestates/?feature=${res.feature_states[0].feature}`, - ), - data.get( - `${Project.api}projects/${projectId}/features/${res.feature_states[0].feature}/`, + `${Project.api}environments/${environmentId}/featurestates/?feature=${feature}`, ), + data.get(`${Project.api}projects/${projectId}/features/${feature}/`), ]).then(([environmentFlag, projectFlag]) => { store.model[id] = res store.flags[id] = { diff --git a/frontend/common/stores/feature-list-store.ts b/frontend/common/stores/feature-list-store.ts index 4e24d2a0322c..1227914103b7 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -1,10 +1,14 @@ import Constants from 'common/constants' import { getIsWidget } from 'components/pages/WidgetPage' import ProjectStore from './project-store' -import { createAndSetFeatureVersion } from 'common/services/useFeatureVersion' +import { + createAndSetFeatureVersion, + getFeatureStateCrud, +} from 'common/services/useFeatureVersion' import { updateSegmentPriorities } from 'common/services/useSegmentPriority' import { createProjectFlag, + getProjectFlag, updateProjectFlag, } from 'common/services/useProjectFlag' import OrganisationStore from './organisation-store' @@ -13,6 +17,7 @@ import { Environment, FeatureState, MultivariateOption, + PagedResponse, ProjectFlag, } from 'common/types/responses' import Utils from 'common/utils/utils' @@ -25,9 +30,10 @@ const { createSegmentOverride } = require('../services/useSegmentOverride') const { getStore } = require('../store') import flagsmith from 'flagsmith' import API from 'project/api' -import segmentOverrides from 'components/SegmentOverrides' import { Req } from 'common/types/requests' import { getVersionFeatureState } from 'common/services/useVersionFeatureState' +import { getFeatureStates } from 'common/services/useFeatureState' +import { getSegments } from 'common/services/useSegment' let createdFirstFeature = false const PAGE_SIZE = 50 function recursivePageGet(url, parentRes) { @@ -55,6 +61,7 @@ const convertSegmentOverrideToFeatureState = ( ) => { return { enabled: override.enabled, + feature: override.feature, feature_segment: { id: override.id, priority: i, @@ -458,135 +465,216 @@ const controller = { mode: 'VALUE' | 'SEGMENT', ) => { store.saving() - API.trackEvent(Constants.events.EDIT_FEATURE) - const env: Environment = ProjectStore.getEnvironment(environmentId) as any - let environment_feature_versions = [] - if (env.use_v2_feature_versioning) { - let featureStates + 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 } = await getFeatureStates( + getStore(), + { + environment: environmentFlag.environment, + feature: projectFlag.id, + }, + ) + let segments = null if (mode === 'SEGMENT') { - featureStates = segmentOverrides?.map((override: any, i: number) => - convertSegmentOverrideToFeatureState(override, i, changeRequest), - ) - } else { - featureStates = [ - Object.assign({}, environmentFlag, { - enabled: flag.default_enabled, - feature_state_value: flag.initial_value, - live_from: flag.live_from, - }), - ] + const res = await getSegments(getStore(), { + include_feature_specific: true, + page_size: 1000, + projectId, + }) + segments = res.data.results } - - const { data: version } = await createAndSetFeatureVersion(getStore(), { - environmentId: env.id, - featureId: projectFlag.id, - featureStates, - liveFrom: changeRequest.live_from, - skipPublish: true, + const oldFeatureStates = res.data.results.filter((v) => { + return mode === 'VALUE' ? !v.feature_segment : !!v.feature_segment }) - environment_feature_versions = [version.version_sha] - } - const prom = data - .get( - `${Project.api}environments/${environmentId}/featurestates/${environmentFlag.id}/`, - ) - .then(() => { - const { - approvals, - featureStateId, - multivariate_options, - ...changeRequestData - } = changeRequest - const userApprovals = approvals.filter((u) => { - const keys = Object.keys(u) - return keys.includes('user') - }) + let feature_states_to_create: + | Req['createFeatureVersion']['feature_states_to_create'] + | undefined = undefined, + feature_states_to_update: + | Req['createFeatureVersion']['feature_states_to_update'] + | undefined = undefined, + segment_ids_to_delete_overrides: + | 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) => + convertSegmentOverrideToFeatureState(override, i, changeRequest), + ) + } else { + featureStates = [ + Object.assign({}, environmentFlag, { + enabled: flag.default_enabled, + feature_state_value: flag.initial_value, + live_from: flag.live_from, + }), + ] + } - const group_assignments = approvals.filter((g) => { - const keys = Object.keys(g) - return keys.includes('group') + const version = getFeatureStateCrud( + featureStates.map((v) => ({ + ...v, + // endpoint returns object for feature_state_value rather than the value + feature_state_value: Utils.valueToFeatureState( + v.feature_state_value, + ), + })), + 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, + ) + segment_ids_to_delete_overrides = + version.segment_ids_to_delete_overrides - const req = { - approvals: userApprovals, - environment_feature_versions, - feature_states: !env.use_v2_feature_versioning - ? [ - { - enabled: flag.default_enabled, - feature: projectFlag.id, - feature_state_value: Utils.valueToFeatureState( - flag.initial_value, - ), - id: featureStateId, - live_from: - changeRequest.live_from || new Date().toISOString(), - }, - ] - : [], - group_assignments, - ...changeRequestData, + if ( + !feature_states_to_create.length && + !feature_states_to_update.length && + !segment_ids_to_delete_overrides.length + ) { + throw new Error('Change request contains no changes') } - const reqType = req.id ? 'put' : 'post' - const url = req.id - ? `${Project.api}features/workflows/change-requests/${req.id}/` - : `${Project.api}environments/${environmentId}/create-change-request/` - return data[reqType](url, req).then((v) => { - let prom = Promise.resolve() - if (multivariate_options && v.feature_states?.[0]) { - v.feature_states[0].multivariate_feature_state_values = - v.feature_states[0].multivariate_feature_state_values.map((v) => { - const matching = multivariate_options.find( - (m) => - (v.multivariate_feature_option || v.id) === - (m.multivariate_feature_option || m.id), + } + const prom = data + .get( + `${Project.api}environments/${environmentId}/featurestates/${environmentFlag.id}/`, + ) + .then(() => { + const { + approvals, + featureStateId, + multivariate_options, + ...changeRequestData + } = changeRequest + + const userApprovals = approvals.filter((u) => { + const keys = Object.keys(u) + return keys.includes('user') + }) + + const group_assignments = approvals.filter((g) => { + const keys = Object.keys(g) + return keys.includes('group') + }) + + const changeSets = + feature_states_to_create || + feature_states_to_update || + segment_ids_to_delete_overrides + ? [ + { + feature: projectFlag.id, + feature_states_to_create: + feature_states_to_create || undefined, + feature_states_to_update: + feature_states_to_update || undefined, + live_from: + changeRequest.live_from || new Date().toISOString(), + segment_ids_to_delete_overrides: + segment_ids_to_delete_overrides || undefined, + }, + ] + : undefined + const req = { + approvals: userApprovals, + change_sets: changeSets, + feature_states: !env.use_v2_feature_versioning + ? [ + { + enabled: flag.default_enabled, + feature: projectFlag.id, + feature_state_value: Utils.valueToFeatureState( + flag.initial_value, + ), + id: featureStateId, + live_from: + changeRequest.live_from || new Date().toISOString(), + }, + ] + : [], + group_assignments, + + ...changeRequestData, + } + const reqType = req.id ? 'put' : 'post' + const url = req.id + ? `${Project.api}features/workflows/change-requests/${req.id}/` + : `${Project.api}environments/${environmentId}/create-change-request/` + return data[reqType](url, req).then((v) => { + let prom = Promise.resolve() + if (multivariate_options && v.feature_states?.[0]) { + v.feature_states[0].multivariate_feature_state_values = + v.feature_states[0].multivariate_feature_state_values.map( + (v) => { + const matching = multivariate_options.find( + (m) => + (v.multivariate_feature_option || v.id) === + (m.multivariate_feature_option || m.id), + ) + return { + ...v, + percentage_allocation: matching + ? typeof matching.percentage_allocation === 'number' + ? matching.percentage_allocation + : matching.default_percentage_allocation + : v.percentage_allocation, + } + }, ) - return { + } + + prom = data + .put( + `${Project.api}features/workflows/change-requests/${v.id}/`, + { ...v, - percentage_allocation: matching - ? typeof matching.percentage_allocation === 'number' - ? matching.percentage_allocation - : matching.default_percentage_allocation - : v.percentage_allocation, + change_sets: changeSets, + }, + ) + .then((v) => { + if (commit) { + AppActions.actionChangeRequest(v.id, 'commit', () => { + AppActions.refreshFeatures(projectId, environmentId) + }) + } else { + AppActions.getChangeRequest(v.id, projectId, environmentId) } }) - } + prom.then(() => { + AppActions.getChangeRequests(environmentId, {}) + AppActions.getChangeRequests(environmentId, { committed: true }) + AppActions.getChangeRequests(environmentId, { + live_from_after: new Date().toISOString(), + }) - prom = data - .put(`${Project.api}features/workflows/change-requests/${v.id}/`, { - ...v, - }) - .then((v) => { - if (commit) { - AppActions.actionChangeRequest(v.id, 'commit', () => { - AppActions.refreshFeatures(projectId, environmentId) - }) - } else { - AppActions.getChangeRequest(v.id, projectId, environmentId) + if (featureStateId) { + AppActions.getChangeRequest( + changeRequestData.id, + projectId, + environmentId, + ) } }) - prom.then(() => { - AppActions.getChangeRequests(environmentId, {}) - AppActions.getChangeRequests(environmentId, { committed: true }) - AppActions.getChangeRequests(environmentId, { - live_from_after: new Date().toISOString(), - }) - - if (featureStateId) { - AppActions.getChangeRequest( - changeRequestData.id, - projectId, - environmentId, - ) - } }) }) - }) - Promise.all([prom]).then(() => { - store.saved({ changeRequest: true, isCreate: true }) - }) + Promise.all([prom]).then(() => { + store.saved({ changeRequest: true, isCreate: true }) + }) + } catch (e) { + API.ajaxHandler(store, e) + } }, editVersionedFeatureState: ( projectId, diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index 7fd0aab64f99..6c1dd38321ea 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -306,22 +306,17 @@ export type Req = { environmentId: number featureId: number skipPublish?: boolean - featureStates: Pick< - FeatureState, - | 'enabled' - | 'feature_segment' - | 'uuid' - | 'feature_state_value' - | 'id' - | 'toRemove' - | 'multivariate_feature_state_values' - >[] + featureStates: FeatureState[] liveFrom?: string } createFeatureVersion: { environmentId: number featureId: number - liveFrom?: string + live_from?: string + feature_states_to_create: Omit[] + feature_states_to_update: Omit[] + publish_immediately: boolean + segment_ids_to_delete_overrides: number[] } publishFeatureVersion: { sha: string diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 95e13fd623a5..15159fee182c 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -472,6 +472,14 @@ export type Role = { organisation: number } +export type ChangeSet = { + feature: number + live_from: string | null + feature_states_to_update: FeatureState[] + feature_states_to_create: FeatureState[] + segment_ids_to_delete_overrides: number[] +} + export type RolePermissionUser = { user: number role: number @@ -484,6 +492,15 @@ export type RolePermissionGroup = { id: number role_name: string } +export type FeatureConflict = { + segment_id: number | null + original_cr_id: number | null + published_at: string + is_environment_default: boolean +} +export type FeatureStateWithConflict = FeatureState & { + conflict?: FeatureConflict +} export type ChangeRequest = { id: number created_at: string @@ -501,6 +518,7 @@ export type ChangeRequest = { user: number approved_at: null | string }[] + change_sets?: ChangeSet[] is_approved: boolean is_committed: boolean group_assignments: { group: number }[] @@ -508,6 +526,8 @@ export type ChangeRequest = { uuid: string feature_states: FeatureState[] }[] + + conflicts: FeatureConflict[] } export type FeatureVersion = { created_at: string diff --git a/frontend/common/utils/utils.tsx b/frontend/common/utils/utils.tsx index 984933bc46f3..0a5f66716bf1 100644 --- a/frontend/common/utils/utils.tsx +++ b/frontend/common/utils/utils.tsx @@ -2,6 +2,7 @@ import AccountStore from 'common/stores/account-store' import ProjectStore from 'common/stores/project-store' import Project from 'common/project' import { + ChangeSet, ContentType, FeatureState, FeatureStateValue, @@ -519,7 +520,6 @@ const Utils = Object.assign({}, require('./base/_utils'), { getViewIdentitiesPermission() { return 'VIEW_IDENTITIES' }, - isMigrating() { const model = ProjectStore.model as null | ProjectType if ( @@ -530,8 +530,8 @@ const Utils = Object.assign({}, require('./base/_utils'), { } return false }, - isSaas: () => global.flagsmithVersion?.backend?.is_saas, + isValidNumber(value: any) { return /^-?\d*\.?\d+$/.test(`${value}`) }, diff --git a/frontend/e2e/helpers.cafe.ts b/frontend/e2e/helpers.cafe.ts index e10c5f0a7948..8b63eabba38c 100644 --- a/frontend/e2e/helpers.cafe.ts +++ b/frontend/e2e/helpers.cafe.ts @@ -175,8 +175,8 @@ export const addSegmentOverrideConfig = async ( await click(byId(`select-segment-option-${selectionIndex}`)) await waitForElementVisible(byId(`segment-override-value-${index}`)) - await setText(byId(`segment-override-value-${0}`), `${value}`) - await click(byId('segment-override-toggle-0')) + await setText(byId(`segment-override-value-${index}`), `${value}`) + await click(byId(`segment-override-toggle-${index}`)) } export const addSegmentOverride = async ( @@ -189,7 +189,7 @@ export const addSegmentOverride = async ( await click(byId(`select-segment-option-${selectionIndex}`)) await waitForElementVisible(byId(`segment-override-value-${index}`)) if (value) { - await click(`${byId(`segment-override-${0}`)} [role="switch"]`) + await click(`${byId(`segment-override-${index}`)} [role="switch"]`) } if (mvs) { await Promise.all( diff --git a/frontend/e2e/tests/segment-test.ts b/frontend/e2e/tests/segment-test.ts index f7c4a1f02621..2505dc86647f 100644 --- a/frontend/e2e/tests/segment-test.ts +++ b/frontend/e2e/tests/segment-test.ts @@ -168,13 +168,13 @@ export const testSegment2 = async () => { log('Set segment overrides features') await viewFeature(0) - await addSegmentOverrideConfig(0, 3, 2) - await addSegmentOverrideConfig(1, 2, 1) - await addSegmentOverrideConfig(2, 1, 0) + await addSegmentOverrideConfig(0, 1, 0) + await addSegmentOverrideConfig(1, 2, 0) + await addSegmentOverrideConfig(2, 3, 0) await saveFeatureSegments() await viewFeature(1) - await addSegmentOverride(0, true, 2) - await addSegmentOverride(1, false, 1) + await addSegmentOverride(0, true, 0) + await addSegmentOverride(1, false, 0) await addSegmentOverride(2, true, 0) await saveFeatureSegments() diff --git a/frontend/web/components/FeatureVersion.tsx b/frontend/web/components/FeatureVersion.tsx index 6004ac5016ae..6cfad590c2a2 100644 --- a/frontend/web/components/FeatureVersion.tsx +++ b/frontend/web/components/FeatureVersion.tsx @@ -64,6 +64,7 @@ const FeatureVersion: FC = ({ { + const container = document.querySelector('.tabs-content .tab-active') + if (container) { + container.scrollTop = container.scrollHeight + } + }, 0) } confirmRemove = (i) => { diff --git a/frontend/web/components/WarningMessage.tsx b/frontend/web/components/WarningMessage.tsx index 3dc4bb10b5a1..af5e560a8892 100644 --- a/frontend/web/components/WarningMessage.tsx +++ b/frontend/web/components/WarningMessage.tsx @@ -1,10 +1,10 @@ -import React, { FC } from 'react' +import React, { FC, ReactNode } from 'react' import Icon from './Icon' import Button from './base/forms/Button' import Constants from 'common/constants' type WarningMessageType = { - warningMessage: string + warningMessage: ReactNode enabledButton?: boolean warningMessageClass?: string } diff --git a/frontend/web/components/diff/DiffChangeRequest.tsx b/frontend/web/components/diff/DiffChangeRequest.tsx index 15d7f4170c61..c8e12cb58d59 100644 --- a/frontend/web/components/diff/DiffChangeRequest.tsx +++ b/frontend/web/components/diff/DiffChangeRequest.tsx @@ -1,17 +1,19 @@ -import { FC } from 'react' -import { ChangeRequest, ChangeRequestSummary } from 'common/types/responses' +import { FC, useMemo } from 'react' +import { ChangeRequest } from 'common/types/responses' import { useGetFeatureStatesQuery } from 'common/services/useFeatureState' import DiffFeature from './DiffFeature' +import { mergeChangeSets } from 'common/services/useChangeRequest' type DiffChangeRequestType = { changeRequest: ChangeRequest | null feature: number projectId: string + environmentId: string isVersioned: boolean } - const DiffChangeRequest: FC = ({ changeRequest, + environmentId, feature, isVersioned, projectId, @@ -23,6 +25,17 @@ const DiffChangeRequest: FC = ({ }, { refetchOnMountOrArgChange: true, skip: !changeRequest }, ) + + const newState = useMemo(() => { + const changeSets = changeRequest?.change_sets?.filter( + (v) => v.feature === feature, + ) + if (!changeSets?.length) { + return changeRequest?.feature_states + } + return mergeChangeSets(changeSets, data?.results, changeRequest?.conflicts) + }, [changeRequest, feature, data]) + if (!changeRequest) { return null } @@ -36,10 +49,12 @@ const DiffChangeRequest: FC = ({ } return ( ) diff --git a/frontend/web/components/diff/DiffFeature.tsx b/frontend/web/components/diff/DiffFeature.tsx index d4e0475906ff..24f132903457 100644 --- a/frontend/web/components/diff/DiffFeature.tsx +++ b/frontend/web/components/diff/DiffFeature.tsx @@ -1,5 +1,9 @@ import React, { FC, useEffect, useState } from 'react' -import { FeatureState } from 'common/types/responses' +import { + FeatureConflict, + FeatureState, + FeatureStateWithConflict, +} from 'common/types/responses' import Tabs from 'components/base/forms/Tabs' import TabItem from 'components/base/forms/TabItem' import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' @@ -14,19 +18,26 @@ import DiffEnabled from './DiffEnabled' import DiffSegments from './DiffSegments' import DiffVariations from './DiffVariations' import InfoMessage from 'components/InfoMessage' +import Icon from 'components/Icon' +import WarningMessage from 'components/WarningMessage' +import { Link } from 'react-router-dom' type FeatureDiffType = { - oldState: FeatureState[] - newState: FeatureState[] + oldState: FeatureStateWithConflict[] + newState: FeatureStateWithConflict[] noChangesMessage?: string featureId: number projectId: string + environmentId: string tabTheme?: string + conflicts?: FeatureConflict[] | undefined disableSegments?: boolean } const DiffFeature: FC = ({ + conflicts, disableSegments, + environmentId, featureId, newState, noChangesMessage, @@ -42,16 +53,18 @@ const DiffFeature: FC = ({ }) const diff = getFeatureStateDiff(oldEnv, newEnv) - const { totalChanges } = diff + const { conflict: valueConflict, totalChanges } = diff const [value, setValue] = useState(0) const { data: segments } = useGetSegmentsQuery({ + include_feature_specific: true, + page_size: 1000, projectId, }) const segmentDiffs = disableSegments ? { diffs: [], totalChanges: 0 } - : getSegmentDiff(oldState, newState, segments?.results) + : getSegmentDiff(oldState, newState, segments?.results, conflicts) const variationDiffs = getVariationDiff(oldEnv, newEnv, feature) const totalSegmentChanges = segmentDiffs?.totalChanges const totalVariationChanges = variationDiffs?.totalChanges @@ -82,8 +95,9 @@ const DiffFeature: FC = ({ > +
Value + {!!valueConflict && } {totalChanges ? ( {totalChanges} ) : null} @@ -95,6 +109,28 @@ const DiffFeature: FC = ({ No Changes Found
)} + {!!valueConflict && ( +
+ + A change request was published since the creation of + this one that also modified the value. +
+ Please review the following changes to make sure they + are correct. +
+ + View published change request + +
+
+ } + /> + + )}
@@ -150,8 +186,11 @@ const DiffFeature: FC = ({ {!!segmentDiffs?.diffs.length && ( +
Segment Overrides + {!!segmentDiffs.totalConflicts && ( + + )} {!!segmentDiffs.totalChanges && ( {segmentDiffs.totalChanges} @@ -160,7 +199,28 @@ const DiffFeature: FC = ({
} > - + {!!segmentDiffs.totalConflicts && ( +
+ + A change request was published since the creation of + this one that also modified{' '} + {segmentDiffs.totalConflicts === 1 ? 'one' : 'some'}{' '} + of the segment overrides. +
+ Please review the following changes to make sure they + are correct. +
+ } + /> +
+ )} + )} diff --git a/frontend/web/components/diff/DiffSegments.tsx b/frontend/web/components/diff/DiffSegments.tsx index 7cd098ec232c..ab6784595812 100644 --- a/frontend/web/components/diff/DiffSegments.tsx +++ b/frontend/web/components/diff/DiffSegments.tsx @@ -6,17 +6,44 @@ import { sortBy } from 'lodash' import Tabs from 'components/base/forms/Tabs' import TabItem from 'components/base/forms/TabItem' import Utils from 'common/utils/utils' +import Icon from 'components/Icon' +import Tooltip from 'components/Tooltip' +import { Link } from 'react-router-dom' type DiffSegment = { diff: TDiffSegment + projectId: string + environmentId: string } const widths = [200, 80, 105] -const DiffSegment: FC = ({ diff }) => { +const DiffSegment: FC = ({ diff, environmentId, projectId }) => { return (
-
- {diff.segment?.name} +
+
+ +
+ {!!diff.conflict && } + {diff.segment?.name} +
+ {!!diff.conflict && ( + + View Change Request + + )} + + } + > + {diff.conflict + ? 'A change request was published since the creation of this one that also modified the value for this segment.' + : null} + +
= ({ diff }) => { type DiffSegmentsType = { diffs: TDiffSegment[] | undefined + projectId: string + environmentId: string } -const DiffSegments: FC = ({ diffs }) => { +const DiffSegments: FC = ({ + diffs, + environmentId, + projectId, +}) => { const { created, deleted, modified, unChanged } = useMemo(() => { const created: TDiffSegment[] = [] const deleted: TDiffSegment[] = [] @@ -91,7 +124,12 @@ const DiffSegments: FC = ({ diffs }) => { > {tableHeader} {created.map((diff, i) => ( - + ))} )} @@ -105,7 +143,12 @@ const DiffSegments: FC = ({ diffs }) => { > {tableHeader} {deleted.map((diff, i) => ( - + ))} )} @@ -119,7 +162,12 @@ const DiffSegments: FC = ({ diffs }) => { > {tableHeader} {modified.map((diff, i) => ( - + ))} )} @@ -133,7 +181,12 @@ const DiffSegments: FC = ({ diffs }) => { > {tableHeader} {unChanged.map((diff, i) => ( - + ))} )} diff --git a/frontend/web/components/diff/diff-utils.ts b/frontend/web/components/diff/diff-utils.ts index 8bd5d07bb352..db368c6dfd7d 100644 --- a/frontend/web/components/diff/diff-utils.ts +++ b/frontend/web/components/diff/diff-utils.ts @@ -1,9 +1,15 @@ -import { FeatureState, ProjectFlag, Segment } from 'common/types/responses' +import { + FeatureConflict, + FeatureState, + FeatureStateWithConflict, + ProjectFlag, + Segment, +} from 'common/types/responses' import Utils from 'common/utils/utils' import { sortBy } from 'lodash' export function getFeatureStateDiff( oldFeatureState: FeatureState | undefined, - newFeatureState: FeatureState | undefined, + newFeatureState: FeatureStateWithConflict | undefined, ) { const oldValue = Utils.getTypedValue( Utils.featureStateToValue(oldFeatureState?.feature_state_value), @@ -16,6 +22,7 @@ export function getFeatureStateDiff( const enabledChanged = oldEnabled !== newEnabled const valueChanged = oldValue !== newValue const diff = { + conflict: newFeatureState?.conflict, enabledChanged, newEnabled, newValue, @@ -35,6 +42,7 @@ export type TDiffSegment = { oldEnabled: boolean oldPriority: number oldValue: string + conflict?: FeatureConflict totalChanges: number created: boolean deleted: boolean @@ -52,9 +60,10 @@ export type TDiffVariations = { } export const getSegmentDiff = ( - oldFeatureStates: FeatureState[] | undefined, - newFeatureStates: FeatureState[] | undefined, + oldFeatureStates: FeatureStateWithConflict[] | undefined, + newFeatureStates: FeatureStateWithConflict[] | undefined, segments: Segment[] | undefined, + conflicts?: FeatureConflict[] | undefined, ) => { if (!oldFeatureStates || !newFeatureStates || !segments) { return null @@ -69,6 +78,7 @@ export const getSegmentDiff = ( (s) => s.name, ) let totalChanges = 0 + let totalConflicts = 0 const diffs = relatedSegments?.map((segment) => { const oldFeatureState = oldFeatureStates?.find( (v) => v.feature_segment?.segment === segment.id, @@ -76,6 +86,16 @@ export const getSegmentDiff = ( const newFeatureState = newFeatureStates?.find( (v) => v.feature_segment?.segment === segment.id, ) + let foundConflict = null + if (!newFeatureState && !!oldFeatureState) { + //detect conflicts where the new change request attempts to delete a segment overrides + foundConflict = conflicts?.find( + (v) => v.segment_id === oldFeatureState.feature_segment?.segment, + ) + if (foundConflict) { + totalConflicts++ + } + } const oldEnabled = !!oldFeatureState?.enabled const oldPriority = oldFeatureState?.feature_segment @@ -100,6 +120,9 @@ export const getSegmentDiff = ( const enabledChanged = oldEnabled !== newEnabled const valueChanged = oldValue !== newValue const priorityChanged = oldPriority !== newPriority + if (newFeatureState?.conflict) { + totalConflicts++ + } const segmentChanges = (enabledChanged ? 1 : 0) + (valueChanged ? 1 : 0) + @@ -108,6 +131,7 @@ export const getSegmentDiff = ( totalChanges += 1 } return { + conflict: foundConflict || newFeatureState?.conflict, created: !oldFeatureState, deleted: !newFeatureState, enabledChanged, @@ -124,6 +148,7 @@ export const getSegmentDiff = ( return { diffs, totalChanges, + totalConflicts, } } diff --git a/frontend/web/components/modals/AssociatedSegmentOverrides.js b/frontend/web/components/modals/AssociatedSegmentOverrides.js index 4af7279608a2..1a236281bef3 100644 --- a/frontend/web/components/modals/AssociatedSegmentOverrides.js +++ b/frontend/web/components/modals/AssociatedSegmentOverrides.js @@ -13,6 +13,7 @@ import SegmentOverrideLimit from 'components/SegmentOverrideLimit' import { getStore } from 'common/store' import { getEnvironment } from 'common/services/useEnvironment' import { saveFeatureWithValidation } from 'components/saveFeatureWithValidation' +import Utils from 'common/utils/utils' class TheComponent extends Component { state = { @@ -262,6 +263,7 @@ export default class SegmentOverridesInner extends Component { const overrides = originalSegmentOverrides .filter((v) => v.segment !== segmentOverrides[0].segment) .concat([segmentOverrides[0]]) + openModal2( 'Edit Segment Override Priorities',
@@ -299,11 +301,14 @@ export default class SegmentOverridesInner extends Component { originalSegmentOverrides, projectFlag, projectId, - readOnly, segmentOverrides, updateSegments, } = this.props - + const environment = ProjectStore.getEnvironment(environmentId) + const changeRequest = Utils.changeRequestsEnabled( + environment?.minimum_change_request_approvals, + ) + const readOnly = this.props.readOnly || !!changeRequest return ( {({}, { editFeatureSegments, isSaving }) => { diff --git a/frontend/web/components/modals/CreateFlag.js b/frontend/web/components/modals/CreateFlag.js index c68d7e29fd5e..27711dd42d7b 100644 --- a/frontend/web/components/modals/CreateFlag.js +++ b/frontend/web/components/modals/CreateFlag.js @@ -766,10 +766,9 @@ const CreateFlag = class extends Component { } placeholder='E.g. header_size' /> - )} - + {identity && description && ( - this.setState({ + <> + + { - this.setState({ - segmentsChanged: true, - }) - this.props.updateSegments( - v, - ) - }} - /> + ) => + this.setState({ + showCreateSegment, + }) + } + feature={projectFlag.id} + projectId={ + this.props.projectId + } + multivariateOptions={ + multivariate_options + } + environmentId={ + this.props.environmentId + } + value={ + this.props + .segmentOverrides + } + controlValue={ + initial_value + } + onChange={(v) => { + this.setState({ + segmentsChanged: true, + }) + this.props.updateSegments( + v, + ) + }} + /> + ) }} @@ -2001,9 +2011,6 @@ const FeatureProvider = (WrappedComponent) => { projectFlag: newProjectFlag, }) } - if (changeRequest) { - closeModal() - } }, ) } diff --git a/frontend/web/components/pages/ChangeRequestPage.js b/frontend/web/components/pages/ChangeRequestPage.js index 39a45eb455c4..acbd1e0966c7 100644 --- a/frontend/web/components/pages/ChangeRequestPage.js +++ b/frontend/web/components/pages/ChangeRequestPage.js @@ -1,4 +1,4 @@ -import React, { Component } from 'react' +import React, { Component, useMemo } from 'react' import ChangeRequestStore from 'common/stores/change-requests-store' import OrganisationStore from 'common/stores/organisation-store' import FeatureListStore from 'common/stores/feature-list-store' @@ -22,6 +22,9 @@ import Breadcrumb from 'components/Breadcrumb' import SettingsButton from 'components/SettingsButton' import DiffChangeRequest from 'components/diff/DiffChangeRequest' import NewVersionWarning from 'components/NewVersionWarning' +import { mergeChangeSets } from 'common/services/useChangeRequest' +import WarningMessage from 'components/WarningMessage' +import ErrorMessage from 'components/ErrorMessage'; const ChangeRequestsPage = class extends Component { static displayName = 'ChangeRequestsPage' @@ -94,6 +97,17 @@ const ChangeRequestsPage = class extends Component { title: changeRequest.title, }) } + + componentDidUpdate(prevProps, prevState, snapshot) { + if (prevProps.match.params.id !== this.props.match.params.id) { + AppActions.getChangeRequest( + this.props.match.params.id, + this.props.match.params.projectId, + this.props.match.params.environmentId, + ) + } + } + componentDidMount = () => {} deleteChangeRequest = () => { @@ -144,14 +158,27 @@ const ChangeRequestsPage = class extends Component { 'side-modal create-feature-modal', ) } - + refreshChangeRequests = () => { + const environmentId = this.props.match.params.environmentId + AppActions.getChangeRequests(environmentId, {}) + AppActions.getChangeRequests(environmentId, { committed: true }) + AppActions.getChangeRequests(environmentId, { + live_from_after: new Date().toISOString(), + }) + } approveChangeRequest = () => { - AppActions.actionChangeRequest(this.props.match.params.id, 'approve') + AppActions.actionChangeRequest( + this.props.match.params.id, + 'approve', + this.refreshChangeRequests, + ) } getScheduledDate = (changeRequest) => { return changeRequest.environment_feature_versions.length > 0 ? moment(changeRequest.environment_feature_versions[0].live_from) + : changeRequest?.change_sets + ? changeRequest?.change_sets?.[0]?.live_from : moment(changeRequest.feature_states[0].live_from) } @@ -159,7 +186,7 @@ const ChangeRequestsPage = class extends Component { const id = this.props.match.params.id const changeRequest = ChangeRequestStore.model[id] const scheduledDate = this.getScheduledDate(changeRequest) - const isScheduled = scheduledDate > moment() + const isScheduled = moment(scheduledDate).valueOf() > moment().valueOf() const featureId = changeRequest && changeRequest.feature_states[0] && @@ -173,7 +200,7 @@ const ChangeRequestsPage = class extends Component { Are you sure you want to {isScheduled ? 'schedule' : 'publish'} this change request {isScheduled - ? ` for ${scheduledDate.format('Do MMM YYYY hh:mma')}` + ? ` for ${moment(scheduledDate).format('Do MMM YYYY hh:mma')}` : ''} ? This will adjust the feature for your environment. + {!!changeRequest?.conflicts?.length && ( +
+ + A change request was published since the creation of this + one that also modified this feature. Please review the + changes on this page to make sure they are correct. +
+ } + /> +
+ )}
), onYes: () => { @@ -193,6 +233,7 @@ const ChangeRequestsPage = class extends Component { this.props.match.params.environmentId, true, ) + this.refreshChangeRequests() }, ) }, @@ -270,7 +311,7 @@ const ChangeRequestsPage = class extends Component { {} const scheduledDate = this.getScheduledDate(changeRequest) - const isScheduled = scheduledDate > moment() + const isScheduled = moment(scheduledDate).valueOf() > moment().valueOf() const approval = changeRequest && @@ -288,7 +329,6 @@ const ChangeRequestsPage = class extends Component { const isVersioned = environment?.use_v2_feature_versioning const minApprovals = environment.minimum_change_request_approvals || 0 const isYourChangeRequest = changeRequest.user === AccountStore.getUser().id - return ( - moment()) && ( + this.getScheduledDate(changeRequest) > moment()) && (