From ef80d7f81e77a8e5fa35ae6cad953080acb0a3e8 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 8 Jul 2024 10:27:01 +0200 Subject: [PATCH] ui limits for flags (#7541) This PR disables the "create feature flag" button when you've reached the limits. This one is a little more complex than the other UI limits, because we also have to take into account the project feature limit. I've tried to touch as little as possible, but I _have_ extracted the calculation of both limits into a single hook. --- .../CreateFeature/CreateFeature.test.tsx | 77 +++++++++++++++++++ .../feature/CreateFeature/CreateFeature.tsx | 76 ++++++++++++++++-- ...s => isProjectFeatureLimitReached.test.ts} | 12 +-- .../CreateFeature/useFlagLimits.test.ts | 71 +++++++++++++++++ .../CreateFeatureButton.tsx | 66 ---------------- .../ProjectFeatureTogglesHeader.tsx | 4 +- .../api/getters/useUiConfig/defaultValue.tsx | 1 + .../openapi/models/resourceLimitsSchema.ts | 3 + 8 files changed, 229 insertions(+), 81 deletions(-) create mode 100644 frontend/src/component/feature/CreateFeature/CreateFeature.test.tsx rename frontend/src/component/feature/CreateFeature/{isFeatureLimitReached.test.ts => isProjectFeatureLimitReached.test.ts} (59%) create mode 100644 frontend/src/component/feature/CreateFeature/useFlagLimits.test.ts delete mode 100644 frontend/src/component/feature/CreateFeatureButton/CreateFeatureButton.tsx diff --git a/frontend/src/component/feature/CreateFeature/CreateFeature.test.tsx b/frontend/src/component/feature/CreateFeature/CreateFeature.test.tsx new file mode 100644 index 0000000000..2c3dae6ed1 --- /dev/null +++ b/frontend/src/component/feature/CreateFeature/CreateFeature.test.tsx @@ -0,0 +1,77 @@ +import { screen, waitFor } from '@testing-library/react'; +import { render } from 'utils/testRenderer'; +import { testServerRoute, testServerSetup } from 'utils/testServer'; +import CreateFeature from './CreateFeature'; +import { CREATE_FEATURE } from 'component/providers/AccessProvider/permissions'; +import { Route, Routes } from 'react-router-dom'; + +const server = testServerSetup(); + +const setupApi = ({ + flagCount, + flagLimit, +}: { flagCount: number; flagLimit: number }) => { + testServerRoute(server, '/api/admin/ui-config', { + flags: { + resourceLimits: true, + }, + resourceLimits: { + featureFlags: flagLimit, + }, + }); + + testServerRoute(server, '/api/admin/search/features', { + total: flagCount, + features: Array.from({ length: flagCount }).map((_, i) => ({ + name: `flag-${i}`, + })), + }); +}; + +test("should allow you to create feature flags when you're below the global limit", async () => { + setupApi({ flagLimit: 3, flagCount: 2 }); + + render( + + } + /> + , + { + route: '/projects/default/create-toggle', + permissions: [{ permission: CREATE_FEATURE }], + }, + ); + + await waitFor(async () => { + const button = await screen.findByRole('button', { + name: /create feature flag/i, + }); + expect(button).not.toBeDisabled(); + }); +}); + +test("should not allow you to create API tokens when you're at the global limit", async () => { + setupApi({ flagLimit: 3, flagCount: 3 }); + + render( + + } + /> + , + { + route: '/projects/default/create-toggle', + permissions: [{ permission: CREATE_FEATURE }], + }, + ); + + await waitFor(async () => { + const button = await screen.findByRole('button', { + name: /create feature flag/i, + }); + expect(button).toBeDisabled(); + }); +}); diff --git a/frontend/src/component/feature/CreateFeature/CreateFeature.tsx b/frontend/src/component/feature/CreateFeature/CreateFeature.tsx index 53328aaab2..c2b3930ff2 100644 --- a/frontend/src/component/feature/CreateFeature/CreateFeature.tsx +++ b/frontend/src/component/feature/CreateFeature/CreateFeature.tsx @@ -17,12 +17,14 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit import useProjectOverview, { featuresCount, } from 'hooks/api/getters/useProjectOverview/useProjectOverview'; +import { useUiFlag } from 'hooks/useUiFlag'; +import { useGlobalFeatureSearch } from '../FeatureToggleList/useGlobalFeatureSearch'; const StyledAlert = styled(Alert)(({ theme }) => ({ marginBottom: theme.spacing(2), })); -export const isFeatureLimitReached = ( +export const isProjectFeatureLimitReached = ( featureLimit: number | null | undefined, currentFeatureCount: number, ): boolean => { @@ -33,6 +35,47 @@ export const isFeatureLimitReached = ( ); }; +const useGlobalFlagLimit = (flagLimit: number, flagCount: number) => { + const resourceLimitsEnabled = useUiFlag('resourceLimits'); + const limitReached = resourceLimitsEnabled && flagCount >= flagLimit; + + return { + limitReached, + limitMessage: limitReached + ? `You have reached the instance-wide limit of ${flagLimit} feature flags.` + : undefined, + }; +}; + +type FlagLimitsProps = { + global: { limit: number; count: number }; + project: { limit?: number; count: number }; +}; + +export const useFlagLimits = ({ global, project }: FlagLimitsProps) => { + const { + limitReached: globalFlagLimitReached, + limitMessage: globalLimitMessage, + } = useGlobalFlagLimit(global.limit, global.count); + + const projectFlagLimitReached = isProjectFeatureLimitReached( + project.limit, + project.count, + ); + + const limitMessage = globalFlagLimitReached + ? globalLimitMessage + : projectFlagLimitReached + ? `You have reached the project limit of ${project.limit} feature flags.` + : undefined; + + return { + limitMessage, + globalFlagLimitReached, + projectFlagLimitReached, + }; +}; + const CreateFeature = () => { const { setToastData, setToastApiError } = useToast(); const { setShowFeedback } = useContext(UIContext); @@ -60,6 +103,21 @@ const CreateFeature = () => { const { createFeatureToggle, loading } = useFeatureApi(); + const { total: totalFlags, loading: loadingTotalFlagCount } = + useGlobalFeatureSearch(); + + const { globalFlagLimitReached, projectFlagLimitReached, limitMessage } = + useFlagLimits({ + global: { + limit: uiConfig.resourceLimits.featureFlags, + count: totalFlags ?? 0, + }, + project: { + limit: projectInfo.featureLimit, + count: featuresCount(projectInfo), + }, + }); + const handleSubmit = async (e: Event) => { e.preventDefault(); clearErrors(); @@ -98,10 +156,6 @@ const CreateFeature = () => { navigate(GO_BACK); }; - const featureLimitReached = isFeatureLimitReached( - projectInfo.featureLimit, - featuresCount(projectInfo), - ); return ( { formatApiCode={formatApiCode} > Feature flag project limit reached. To @@ -145,10 +199,18 @@ const CreateFeature = () => { > diff --git a/frontend/src/component/feature/CreateFeature/isFeatureLimitReached.test.ts b/frontend/src/component/feature/CreateFeature/isProjectFeatureLimitReached.test.ts similarity index 59% rename from frontend/src/component/feature/CreateFeature/isFeatureLimitReached.test.ts rename to frontend/src/component/feature/CreateFeature/isProjectFeatureLimitReached.test.ts index 4ac55c90a7..77a520f979 100644 --- a/frontend/src/component/feature/CreateFeature/isFeatureLimitReached.test.ts +++ b/frontend/src/component/feature/CreateFeature/isProjectFeatureLimitReached.test.ts @@ -1,21 +1,21 @@ -import { isFeatureLimitReached } from './CreateFeature'; +import { isProjectFeatureLimitReached } from './CreateFeature'; test('isFeatureLimitReached should return false when featureLimit is null', async () => { - expect(isFeatureLimitReached(null, 5)).toBe(false); + expect(isProjectFeatureLimitReached(null, 5)).toBe(false); }); test('isFeatureLimitReached should return false when featureLimit is undefined', async () => { - expect(isFeatureLimitReached(undefined, 5)).toBe(false); + expect(isProjectFeatureLimitReached(undefined, 5)).toBe(false); }); test('isFeatureLimitReached should return false when featureLimit is smaller current feature count', async () => { - expect(isFeatureLimitReached(6, 5)).toBe(false); + expect(isProjectFeatureLimitReached(6, 5)).toBe(false); }); test('isFeatureLimitReached should return true when featureLimit is smaller current feature count', async () => { - expect(isFeatureLimitReached(4, 5)).toBe(true); + expect(isProjectFeatureLimitReached(4, 5)).toBe(true); }); test('isFeatureLimitReached should return true when featureLimit is equal to current feature count', async () => { - expect(isFeatureLimitReached(5, 5)).toBe(true); + expect(isProjectFeatureLimitReached(5, 5)).toBe(true); }); diff --git a/frontend/src/component/feature/CreateFeature/useFlagLimits.test.ts b/frontend/src/component/feature/CreateFeature/useFlagLimits.test.ts new file mode 100644 index 0000000000..b8abb2cf31 --- /dev/null +++ b/frontend/src/component/feature/CreateFeature/useFlagLimits.test.ts @@ -0,0 +1,71 @@ +import { renderHook } from '@testing-library/react'; +import { useFlagLimits } from './CreateFeature'; +import { vi } from 'vitest'; + +vi.mock('hooks/useUiFlag', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...(actual as {}), + useUiFlag: (flag: string) => flag === 'resourceLimits', + }; +}); + +test('if both global and project-level limits are reached, then the error message shows the message for instance-wide limits', () => { + const { result } = renderHook(() => + useFlagLimits({ + global: { limit: 1, count: 1 }, + project: { limit: 1, count: 1 }, + }), + ); + + expect(result.current).toMatchObject({ + globalFlagLimitReached: true, + projectFlagLimitReached: true, + limitMessage: expect.stringContaining('instance-wide limit'), + }); +}); + +test('if only global level is reached, the projectFlagLimitReached property is false', () => { + const { result } = renderHook(() => + useFlagLimits({ + global: { limit: 1, count: 1 }, + project: { limit: 1, count: 0 }, + }), + ); + + expect(result.current).toMatchObject({ + globalFlagLimitReached: true, + projectFlagLimitReached: false, + limitMessage: expect.stringContaining('instance-wide limit'), + }); +}); + +test('if only the project limit is reached, the limit message talks about the project limit', () => { + const { result } = renderHook(() => + useFlagLimits({ + global: { limit: 2, count: 1 }, + project: { limit: 1, count: 1 }, + }), + ); + + expect(result.current).toMatchObject({ + globalFlagLimitReached: false, + projectFlagLimitReached: true, + limitMessage: expect.stringContaining('project limit'), + }); +}); + +test('if neither limit is reached, the limit message is undefined', () => { + const { result } = renderHook(() => + useFlagLimits({ + global: { limit: 1, count: 0 }, + project: { limit: 1, count: 0 }, + }), + ); + + expect(result.current).toMatchObject({ + globalFlagLimitReached: false, + projectFlagLimitReached: false, + limitMessage: undefined, + }); +}); diff --git a/frontend/src/component/feature/CreateFeatureButton/CreateFeatureButton.tsx b/frontend/src/component/feature/CreateFeatureButton/CreateFeatureButton.tsx deleted file mode 100644 index 3e44b0c653..0000000000 --- a/frontend/src/component/feature/CreateFeatureButton/CreateFeatureButton.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import classnames from 'classnames'; -import { Link, useNavigate } from 'react-router-dom'; -import useMediaQuery from '@mui/material/useMediaQuery'; -import Add from '@mui/icons-material/Add'; -import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; -import { NAVIGATE_TO_CREATE_FEATURE } from 'utils/testIds'; -import { useCreateFeaturePath } from 'component/feature/CreateFeatureButton/useCreateFeaturePath'; -import PermissionButton from 'component/common/PermissionButton/PermissionButton'; -import { CREATE_FEATURE } from 'component/providers/AccessProvider/permissions'; -import PermissionIconButton from 'component/common/PermissionIconButton/PermissionIconButton'; - -interface ICreateFeatureButtonProps { - loading: boolean; - filter: { - query?: string; - project: string; - }; -} - -export const CreateFeatureButton = ({ - loading, - filter, -}: ICreateFeatureButtonProps) => { - const smallScreen = useMediaQuery('(max-width:800px)'); - const createFeature = useCreateFeaturePath(filter); - const navigate = useNavigate(); - - if (!createFeature) { - return null; - } - - return ( - - - - } - elseShow={ - { - navigate(createFeature.path); - }} - permission={CREATE_FEATURE} - projectId={createFeature.projectId} - color='primary' - variant='contained' - data-testid={NAVIGATE_TO_CREATE_FEATURE} - className={classnames({ skeleton: loading })} - > - New feature flag - - } - /> - ); -}; diff --git a/frontend/src/component/project/Project/PaginatedProjectFeatureToggles/ProjectFeatureTogglesHeader/ProjectFeatureTogglesHeader.tsx b/frontend/src/component/project/Project/PaginatedProjectFeatureToggles/ProjectFeatureTogglesHeader/ProjectFeatureTogglesHeader.tsx index f29ebced07..1d6985278a 100644 --- a/frontend/src/component/project/Project/PaginatedProjectFeatureToggles/ProjectFeatureTogglesHeader/ProjectFeatureTogglesHeader.tsx +++ b/frontend/src/component/project/Project/PaginatedProjectFeatureToggles/ProjectFeatureTogglesHeader/ProjectFeatureTogglesHeader.tsx @@ -1,4 +1,4 @@ -import { type ReactNode, type VFC, useState } from 'react'; +import { type ReactNode, type FC, useState } from 'react'; import { Box, Button, @@ -40,7 +40,7 @@ const StyledResponsiveButton = styled(ResponsiveButton)(() => ({ whiteSpace: 'nowrap', })); -export const ProjectFeatureTogglesHeader: VFC< +export const ProjectFeatureTogglesHeader: FC< IProjectFeatureTogglesHeaderProps > = ({ isLoading, diff --git a/frontend/src/hooks/api/getters/useUiConfig/defaultValue.tsx b/frontend/src/hooks/api/getters/useUiConfig/defaultValue.tsx index 903af08f0a..d29274a266 100644 --- a/frontend/src/hooks/api/getters/useUiConfig/defaultValue.tsx +++ b/frontend/src/hooks/api/getters/useUiConfig/defaultValue.tsx @@ -43,5 +43,6 @@ export const defaultValue: IUiConfig = { projects: 500, segments: 300, apiTokens: 2000, + featureFlags: 5000, }, }; diff --git a/frontend/src/openapi/models/resourceLimitsSchema.ts b/frontend/src/openapi/models/resourceLimitsSchema.ts index a7fb36e34c..86727ee99f 100644 --- a/frontend/src/openapi/models/resourceLimitsSchema.ts +++ b/frontend/src/openapi/models/resourceLimitsSchema.ts @@ -41,4 +41,7 @@ export interface ResourceLimitsSchema { * total number of tokens across all projects in your * organization. */ apiTokens: number; + /** The maximum number of feature flags you can have at the same + * time. Archived flags do not count towards this limit. */ + featureFlags: number; }