diff --git a/frontend/common/constants.ts b/frontend/common/constants.ts index 404eea10c5b3..62a4ac6c2807 100644 --- a/frontend/common/constants.ts +++ b/frontend/common/constants.ts @@ -396,6 +396,7 @@ const Constants = { 'FEATURE_ID': 150, 'SEGMENT_ID': 150, 'TRAITS_ID': 150, + 'VARIANT_KEY': 255, }, }, @@ -651,6 +652,7 @@ const Constants = { 'Features can have values as well as being simply on or off, e.g. a font size for a banner or an environment variable for a server.', REMOTE_CONFIG_DESCRIPTION_VARIATION: 'Features can have values as well as being simply on or off, e.g. a font size for a banner or an environment variable for a server.
Variation values are set per project, the environment weight is per environment.', + RESERVED_VARIANT_KEY: 'control', SEGMENT_OVERRIDES_DESCRIPTION: 'Set different values for your feature based on what segments users are in. Identity overrides will take priority over any segment override.', TAGS_DESCRIPTION: diff --git a/frontend/common/providers/FeatureListProvider.js b/frontend/common/providers/FeatureListProvider.js index eebb9573e36d..76a6adc908fc 100644 --- a/frontend/common/providers/FeatureListProvider.js +++ b/frontend/common/providers/FeatureListProvider.js @@ -79,7 +79,18 @@ const FeatureListProvider = class extends React.Component { environmentFlag, segmentOverrides, ) => { - AppActions.createFlag(projectId, environmentId, flag, segmentOverrides) + AppActions.createFlag( + projectId, + environmentId, + { + ...flag, + multivariate_options: flag.multivariate_options?.map((v, i) => ({ + ...v, + key: v.key || Utils.getDefaultVariantKey(i), + })), + }, + segmentOverrides, + ) } editFeatureValue = ( @@ -94,7 +105,7 @@ const FeatureListProvider = class extends React.Component { Object.assign({}, projectFlag, { multivariate_options: flag.multivariate_options && - flag.multivariate_options.map((v) => { + flag.multivariate_options.map((v, i) => { const matchingProjectVariate = (projectFlag.multivariate_options && projectFlag.multivariate_options.find((p) => p.id === v.id)) || @@ -103,6 +114,7 @@ const FeatureListProvider = class extends React.Component { ...v, default_percentage_allocation: matchingProjectVariate.default_percentage_allocation, + key: v.key || Utils.getDefaultVariantKey(i), } }), }), @@ -192,7 +204,7 @@ const FeatureListProvider = class extends React.Component { Object.assign({}, projectFlag, flag, { multivariate_options: flag.multivariate_options && - flag.multivariate_options.map((v) => { + flag.multivariate_options.map((v, i) => { const matchingProjectVariate = (projectFlag.multivariate_options && projectFlag.multivariate_options.find((p) => p.id === v.id)) || @@ -201,6 +213,7 @@ const FeatureListProvider = class extends React.Component { ...v, default_percentage_allocation: matchingProjectVariate.default_percentage_allocation, + key: v.key || Utils.getDefaultVariantKey(i), } }), }), diff --git a/frontend/common/services/useMultivariateOption.ts b/frontend/common/services/useMultivariateOption.ts new file mode 100644 index 000000000000..03989c8dfae8 --- /dev/null +++ b/frontend/common/services/useMultivariateOption.ts @@ -0,0 +1,132 @@ +import { MultivariateOption, ProjectFlag, Res } from 'common/types/responses' +import { Req } from 'common/types/requests' +import { service } from 'common/service' + +export const multivariateOptionService = service.injectEndpoints({ + endpoints: (builder) => ({ + createMultivariateOption: builder.mutation< + Res['multivariateOption'], + Req['createMultivariateOption'] + >({ + query: (query) => ({ + body: query.body, + method: 'POST', + url: `projects/${query.project_id}/features/${query.feature_id}/mv-options/`, + }), + }), + saveMultivariateOptions: builder.mutation< + Res['saveMultivariateOptions'], + Req['saveMultivariateOptions'] + >({ + // No invalidatesTags: every save chain already ends with a broad + // invalidateTags(['ProjectFlag', 'FeatureList']) once the downstream + // feature-state save completes — invalidating here too would refetch + // every subscribed query twice per save. + queryFn: async (args, _, _2, baseQuery) => { + const featureUrl = `projects/${args.project_id}/features/${args.feature_id}/` + // Diff against the server's current options rather than any client + // cache, so stale state can never turn an update into a duplicate + // create. + const flagRes = await baseQuery({ method: 'GET', url: featureUrl }) + if (flagRes.error) { + return { error: flagRes.error } + } + const serverOptions = + (flagRes.data as ProjectFlag)?.multivariate_options || [] + const errors: Record = {} + // Results are written back by input index — downstream feature + // state saves map weights to option ids positionally. Requests run + // sequentially so newly created options get ascending ids in input + // order, which is the order the UI displays. + const ordered: MultivariateOption[] = [] + for (let i = 0; i < args.multivariate_options.length; i++) { + const v = args.multivariate_options[i] + let original + if (v.id) { + original = serverOptions.find((m) => m.id === v.id) + } else if (v.key) { + original = serverOptions.find((m) => !!m.key && m.key === v.key) + } + const body = { + ...v, + default_percentage_allocation: 0, + feature: args.feature_id, + } + const res = await baseQuery( + original + ? { + body, + method: 'PUT', + url: `${featureUrl}mv-options/${original.id}/`, + } + : { + body, + method: 'POST', + url: `${featureUrl}mv-options/`, + }, + ) + if (res.error) { + errors[i] = (res.error as { data?: any })?.data ?? null + } else { + ordered[i] = res.data as MultivariateOption + } + } + if (Object.keys(errors).length) { + return { data: { errors, multivariate_options: ordered } } + } + const deleted = serverOptions.filter( + (m) => !ordered.find((o) => o?.id === m.id), + ) + const deleteResults = await Promise.all( + deleted.map((m) => + baseQuery({ + method: 'DELETE', + url: `${featureUrl}mv-options/${m.id}/`, + }), + ), + ) + const failedDelete = deleteResults.find((r) => r.error) + if (failedDelete) { + return { error: failedDelete.error } + } + return { data: { errors: null, multivariate_options: ordered } } + }, + }), + // END OF ENDPOINTS + }), +}) + +export async function createMultivariateOption( + store: any, + data: Req['createMultivariateOption'], + options?: Parameters< + typeof multivariateOptionService.endpoints.createMultivariateOption.initiate + >[1], +) { + return store.dispatch( + multivariateOptionService.endpoints.createMultivariateOption.initiate( + data, + options, + ), + ) +} +export async function saveMultivariateOptions( + store: any, + data: Req['saveMultivariateOptions'], + options?: Parameters< + typeof multivariateOptionService.endpoints.saveMultivariateOptions.initiate + >[1], +) { + return store.dispatch( + multivariateOptionService.endpoints.saveMultivariateOptions.initiate( + data, + options, + ), + ) +} + +export const { + useCreateMultivariateOptionMutation, + useSaveMultivariateOptionsMutation, + // END OF EXPORTS +} = multivariateOptionService diff --git a/frontend/common/services/useProjectFlag.ts b/frontend/common/services/useProjectFlag.ts index c481451783ce..40e93ad4c780 100644 --- a/frontend/common/services/useProjectFlag.ts +++ b/frontend/common/services/useProjectFlag.ts @@ -2,6 +2,7 @@ import { PagedResponse, ProjectFlag, Res } from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' import Utils from 'common/utils/utils' +import { sortMultivariateOptions } from 'common/utils/multivariate' /** * Number of features to display per page in the features list. @@ -122,6 +123,12 @@ export const projectFlagService = service pageSize: arg.page_size || FEATURES_PAGE_SIZE, previous: response.previous, }, + results: response.results.map((feature) => ({ + ...feature, + multivariate_options: sortMultivariateOptions( + feature.multivariate_options, + ), + })), }), }), @@ -130,6 +137,12 @@ export const projectFlagService = service query: (query: Req['getProjectFlag']) => ({ url: `projects/${query.project}/features/${query.id}/`, }), + transformResponse: (res: Res['projectFlag']) => ({ + ...res, + multivariate_options: sortMultivariateOptions( + res.multivariate_options, + ), + }), }), getProjectFlags: builder.query< diff --git a/frontend/common/stores/feature-list-store.ts b/frontend/common/stores/feature-list-store.ts index 4f3c8c11e9e2..56ae9b431d5f 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -21,11 +21,13 @@ import { ChangeRequest, Environment, FeatureState, + MultivariateOption, PagedResponse, ProjectFlag, TypedFeatureState, } from 'common/types/responses' import Utils from 'common/utils/utils' +import { sortMultivariateOptions } from 'common/utils/multivariate' import Actions from 'common/dispatcher/action-constants' import Project from 'common/project' import flagsmith from '@flagsmith/flagsmith' @@ -43,6 +45,10 @@ import { FEATURES_PAGE_SIZE } from 'common/services/useProjectFlag' import Dispatcher from 'common/dispatcher/dispatcher' import BaseStore from './base/_store' import data from 'common/data/base/_data' +import { + createMultivariateOption, + saveMultivariateOptions, +} from 'common/services/useMultivariateOption' import { createSegmentOverride } from 'common/services/useSegmentOverride' import { getStore } from 'common/store' let createdFirstFeature = false @@ -113,26 +119,27 @@ const controller = { }), project_id: projectId, }) - .then((res) => { + .then(async (res) => { if (res.error) { throw res.error?.error || res.error } - return Promise.all( - (flag.multivariate_options || []).map((v) => - data - .post( - `${Project.api}projects/${projectId}/features/${res.data.id}/mv-options/`, - { - ...v, - feature: res.data.id, - }, - ) - .then(() => res.data), - ), - ).then(() => - data.get( - `${Project.api}projects/${projectId}/features/${res.data.id}/`, - ), + // Sequential so options get ascending ids in input order, which is + // the order the UI displays. + for (const v of flag.multivariate_options || []) { + const mvRes = await createMultivariateOption(getStore(), { + body: { + ...v, + feature: res.data.id, + }, + feature_id: res.data.id, + project_id: projectId, + }) + if (mvRes.error) { + throw mvRes.error + } + } + return data.get( + `${Project.api}projects/${projectId}/features/${res.data.id}/`, ) }) .then(() => @@ -150,7 +157,7 @@ const controller = { feature: v.id, })) store.model = { - features: features.results, + features: features.results.map(controller.parseFlag), keyedEnvironmentFeatures: environmentFeatures && keyBy(environmentFeatures, 'feature'), } @@ -243,53 +250,45 @@ const controller = { }) return } + store.error = null const originalFlag = store.model && store.model.features ? store.model.features.find((v) => v.id === flag.id) : flag - Promise.all( - (flag.multivariate_options || []).map((v, i) => { - const originalMV = - v.id && originalFlag?.multivariate_options - ? originalFlag.multivariate_options.find((m) => m.id === v.id) - : null - const url = `${Project.api}projects/${projectId}/features/${flag.id}/mv-options/` - const mvData = { - ...v, - default_percentage_allocation: 0, - feature: flag.id, - } - return ( - originalMV - ? data.put(`${url}${originalMV.id}/`, mvData) - : data.post(url, mvData) - ).then((res) => { - // It's important to preserve the original order of multivariate_options, so that editing feature states can use the updated ID - flag.multivariate_options[i] = res - return { - ...v, - id: res.id, - } - }) - }), - ) - .then(() => { - const deletedMv = (originalFlag?.multivariate_options || []).filter( - (v) => !flag.multivariate_options.find((x) => v.id === x.id), - ) - return Promise.all( - deletedMv.map((v) => - data.delete( - `${Project.api}projects/${projectId}/features/${flag.id}/mv-options/${v.id}/`, - ), - ), - ) - }) - .then(() => { - if (onComplete) { - onComplete(flag) - } - }) + // Standard flags carry no multivariate data — skip the round-trip. + if ( + !flag.multivariate_options?.length && + !originalFlag?.multivariate_options?.length + ) { + if (onComplete) { + onComplete(flag) + } + return + } + saveMultivariateOptions(getStore(), { + feature_id: flag.id, + multivariate_options: flag.multivariate_options || [], + project_id: projectId, + }).then((res) => { + if (res.error) { + API.ajaxHandler(store, res.error) + return + } + if (res.data.errors) { + store.error = { multivariate_options: res.data.errors } as any + store.goneABitWest() + return + } + // It's important to preserve the original order of multivariate_options, so that editing feature states can use the updated ID + res.data.multivariate_options.forEach( + (v: MultivariateOption, i: number) => { + flag.multivariate_options[i] = v + }, + ) + if (onComplete) { + onComplete(flag) + } + }) }, editFeatureState: async ( projectId, @@ -852,6 +851,11 @@ const controller = { projectId, }).then((version) => { if (version.error) { + // Multivariate options are saved separately at the project + // level, so an unchanged environment state is not a failure. + if (version.error.message === 'Feature contains no changes') { + return + } throw version.error } const featureState = version.data.feature_states[0].data @@ -986,6 +990,9 @@ const controller = { ...fs, segment: fs.segment.id, })), + multivariate_options: + flag.multivariate_options && + sortMultivariateOptions(flag.multivariate_options), } }, searchFeatures: throttle( diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index eb67f66c6095..5fc1d3e3b228 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -1039,5 +1039,15 @@ export type Req = { body: Req['createMetric']['body'] } deleteMetric: { environmentId: string; metricId: number } + createMultivariateOption: { + project_id: string | number + feature_id: number + body: Partial & { feature: number } + } + saveMultivariateOptions: { + project_id: string | number + feature_id: number + multivariate_options: Partial[] + } // END OF TYPES } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 73b6c5607024..5cc4e39ccaf2 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -573,6 +573,9 @@ export type MultivariateOption = { string_value: string boolean_value?: boolean default_percentage_allocation: number + // A stable, human-readable identifier for the variant (the backend `key`). + // Surfaced in the UI as the variation "Label". Slug-constrained and nullable. + key?: string | null } export type FeatureType = 'STANDARD' | 'MULTIVARIATE' @@ -1410,5 +1413,12 @@ export type Res = { experiment: Experiment metric: Metric metrics: PagedResponse + multivariateOption: MultivariateOption + saveMultivariateOptions: { + multivariate_options: MultivariateOption[] + // Per-option API errors keyed by the input option's index; null when all + // requests succeeded. + errors: Record | null + } // END OF TYPES } diff --git a/frontend/common/utils/__tests__/multivariate.test.ts b/frontend/common/utils/__tests__/multivariate.test.ts new file mode 100644 index 000000000000..270954873854 --- /dev/null +++ b/frontend/common/utils/__tests__/multivariate.test.ts @@ -0,0 +1,54 @@ +import { + getDefaultVariantKey, + sortMultivariateOptions, +} from 'common/utils/multivariate' + +describe('multivariate', () => { + describe('getDefaultVariantKey', () => { + it.each` + index | expected + ${0} | ${'Variant_1'} + ${1} | ${'Variant_2'} + ${9} | ${'Variant_10'} + `( + 'getDefaultVariantKey($index) returns $expected', + ({ expected, index }) => { + expect(getDefaultVariantKey(index)).toBe(expected) + }, + ) + }) + + describe('sortMultivariateOptions', () => { + it('sorts options by id ascending', () => { + const options = [{ id: 3 }, { id: 1 }, { id: 2 }] + + expect(sortMultivariateOptions(options)).toEqual([ + { id: 1 }, + { id: 2 }, + { id: 3 }, + ]) + }) + + it('sorts unsaved options last, preserving their input order', () => { + const options = [ + { id: undefined, value: 'new_a' }, + { id: 2, value: 'saved' }, + { id: null, value: 'new_b' }, + ] + + expect(sortMultivariateOptions(options)).toEqual([ + { id: 2, value: 'saved' }, + { id: undefined, value: 'new_a' }, + { id: null, value: 'new_b' }, + ]) + }) + + it('does not mutate the input array', () => { + const options = [{ id: 2 }, { id: 1 }] + + sortMultivariateOptions(options) + + expect(options).toEqual([{ id: 2 }, { id: 1 }]) + }) + }) +}) diff --git a/frontend/common/utils/multivariate.ts b/frontend/common/utils/multivariate.ts new file mode 100644 index 000000000000..2dbf7d0e1540 --- /dev/null +++ b/frontend/common/utils/multivariate.ts @@ -0,0 +1,15 @@ +// The label a variant displays (and is saved with) when the user never +// sets one — keep display, validation and save payloads consistent. +// Kept outside Utils so Storybook-rendered components can use it without +// pulling in Utils' store dependencies (Storybook stubs out Utils). +export const getDefaultVariantKey = (index: number): string => + `Variant_${index + 1}` + +// Options not yet saved have no id and sort last, in input order. +export const sortMultivariateOptions = ( + options: T[], +): T[] => + [...options].sort( + (a, b) => + (a.id ?? Number.MAX_SAFE_INTEGER) - (b.id ?? Number.MAX_SAFE_INTEGER), + ) diff --git a/frontend/common/utils/utils.tsx b/frontend/common/utils/utils.tsx index db241e4457f6..d3b869e947ca 100644 --- a/frontend/common/utils/utils.tsx +++ b/frontend/common/utils/utils.tsx @@ -21,6 +21,7 @@ import find from 'lodash/find' import ErrorMessage from 'components/ErrorMessage' import WarningMessage from 'components/WarningMessage' import Constants from 'common/constants' +import { getDefaultVariantKey } from './multivariate' import { defaultFlags } from 'common/stores/default-flags' import Color from 'color' import { selectBuildVersion } from 'common/services/useBuildVersion' @@ -92,7 +93,8 @@ const Utils = Object.assign({}, BaseUtils, { } else if (typeof v.default_percentage_allocation === 'number') { total += v.default_percentage_allocation } else { - total += (v as any).percentage_allocation + // A cleared weight input leaves the allocation null — treat as 0. + total += (v as any).percentage_allocation || 0 } return null }) @@ -255,6 +257,7 @@ const Utils = Object.assign({}, BaseUtils, { OrganisationPermission.CREATE_PROJECT ] }, + getDefaultVariantKey, getExistingWaitForTime: ( waitFor: string | undefined, ): diff --git a/frontend/documentation/components/Input.stories.tsx b/frontend/documentation/components/Input.stories.tsx new file mode 100644 index 000000000000..6a030aca7e8d --- /dev/null +++ b/frontend/documentation/components/Input.stories.tsx @@ -0,0 +1,76 @@ +import React, { useState } from 'react' +import type { Meta, StoryObj } from 'storybook' + +import Input from 'components/base/forms/Input' + +const meta: Meta = { + parameters: { layout: 'centered' }, + title: 'Components/Forms/Input', +} +export default meta + +type Story = StoryObj + +const Interactive = (props: Record) => { + const [value, setValue] = useState(props.initialValue ?? '') + return ( + ) => + setValue(e.target.value) + } + /> + ) +} + +export const Default: Story = { + render: () => , +} + +export const Sizes: Story = { + render: () => ( +
+ + + + +
+ ), +} + +export const Search: Story = { + render: () => , +} + +export const Password: Story = { + render: () => ( + + ), +} + +// Borderless input with a bottom underline only — used for inline edits +// such as the variant label in the feature modal. +export const Underline: Story = { + render: () => ( + + ), +} + +// Underline combined with centered, as used for the variant weight input. +export const UnderlineCentered: Story = { + render: () => ( +
+
+ +
+ % +
+ ), +} diff --git a/frontend/e2e/helpers/e2e-helpers.playwright.ts b/frontend/e2e/helpers/e2e-helpers.playwright.ts index 543efe790b67..ad74b4f56157 100644 --- a/frontend/e2e/helpers/e2e-helpers.playwright.ts +++ b/frontend/e2e/helpers/e2e-helpers.playwright.ts @@ -626,6 +626,32 @@ export class E2EHelpers { await this.waitForElementNotExist('#create-feature-modal'); } + // Edit a variant's label (the multivariate option key) and verify it persists + async editVariantLabel(featureName: string, index: number, label: string) { + await this.gotoFeatures(); + const featureRow = this.page.locator('[data-test^="feature-item-"]').filter({ + has: this.page.locator(`span:text-is("${featureName}")`) + }).first(); + await featureRow.waitFor({ state: 'visible', timeout: LONG_TIMEOUT }); + await featureRow.dispatchEvent('click'); + await this.waitForElementVisible(byId('update-feature-btn')); + await this.click(byId(`featureVariationKeyEdit${index}`)); + await this.setText(byId(`featureVariationKeyInput${index}`), label); + await this.click(byId(`featureVariationKeySave${index}`)); + await expect(this.page.locator(byId(`featureVariationKey${index}`))).toHaveText(label); + await this.waitForToastsToClear(); + await this.click(byId('update-feature-btn')); + await this.waitForToast(); + await this.closeModal(); + await this.waitForElementNotExist('#create-feature-modal'); + // Reopen the feature and verify the label was saved + await featureRow.dispatchEvent('click'); + await this.waitForElementVisible(byId('update-feature-btn')); + await expect(this.page.locator(byId(`featureVariationKey${index}`))).toHaveText(label); + await this.closeModal(); + await this.waitForElementNotExist('#create-feature-modal'); + } + // Create an environment async createEnvironment(name: string) { await this.page.waitForLoadState('networkidle'); diff --git a/frontend/e2e/tests/flag-tests.pw.ts b/frontend/e2e/tests/flag-tests.pw.ts index 805d103680ca..c349137c0fb3 100644 --- a/frontend/e2e/tests/flag-tests.pw.ts +++ b/frontend/e2e/tests/flag-tests.pw.ts @@ -10,6 +10,7 @@ test.describe('Flag Tests', () => { createRemoteConfig, deleteFeature, editRemoteConfig, + editVariantLabel, gotoFeatures, gotoProject, login, @@ -71,6 +72,9 @@ test.describe('Flag Tests', () => { expect(json.header_size.value).toBe('big') expect(json.mv_flag.value).toBe('big') + log('Edit variant label') + await editVariantLabel('mv_flag', 0, 'variant_medium') + log('Update feature') await editRemoteConfig('header_size', 12) diff --git a/frontend/e2e/tests/mv-options-tests.pw.ts b/frontend/e2e/tests/mv-options-tests.pw.ts new file mode 100644 index 000000000000..6cd540120f9f --- /dev/null +++ b/frontend/e2e/tests/mv-options-tests.pw.ts @@ -0,0 +1,110 @@ +import { test, expect } from '../test-setup'; +import { byId, log, createHelpers, LONG_TIMEOUT } from '../helpers'; +import { E2E_USER, PASSWORD, E2E_TEST_PROJECT } from '../config'; +import type { Page } from '@playwright/test'; + +// Regression tests for the multivariate option save flow: variants must +// never duplicate on repeated saves and deletes must apply. The v2 +// feature versioning coverage lives in versioning-tests.pw.ts, which +// owns the (irreversible) versioned environment setup. + +const openFeature = async (page: Page, name: string) => { + const featureRow = page.locator('[data-test^="feature-item-"]').filter({ + has: page.locator(`span:text-is("${name}")`), + }).first(); + await featureRow.waitFor({ state: 'visible', timeout: LONG_TIMEOUT }); + await featureRow.dispatchEvent('click'); + await page.locator(byId('update-feature-btn')).first().waitFor({ state: 'visible', timeout: LONG_TIMEOUT }); +}; + +const variantCards = (page: Page) => page.locator('#create-feature-modal .variant-card'); + +test.describe('Multivariate Options', () => { + test('Repeated saves keep the variant set stable @oss', async ({ page }) => { + const { + closeModal, + createRemoteConfig, + editRemoteConfig, + editVariantLabel, + gotoFeatures, + gotoProject, + login, + waitForElementNotExist, + } = createHelpers(page); + + log('Login'); + await login(E2E_USER, PASSWORD); + await gotoProject(E2E_TEST_PROJECT); + + log('Create multivariate flag'); + await createRemoteConfig({ name: 'mv_repeat_save', value: 'ctrl_value', mvs: [ + { value: 'va', weight: 0 }, + { value: 'vb', weight: 0 }, + ]}); + + log('First save: label-only edit'); + await editVariantLabel('mv_repeat_save', 0, 'first_variant'); + + log('Second save: value and weights, without structural changes'); + await editRemoteConfig('mv_repeat_save', 'ctrl_value2', false, [ + { value: 'va', weight: 30 }, + { value: 'vb', weight: 20 }, + ]); + + log('Variant set is unchanged after both saves'); + await gotoFeatures(); + await openFeature(page, 'mv_repeat_save'); + await expect(variantCards(page)).toHaveCount(2); + await expect(page.locator(byId('featureVariationKey0'))).toHaveText('first_variant'); + await closeModal(); + await waitForElementNotExist('#create-feature-modal'); + }); + + test('Variants can be added and removed in a single save @oss', async ({ page }) => { + const { + click, + closeModal, + createRemoteConfig, + gotoFeatures, + gotoProject, + login, + setText, + waitForElementNotExist, + waitForToast, + } = createHelpers(page); + + log('Login'); + await login(E2E_USER, PASSWORD); + await gotoProject(E2E_TEST_PROJECT); + + log('Create multivariate flag'); + await createRemoteConfig({ name: 'mv_add_remove', value: 'root_val', mvs: [ + { value: 'keep', weight: 0 }, + { value: 'drop', weight: 0 }, + ]}); + + log('Remove one variant and add another in the same edit'); + await openFeature(page, 'mv_add_remove'); + await page.locator('#create-feature-modal #delete-multivariate').nth(1).click(); + await click('#confirm-btn-yes'); + await expect(variantCards(page)).toHaveCount(1); + await click(byId('add-variation')); + await page.waitForTimeout(200); + await setText(byId('featureVariationValue1'), 'added'); + await page.waitForTimeout(500); + await click(byId('update-feature-btn')); + await waitForToast(); + await closeModal(); + await waitForElementNotExist('#create-feature-modal'); + + log('Saved set is exactly the kept and added variants'); + await gotoFeatures(); + await openFeature(page, 'mv_add_remove'); + await expect(variantCards(page)).toHaveCount(2); + await expect(page.locator(byId('featureVariationWeightkeep'))).toBeVisible(); + await expect(page.locator(byId('featureVariationWeightadded'))).toBeVisible(); + await expect(page.locator(byId('featureVariationWeightdrop'))).toHaveCount(0); + await closeModal(); + await waitForElementNotExist('#create-feature-modal'); + }); +}); diff --git a/frontend/e2e/tests/versioning-tests.pw.ts b/frontend/e2e/tests/versioning-tests.pw.ts index dbf2248b894e..f512c5c5553d 100644 --- a/frontend/e2e/tests/versioning-tests.pw.ts +++ b/frontend/e2e/tests/versioning-tests.pw.ts @@ -12,16 +12,22 @@ test('Versioning tests - Create, edit, and compare feature versions @oss', async const { assertNumberOfVersions, click, + closeModal, compareVersion, createFeature, createOrganisationAndProject, createRemoteConfig, editRemoteConfig, + gotoFeature, + gotoFeatures, login, + setText, toggleFeature, tryItExpect, + waitForElementNotExist, waitForElementVisible, waitForFeatureSwitch, + waitForToast, waitForToastsToClear, } = createHelpers(page) const flagsmith = await getFlagsmith() @@ -79,6 +85,38 @@ test('Versioning tests - Create, edit, and compare feature versions @oss', async await compareVersion('b', 0, null, true, true, 'small', 'small') await compareVersion('c', 0, null, false, true, null, null) + // =================================================================================== + // Test: Multivariate option edits in a versioned environment + // A label edit plus a structural change (new variant) in a single save must + // survive a reopen — the versioned save consumes option ids positionally + // from the multivariate save, so this pins that contract under v2. + // =================================================================================== + log('Edit variant label and add a variant in one save (versioned env)') + await gotoFeatures() + await gotoFeature('b') + await click(byId('featureVariationKeyEdit0')) + await setText(byId('featureVariationKeyInput0'), 'primary') + await click(byId('featureVariationKeySave0')) + await expect(page.locator(byId('featureVariationKey0'))).toHaveText('primary') + await click(byId('add-variation')) + await page.waitForTimeout(200) + await setText(byId('featureVariationValue2'), 'huge') + await page.waitForTimeout(500) + await click(byId('update-feature-btn')) + await waitForToast() + await closeModal() + await waitForElementNotExist('#create-feature-modal') + + log('Label and new variant survived the versioned save') + await gotoFeatures() + await gotoFeature('b') + await expect(page.locator('#create-feature-modal .variant-card')).toHaveCount(3) + await expect(page.locator(byId('featureVariationKey0'))).toHaveText('primary') + await expect(page.locator(byId('featureVariationWeightbig'))).toHaveValue('100') + await expect(page.locator(byId('featureVariationWeighthuge'))).toBeVisible() + await closeModal() + await waitForElementNotExist('#create-feature-modal') + // =================================================================================== // Test: Row toggle in versioned environment // This tests that toggling a feature via the row switch works when Feature Versioning diff --git a/frontend/web/components/base/forms/Input.js b/frontend/web/components/base/forms/Input.js index 5042ec2c48b7..f00c39f82520 100644 --- a/frontend/web/components/base/forms/Input.js +++ b/frontend/web/components/base/forms/Input.js @@ -75,6 +75,7 @@ const Input = class extends React.Component { render() { const { + centered, disabled, inputClassName, isValid, @@ -82,6 +83,7 @@ const Input = class extends React.Component { placeholderChar, showSuccess, size, + underline, ...rest } = this.props @@ -91,6 +93,7 @@ const Input = class extends React.Component { { 'focused': this.state.isFocused, 'input-container': true, + 'input-underline': underline, invalid, 'password': this.props.type === 'password', 'search': this.props.search, @@ -101,7 +104,8 @@ const Input = class extends React.Component { const innerClassName = cn( { - input: true, + 'input': true, + 'text-center': centered, }, inputClassName, sizeClassNames[size], @@ -215,6 +219,7 @@ Input.defaultProps = { Input.propTypes = { autocomplete: propTypes.string, + centered: propTypes.bool, className: propTypes.any, inputClassName: OptionalString, isValid: propTypes.any, @@ -226,6 +231,7 @@ Input.propTypes = { placeholderChar: OptionalString, search: propTypes.Boolean, size: OptionalString, + underline: propTypes.bool, } export default Input diff --git a/frontend/web/components/base/forms/InputGroup.js b/frontend/web/components/base/forms/InputGroup.js index 5d53cd1f703e..564a30f5db6c 100644 --- a/frontend/web/components/base/forms/InputGroup.js +++ b/frontend/web/components/base/forms/InputGroup.js @@ -29,7 +29,10 @@ const InputGroup = class extends Component { {this.props.tooltip ? ( +