diff --git a/src/lib/db/feature-environment-store.ts b/src/lib/db/feature-environment-store.ts index 2d08914a8b..9f2754e6d6 100644 --- a/src/lib/db/feature-environment-store.ts +++ b/src/lib/db/feature-environment-store.ts @@ -490,4 +490,13 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { ); } } + + async variantExists(featureName: string): Promise { + const result = await this.db.raw( + `SELECT EXISTS (SELECT 1 FROM ${T.featureEnvs} WHERE feature_name = ? AND variants <> '[]'::jsonb) AS present`, + [featureName], + ); + const { present } = result.rows[0]; + return present; + } } diff --git a/src/lib/features/dependent-features/dependent.features.e2e.test.ts b/src/lib/features/dependent-features/dependent.features.e2e.test.ts index 2d6e085031..30decd0d82 100644 --- a/src/lib/features/dependent-features/dependent.features.e2e.test.ts +++ b/src/lib/features/dependent-features/dependent.features.e2e.test.ts @@ -26,6 +26,7 @@ beforeAll(async () => { experimental: { flags: { strictSchemaValidation: true, + enableLegacyVariants: true, }, }, }, diff --git a/src/lib/features/export-import-toggles/export-import-service.ts b/src/lib/features/export-import-toggles/export-import-service.ts index 1bc4b3c19f..eed0322525 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -457,7 +457,7 @@ export default class ExportImportService await Promise.all( featureEnvsWithVariants.map((featureEnvironment) => { return featureEnvironment.featureName - ? this.featureToggleService.saveVariantsOnEnv( + ? this.featureToggleService.legacySaveVariantsOnEnv( dto.project, featureEnvironment.featureName, dto.environment, diff --git a/src/lib/features/export-import-toggles/export-import.e2e.test.ts b/src/lib/features/export-import-toggles/export-import.e2e.test.ts index 5293285d5d..9d220a9519 100644 --- a/src/lib/features/export-import-toggles/export-import.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import.e2e.test.ts @@ -103,7 +103,7 @@ const createContext = async (context: ContextFieldSchema = defaultContext) => { }; const createVariants = async (feature: string, variants: IVariant[]) => { - await app.services.featureToggleService.saveVariantsOnEnv( + await app.services.featureToggleService.legacySaveVariantsOnEnv( DEFAULT_PROJECT, feature, DEFAULT_ENV, diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index a4aca46d09..72522e862b 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -2184,6 +2184,21 @@ class FeatureToggleService { return featureToggle; } + private async verifyLegacyVariants(featureName: string) { + const existingLegacyVariantsExist = + await this.featureEnvironmentStore.variantExists(featureName); + const enableLegacyVariants = this.flagResolver.isEnabled( + 'enableLegacyVariants', + ); + const useLegacyVariants = + existingLegacyVariantsExist || enableLegacyVariants; + if (!useLegacyVariants) { + throw new InvalidOperationError( + `Environment variants deprecated for feature: ${featureName}. Use strategy variants instead.`, + ); + } + } + async saveVariantsOnEnv( projectId: string, featureName: string, @@ -2191,6 +2206,25 @@ class FeatureToggleService { newVariants: IVariant[], auditUser: IAuditUser, oldVariants?: IVariant[], + ): Promise { + await this.verifyLegacyVariants(featureName); + return this.legacySaveVariantsOnEnv( + projectId, + featureName, + environment, + newVariants, + auditUser, + oldVariants, + ); + } + + async legacySaveVariantsOnEnv( + projectId: string, + featureName: string, + environment: string, + newVariants: IVariant[], + auditUser: IAuditUser, + oldVariants?: IVariant[], ): Promise { await variantsArraySchema.validateAsync(newVariants); const fixedVariants = this.fixVariantWeights(newVariants); diff --git a/src/lib/types/stores/feature-environment-store.ts b/src/lib/types/stores/feature-environment-store.ts index d423d2e07b..cc11d1830f 100644 --- a/src/lib/types/stores/feature-environment-store.ts +++ b/src/lib/types/stores/feature-environment-store.ts @@ -86,4 +86,6 @@ export interface IFeatureEnvironmentStore ): Promise; clonePreviousVariants(environment: string, project: string): Promise; + + variantExists(featureName: string): Promise; } diff --git a/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts new file mode 100644 index 0000000000..ffdba9c7ab --- /dev/null +++ b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts @@ -0,0 +1,114 @@ +import { + type IUnleashTest, + setupAppWithCustomConfig, +} from '../../../helpers/test-helper'; +import dbInit, { type ITestDb } from '../../../helpers/database-init'; +import getLogger from '../../../../fixtures/no-logger'; +import { WeightType } from '../../../../../lib/types/model'; + +let app: IUnleashTest; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('project_feature_variants_api_sunset', getLogger); + app = await setupAppWithCustomConfig(db.stores, { + experimental: { + flags: { + strictSchemaValidation: true, + enableLegacyVariants: false, + }, + }, + }); + await db.stores.environmentStore.create({ + name: 'development', + type: 'development', + }); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +beforeEach(async () => { + await db.stores.featureToggleStore.deleteAll(); +}); + +test('Cannot add environment variants to a new feature', async () => { + const featureName = 'feature-variants-patch'; + + await db.stores.featureToggleStore.create('default', { + name: featureName, + createdByUserId: 9999, + }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'development', + true, + ); + + const patch = [ + { + op: 'add', + path: '/0', + value: { + name: 'a', + weightType: WeightType.VARIABLE, + weight: 1000, + }, + }, + ]; + + await app.request + .patch( + `/api/admin/projects/default/features/${featureName}/environments/development/variants`, + ) + .send(patch) + .expect(403) + .expect((res) => { + expect(res.body.message).toBe( + 'Environment variants deprecated for feature: feature-variants-patch. Use strategy variants instead.', + ); + }); +}); + +test('Can add environment variants when existing ones exist for this feature', async () => { + const featureName = 'feature-variants-patch'; + + await db.stores.featureToggleStore.create('default', { + name: featureName, + createdByUserId: 9999, + }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'development', + true, + ); + await db.stores.featureToggleStore.saveVariants('default', featureName, [ + { + name: 'existing-variant', + stickiness: 'default', + weight: 1000, + weightType: WeightType.VARIABLE, + }, + ]); + + const patch = [ + { + op: 'add', + path: '/0', + value: { + name: 'a', + weightType: WeightType.VARIABLE, + weight: 1000, + }, + }, + ]; + + await app.request + .patch( + `/api/admin/projects/default/features/${featureName}/environments/development/variants`, + ) + .send(patch) + .expect(200); +}); diff --git a/src/test/e2e/api/admin/project/variants.e2e.test.ts b/src/test/e2e/api/admin/project/variants.e2e.test.ts index 5751926624..8e42f66d5d 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -16,6 +16,7 @@ beforeAll(async () => { experimental: { flags: { strictSchemaValidation: true, + enableLegacyVariants: true, }, }, }); diff --git a/src/test/fixtures/fake-feature-environment-store.ts b/src/test/fixtures/fake-feature-environment-store.ts index 82ba2de6cb..92c06633a6 100644 --- a/src/test/fixtures/fake-feature-environment-store.ts +++ b/src/test/fixtures/fake-feature-environment-store.ts @@ -245,4 +245,13 @@ export default class FakeFeatureEnvironmentStore features.includes(featureEnv.featureName), ); } + + async variantExists(featureName: string) { + return this.featureEnvironments.some( + (featureEnvironment) => + featureEnvironment.featureName === featureName && + featureEnvironment.variants && + featureEnvironment.variants.length > 0, + ); + } }