From 5bd32f264d6b40a261a8389c31cb3a3cd7cb7e43 Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Fri, 28 Jun 2024 11:18:44 +0200 Subject: [PATCH] feat: strategy limit to 30 (#7473) --- .../FeatureStrategyMenu.tsx | 9 +- .../FeatureOverviewEnvironment.test.tsx | 97 +++++++++++++++++++ .../FeatureOverviewEnvironment.tsx | 16 +++ .../createExportImportService.ts | 2 +- .../createFeatureToggleService.ts | 6 +- .../feature-toggle/feature-toggle-service.ts | 26 +++++ .../feature-toggle-service.limit.test.ts | 41 ++++++++ .../frontend-api/createFrontendApiService.ts | 3 +- .../features/project/createProjectService.ts | 2 +- 9 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.test.tsx create mode 100644 src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx index a961ef03bb16..6bc5083bbb7d 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyMenu/FeatureStrategyMenu.tsx @@ -19,6 +19,7 @@ interface IFeatureStrategyMenuProps { variant?: IPermissionButtonProps['variant']; matchWidth?: boolean; size?: IPermissionButtonProps['size']; + disableReason?: string; } const StyledStrategyMenu = styled('div')({ @@ -43,6 +44,7 @@ export const FeatureStrategyMenu = ({ variant, size, matchWidth, + disableReason, }: IFeatureStrategyMenuProps) => { const [anchor, setAnchor] = useState(); const navigate = useNavigate(); @@ -86,6 +88,10 @@ export const FeatureStrategyMenu = ({ variant={variant} size={size} sx={{ minWidth: matchWidth ? '282px' : 'auto' }} + disabled={Boolean(disableReason)} + tooltipProps={{ + title: disableReason ? disableReason : undefined, + }} > {label} @@ -99,8 +105,9 @@ export const FeatureStrategyMenu = ({ variant='outlined' size={size} hideLockIcon + disabled={Boolean(disableReason)} tooltipProps={{ - title: 'More strategies', + title: disableReason ? disableReason : 'More strategies', }} > { + testServerRoute(server, '/api/admin/ui-config', { + flags: { + resourceLimits: true, + }, + }); + + testServerRoute( + server, + '/api/admin/projects/default/features/featureWithoutStrategies', + { + environments: [environmentWithoutStrategies], + }, + ); + + testServerRoute( + server, + '/api/admin/projects/default/features/featureWithManyStrategies', + { + environments: [environmentWithManyStrategies], + }, + ); +}; + +const strategy = { + name: 'default', +} as IFeatureStrategy; +const environmentWithoutStrategies = { + name: 'production', + enabled: true, + type: 'production', + strategies: [], +}; +const environmentWithManyStrategies = { + name: 'production', + enabled: true, + type: 'production', + strategies: [...Array(30).keys()].map(() => strategy), +}; + +test('should allow to add strategy when no strategies', async () => { + setupApi(); + render( + + + } + /> + , + { + route: '/projects/default/features/featureWithoutStrategies/strategies/create', + permissions: [{ permission: CREATE_FEATURE_STRATEGY }], + }, + ); + + const button = await screen.findByText('Add strategy'); + expect(button).toBeEnabled(); +}); + +test('should not allow to add strategy when limit reached', async () => { + setupApi(); + render( + + + } + /> + , + { + route: '/projects/default/features/featureWithManyStrategies/strategies/create', + permissions: [{ permission: CREATE_FEATURE_STRATEGY }], + }, + ); + + await waitFor(async () => { + const button = await screen.findByText('Add strategy'); + expect(button).toBeDisabled(); + }); +}); diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx index 5b6e340dffef..4dada03ff13a 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx @@ -22,6 +22,7 @@ import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { FeatureStrategyIcons } from 'component/feature/FeatureStrategy/FeatureStrategyIcons/FeatureStrategyIcons'; import { useGlobalLocalStorage } from 'hooks/useGlobalLocalStorage'; import { Badge } from 'component/common/Badge/Badge'; +import { useUiFlag } from 'hooks/useUiFlag'; interface IFeatureOverviewEnvironmentProps { env: IFeatureEnvironment; @@ -131,6 +132,11 @@ const FeatureOverviewEnvironment = ({ const featureEnvironment = feature?.environments.find( (featureEnvironment) => featureEnvironment.name === env.name, ); + const resourceLimitsEnabled = useUiFlag('resourceLimits'); + const limitReached = + resourceLimitsEnabled && + Array.isArray(featureEnvironment?.strategies) && + featureEnvironment?.strategies.length >= 30; return ( { +export const createFakeFeatureToggleService = (config: IUnleashConfig) => { const { getLogger, flagResolver } = config; const eventStore = new FakeEventStore(); const strategyStore = new FakeStrategiesStore(); @@ -216,5 +214,5 @@ export const createFakeFeatureToggleService = ( dependentFeaturesService, featureLifecycleReadModel, ); - return featureToggleService; + return { featureToggleService, featureToggleStore }; }; diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 4477cf333498..473371a73ac0 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -358,6 +358,27 @@ class FeatureToggleService { } } + async validateStrategyLimit( + featureEnv: { + projectId: string; + environment: string; + featureName: string; + }, + limit: number, + ) { + if (!this.flagResolver.isEnabled('resourceLimits')) return; + const existingCount = ( + await this.featureStrategiesStore.getStrategiesForFeatureEnv( + featureEnv.projectId, + featureEnv.featureName, + featureEnv.environment, + ) + ).length; + if (existingCount >= limit) { + throw new BadDataError(`Strategy limit of ${limit} exceeded}.`); + } + } + async validateStrategyType( strategyName: string | undefined, ): Promise { @@ -624,6 +645,11 @@ class FeatureToggleService { strategyConfig.variants = fixedVariants; } + await this.validateStrategyLimit( + { featureName, projectId, environment }, + 30, + ); + try { const newFeatureStrategy = await this.featureStrategiesStore.createStrategyFeatureEnv({ diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts new file mode 100644 index 000000000000..7013a29681ee --- /dev/null +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts @@ -0,0 +1,41 @@ +import { createFakeFeatureToggleService } from '../createFeatureToggleService'; +import type { + IAuditUser, + IFlagResolver, + IStrategyConfig, + IUnleashConfig, +} from '../../../types'; +import getLogger from '../../../../test/fixtures/no-logger'; + +const alwaysOnFlagResolver = { + isEnabled() { + return true; + }, +} as unknown as IFlagResolver; + +test('Should not allow to exceed strategy limit', async () => { + const { featureToggleService, featureToggleStore } = + createFakeFeatureToggleService({ + getLogger, + flagResolver: alwaysOnFlagResolver, + } as unknown as IUnleashConfig); + + const addStrategy = () => + featureToggleService.unprotectedCreateStrategy( + { name: 'default', featureName: 'feature' } as IStrategyConfig, + { projectId: 'default', featureName: 'feature' } as any, + {} as IAuditUser, + ); + await featureToggleStore.create('default', { + name: 'feature', + createdByUserId: 1, + }); + + for (let i = 0; i < 30; i++) { + await addStrategy(); + } + + await expect(addStrategy()).rejects.toThrow( + 'Strategy limit of 30 exceeded', + ); +}); diff --git a/src/lib/features/frontend-api/createFrontendApiService.ts b/src/lib/features/frontend-api/createFrontendApiService.ts index 51087589fd51..7eb25ad6ce30 100644 --- a/src/lib/features/frontend-api/createFrontendApiService.ts +++ b/src/lib/features/frontend-api/createFrontendApiService.ts @@ -72,7 +72,8 @@ export const createFakeFrontendApiService = ( eventService, ); // TODO: remove this dependency after we migrate frontend API - const featureToggleServiceV2 = createFakeFeatureToggleService(config); + const featureToggleServiceV2 = + createFakeFeatureToggleService(config).featureToggleService; const clientFeatureToggleReadModel = new FakeClientFeatureToggleReadModel(); const globalFrontendApiCache = new GlobalFrontendApiCache( config, diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index 261d17db10c5..6744b34698f9 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -150,7 +150,7 @@ export const createFakeProjectService = ( const featureTypeStore = new FakeFeatureTypeStore(); const projectStatsStore = new FakeProjectStatsStore(); const { accessService } = createFakeAccessService(config); - const featureToggleService = createFakeFeatureToggleService(config); + const { featureToggleService } = createFakeFeatureToggleService(config); const favoriteFeaturesStore = new FakeFavoriteFeaturesStore(); const favoriteProjectsStore = new FakeFavoriteProjectsStore(); const eventService = new EventService(