diff --git a/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraFeedback/ProjectDoraFeedback.tsx b/frontend/src/component/common/ExperimentalFeedback/ExperimentalFeedback.tsx similarity index 75% rename from frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraFeedback/ProjectDoraFeedback.tsx rename to frontend/src/component/common/ExperimentalFeedback/ExperimentalFeedback.tsx index c5e8ede757..5827f55016 100644 --- a/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraFeedback/ProjectDoraFeedback.tsx +++ b/frontend/src/component/common/ExperimentalFeedback/ExperimentalFeedback.tsx @@ -1,7 +1,7 @@ import { useState, useEffect } from 'react'; import { Box, Button, Divider, Typography, styled } from '@mui/material'; import { PermMedia, Send } from '@mui/icons-material'; -import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; +import { CustomEvents, usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { createLocalStorage } from 'utils/createLocalStorage'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; @@ -61,12 +61,21 @@ const StyledLink = styled('a')(({ theme }) => ({ textDecoration: 'none', })); -export const ProjectDoraFeedback = () => { +interface IExperimentalFeedbackProps { + trackerKey: string; + eventKey: CustomEvents; + description: string; + sketchURL: string; +} + +export const ExperimentalFeedback: React.FC = ({ + trackerKey, + eventKey, + description, + sketchURL, +}) => { const { trackEvent } = usePlausibleTracker(); - const { value, setValue } = createLocalStorage( - `project:metrics:plausible`, - { sent: false }, - ); + const { value, setValue } = createLocalStorage(trackerKey, { sent: false }); const [metrics, setMetrics] = useState(value); useEffect(() => { @@ -75,7 +84,7 @@ export const ProjectDoraFeedback = () => { const onBtnClick = (type: string) => { try { - trackEvent('project-metrics', { + trackEvent(eventKey, { props: { eventType: type, }, @@ -91,7 +100,7 @@ export const ProjectDoraFeedback = () => { const recipientEmail = 'ux@getunleash.io'; const emailSubject = "I'd like to get involved"; - const emailBody = `Hello Unleash,\n\nI just saw the new metrics page you are experimenting with in Unleash. I'd like to be involved in user tests and give my feedback on this feature.\n\nRegards,\n`; + const emailBody = `Hello Unleash,\n\nI just saw your ${eventKey} experiment. I'd like to be involved in user tests and give my feedback on this feature.\n\nRegards,\n`; const mailtoURL = `mailto:${recipientEmail}?subject=${encodeURIComponent( emailSubject, @@ -102,31 +111,10 @@ export const ProjectDoraFeedback = () => { We are trying something experimental! - - We are considering adding project metrics to see how a project - performs. As a first step, we have added a{' '} - lead time for changes indicator that is calculated per - feature toggle based on the creation of the feature toggle and - when it was first turned on in an environment of type - production. - + {description}
- - DORA is a method for measuring the performance of your DevOps - teams. It measures four different metrics. You can read Google's - blog post about{' '} - - DORA metrics - {' '} - for more information. - - { diff --git a/frontend/src/component/feature/FeatureForm/FeatureForm.tsx b/frontend/src/component/feature/FeatureForm/FeatureForm.tsx index 2da886aa89..436c625e64 100644 --- a/frontend/src/component/feature/FeatureForm/FeatureForm.tsx +++ b/frontend/src/component/feature/FeatureForm/FeatureForm.tsx @@ -23,7 +23,6 @@ import React from 'react'; import { useAuthPermissions } from 'hooks/api/getters/useAuth/useAuthPermissions'; import { FeatureNamingType } from 'interfaces/project'; import { FeatureNamingPatternInfo } from '../FeatureNamingPatternInfo/FeatureNamingPatternInfo'; -import { useUiFlag } from 'hooks/useUiFlag'; interface IFeatureToggleForm { type: string; @@ -122,15 +121,12 @@ const FeatureForm: React.FC = ({ const navigate = useNavigate(); const { permissions } = useAuthPermissions(); const editable = mode !== 'Edit'; - const featureNamingPatternEnabled = useUiFlag('featureNamingPattern'); const renderToggleDescription = () => { return featureTypes.find((toggle) => toggle.id === type)?.description; }; - const displayFeatureNamingInfo = Boolean( - featureNamingPatternEnabled && featureNaming?.pattern, - ); + const displayFeatureNamingInfo = Boolean(featureNaming?.pattern); React.useEffect(() => { if (featureNaming?.pattern && validateToggleName && name) { diff --git a/frontend/src/component/project/Project/HiddenProjectIconWithTooltip/HiddenProjectIconWithTooltip.tsx b/frontend/src/component/project/Project/HiddenProjectIconWithTooltip/HiddenProjectIconWithTooltip.tsx new file mode 100644 index 0000000000..bb753b5caa --- /dev/null +++ b/frontend/src/component/project/Project/HiddenProjectIconWithTooltip/HiddenProjectIconWithTooltip.tsx @@ -0,0 +1,17 @@ +import { styled } from '@mui/material'; +import { VisibilityOff } from '@mui/icons-material'; +import { HtmlTooltip } from 'component/common/HtmlTooltip/HtmlTooltip'; + +export const StyledVisibilityIcon = styled(VisibilityOff)(({ theme }) => ({ + color: theme.palette.action.disabled, +})); + +export const HiddenProjectIconWithTooltip = () => ( + + + +); diff --git a/frontend/src/component/project/Project/Project.tsx b/frontend/src/component/project/Project/Project.tsx index cf1ff921a9..500e00c896 100644 --- a/frontend/src/component/project/Project/Project.tsx +++ b/frontend/src/component/project/Project/Project.tsx @@ -41,6 +41,7 @@ import { Badge } from 'component/common/Badge/Badge'; import { ProjectDoraMetrics } from './ProjectDoraMetrics/ProjectDoraMetrics'; import { UiFlags } from 'interfaces/uiConfig'; import { ExperimentalProjectFeatures } from './ExperimentalProjectFeatures/ExperimentalProjectFeatures'; +import { HiddenProjectIconWithTooltip } from './HiddenProjectIconWithTooltip/HiddenProjectIconWithTooltip'; const StyledBadge = styled(Badge)(({ theme }) => ({ position: 'absolute', @@ -102,8 +103,7 @@ export const Project = () => { title: 'Metrics', path: `${basePath}/metrics`, name: 'dora', - flag: 'doraMetrics', - new: true, + isEnterprise: true, }, { title: 'Event log', @@ -190,6 +190,10 @@ export const Project = () => { isFavorite={project?.favorite} /> + } + /> {projectName} diff --git a/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraMetrics.tsx b/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraMetrics.tsx index 2ad4c61c18..5cfb217d61 100644 --- a/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraMetrics.tsx +++ b/frontend/src/component/project/Project/ProjectDoraMetrics/ProjectDoraMetrics.tsx @@ -15,7 +15,6 @@ import { PageContent } from 'component/common/PageContent/PageContent'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { PageHeader } from 'component/common/PageHeader/PageHeader'; import { Badge } from 'component/common/Badge/Badge'; -import { ProjectDoraFeedback } from './ProjectDoraFeedback/ProjectDoraFeedback'; import { useConditionallyHiddenColumns } from 'hooks/useConditionallyHiddenColumns'; import theme from 'themes/theme'; @@ -194,7 +193,6 @@ export const ProjectDoraMetrics = () => { return ( <> - { open: - everyone can submit change requests + Everyone can submit change requests protected: - only admins and project members can submit change + Only admins and project members can submit change requests @@ -39,8 +39,9 @@ export const CollaborationModeTooltip: FC = () => { private: - only projects members can and access see the - project + Only admins, editors and project members can + see and access the project and associated + feature toggles } diff --git a/frontend/src/component/project/Project/ProjectEnterpriseSettingsForm/ProjectEnterpriseSettingsForm.tsx b/frontend/src/component/project/Project/ProjectEnterpriseSettingsForm/ProjectEnterpriseSettingsForm.tsx index 559d04e590..41f54c664d 100644 --- a/frontend/src/component/project/Project/ProjectEnterpriseSettingsForm/ProjectEnterpriseSettingsForm.tsx +++ b/frontend/src/component/project/Project/ProjectEnterpriseSettingsForm/ProjectEnterpriseSettingsForm.tsx @@ -136,7 +136,6 @@ const ProjectEnterpriseSettingsForm: React.FC = clearErrors, }) => { const privateProjects = useUiFlag('privateProjects'); - const shouldShowFlagNaming = useUiFlag('featureNamingPattern'); const { setPreviousPattern, trackPattern } = useFeatureNamePatternTracking(); @@ -253,115 +252,104 @@ const ProjectEnterpriseSettingsForm: React.FC = options={projectModeOptions} /> - - - Feature flag naming pattern? - - - - -

- Define a{' '} - - JavaScript RegEx - {' '} - used to enforce feature flag names - within this project. The regex will be - surrounded by a leading ^{' '} - and a trailing $. -

-

- Leave it empty if you don’t want to add - a naming pattern. -

- - - - - ^ - - ), - endAdornment: ( - - $ - - ), - }} - type={'text'} - value={featureNamingPattern || ''} - error={Boolean(errors.featureNamingPattern)} - errorText={errors.featureNamingPattern} - onChange={(e) => - onSetFeatureNamingPattern( - e.target.value, - ) - } - /> - -

- The example and description will be - shown to users when they create a new - feature flag in this project. -

- + + + Feature flag naming pattern? + + + + +

+ Define a{' '} + + JavaScript RegEx + {' '} + used to enforce feature flag names within this + project. The regex will be surrounded by a + leading ^ and a trailing{' '} + $. +

+

+ Leave it empty if you don’t want to add a naming + pattern. +

+ + + + + ^ + + ), + endAdornment: ( + + $ + + ), + }} + type={'text'} + value={featureNamingPattern || ''} + error={Boolean(errors.featureNamingPattern)} + errorText={errors.featureNamingPattern} + onChange={(e) => + onSetFeatureNamingPattern(e.target.value) + } + /> + +

+ The example and description will be shown to + users when they create a new feature flag in + this project. +

+ - - onSetFeatureNamingExample( - e.target.value, - ) - } - /> - .. + + onSetFeatureNamingExample(e.target.value) + } + /> + .. The flag name should contain the project name, the feature name, and the ticket number, each separated by a dot.`} - multiline - minRows={5} - value={featureNamingDescription || ''} - onChange={(e) => - onSetFeatureNamingDescription( - e.target.value, - ) - } - /> - - - } - /> + multiline + minRows={5} + value={featureNamingDescription || ''} + onChange={(e) => + onSetFeatureNamingDescription(e.target.value) + } + /> + + {children} ); diff --git a/frontend/src/component/project/ProjectCard/ProjectCard.styles.ts b/frontend/src/component/project/ProjectCard/ProjectCard.styles.ts index bf78de9c48..397947a832 100644 --- a/frontend/src/component/project/ProjectCard/ProjectCard.styles.ts +++ b/frontend/src/component/project/ProjectCard/ProjectCard.styles.ts @@ -75,3 +75,8 @@ export const StyledParagraphInfo = styled('p')(({ theme }) => ({ color: theme.palette.primary.dark, fontWeight: 'bold', })); + +export const StyledIconBox = styled(Box)(() => ({ + display: 'flex', + justifyContent: 'center', +})); diff --git a/frontend/src/component/project/ProjectCard/ProjectCard.tsx b/frontend/src/component/project/ProjectCard/ProjectCard.tsx index 52ff4a5eac..f35d3b612b 100644 --- a/frontend/src/component/project/ProjectCard/ProjectCard.tsx +++ b/frontend/src/component/project/ProjectCard/ProjectCard.tsx @@ -1,4 +1,4 @@ -import { Menu, MenuItem } from '@mui/material'; +import { Menu, MenuItem, Tooltip, Box } from '@mui/material'; import MoreVertIcon from '@mui/icons-material/MoreVert'; import React, { SyntheticEvent, useContext, useState } from 'react'; import { useNavigate } from 'react-router-dom'; @@ -26,8 +26,10 @@ import { StyledDivInfo, StyledDivInfoContainer, StyledParagraphInfo, + StyledIconBox, } from './ProjectCard.styles'; import useToast from 'hooks/useToast'; +import { HiddenProjectIconWithTooltip } from '../Project/HiddenProjectIconWithTooltip/HiddenProjectIconWithTooltip'; interface IProjectCardProps { name: string; @@ -37,6 +39,7 @@ interface IProjectCardProps { id: string; onHover: () => void; isFavorite?: boolean; + mode: string; } export const ProjectCard = ({ @@ -46,6 +49,7 @@ export const ProjectCard = ({ memberCount, onHover, id, + mode, isFavorite = false, }: IProjectCardProps) => { const { hasAccess } = useContext(AccessContext); @@ -127,9 +131,13 @@ export const ProjectCard = ({ -
- -
+ + } + elseShow={} + /> + diff --git a/frontend/src/component/project/ProjectList/ProjectList.tsx b/frontend/src/component/project/ProjectList/ProjectList.tsx index a9f5c5ce67..a6d8d9279e 100644 --- a/frontend/src/component/project/ProjectList/ProjectList.tsx +++ b/frontend/src/component/project/ProjectList/ProjectList.tsx @@ -245,6 +245,7 @@ export const ProjectListNew = () => { key={project.id} name={project.name} id={project.id} + mode={project.mode} memberCount={2} health={95} featureCount={4} @@ -263,6 +264,7 @@ export const ProjectListNew = () => { handleHover(project.id) } name={project.name} + mode={project.mode} memberCount={ project.memberCount ?? 0 } diff --git a/frontend/src/component/project/ProjectList/loadingData.ts b/frontend/src/component/project/ProjectList/loadingData.ts index 04de3f2973..4e6f4d7033 100644 --- a/frontend/src/component/project/ProjectList/loadingData.ts +++ b/frontend/src/component/project/ProjectList/loadingData.ts @@ -7,6 +7,7 @@ const loadingData = [ featureCount: 4, createdAt: '', description: '', + mode: '', }, { id: 'loading2', @@ -16,6 +17,7 @@ const loadingData = [ featureCount: 4, createdAt: '', description: '', + mode: '', }, { id: 'loading3', @@ -25,6 +27,7 @@ const loadingData = [ featureCount: 4, createdAt: '', description: '', + mode: '', }, { id: 'loading4', @@ -34,6 +37,7 @@ const loadingData = [ featureCount: 4, createdAt: '', description: '', + mode: '', }, ]; diff --git a/frontend/src/hooks/useTableState.test.ts b/frontend/src/hooks/useTableState.test.ts new file mode 100644 index 0000000000..ce2f716d4f --- /dev/null +++ b/frontend/src/hooks/useTableState.test.ts @@ -0,0 +1,291 @@ +import { vi, type Mock } from 'vitest'; +import { renderHook } from '@testing-library/react-hooks'; +import { useTableState } from './useTableState'; +import { createLocalStorage } from 'utils/createLocalStorage'; +import { useSearchParams } from 'react-router-dom'; +import { act } from 'react-test-renderer'; + +vi.mock('react-router-dom', () => ({ + useSearchParams: vi.fn(() => [new URLSearchParams(), vi.fn()]), +})); +vi.mock('../utils/createLocalStorage', () => ({ + createLocalStorage: vi.fn(() => ({ + value: {}, + setValue: vi.fn(), + })), +})); + +const mockStorage = createLocalStorage as Mock; +const mockQuery = useSearchParams as Mock; + +describe('useTableState', () => { + beforeEach(() => { + mockStorage.mockRestore(); + mockQuery.mockRestore(); + }); + + it('should return default params', () => { + const { result } = renderHook(() => + useTableState<{ + page: '0'; + pageSize: '10'; + }>({ page: '0', pageSize: '10' }, 'test', [], []), + ); + expect(result.current[0]).toEqual({ page: '0', pageSize: '10' }); + }); + + it('should return params from local storage', () => { + mockStorage.mockReturnValue({ + value: { pageSize: 25 }, + setValue: vi.fn(), + }); + + const { result } = renderHook(() => + useTableState({ pageSize: '10' }, 'test', [], []), + ); + + expect(result.current[0]).toEqual({ pageSize: 25 }); + }); + + it('should return params from url', () => { + mockQuery.mockReturnValue([new URLSearchParams('page=1'), vi.fn()]); + + const { result } = renderHook(() => + useTableState({ page: '0' }, 'test', [], []), + ); + + expect(result.current[0]).toEqual({ page: '1' }); + }); + + it('should use params from url over local storage', () => { + mockQuery.mockReturnValue([ + new URLSearchParams('page=2&pageSize=25'), + vi.fn(), + ]); + mockStorage.mockReturnValue({ + value: { pageSize: '10', sortOrder: 'desc' }, + setValue: vi.fn(), + }); + + const { result } = renderHook(() => + useTableState({ page: '1', pageSize: '5' }, 'test', [], []), + ); + + expect(result.current[0]).toEqual({ + page: '2', + pageSize: '25', + }); + }); + + it('sets local state', () => { + const { result } = renderHook(() => + useTableState({ page: '1' }, 'test', [], []), + ); + const setParams = result.current[1]; + + act(() => { + setParams({ page: '2' }); + }); + + expect(result.current[0]).toEqual({ page: '2' }); + }); + + it('keeps previous state', () => { + const { result } = renderHook(() => + useTableState({ page: '1', pageSize: '10' }, 'test', [], []), + ); + const setParams = result.current[1]; + + act(() => { + setParams({ page: '2' }); + }); + + expect(result.current[0]).toEqual({ page: '2', pageSize: '10' }); + }); + + it('removes params from previous state', () => { + const { result } = renderHook(() => + useTableState({ page: '1', pageSize: '10' }, 'test', [], []), + ); + const setParams = result.current[1]; + + act(() => { + setParams({ pageSize: undefined }); + }); + + expect(result.current[0]).toEqual({ page: '1' }); + + // ensure that there are no keys with undefined values + expect(Object.keys(result.current[0])).toHaveLength(1); + }); + + it('removes params from url', () => { + const querySetter = vi.fn(); + mockQuery.mockReturnValue([new URLSearchParams('page=2'), querySetter]); + + const { result } = renderHook(() => + useTableState( + { page: '1', pageSize: '10' }, + 'test', + ['page', 'pageSize'], + [], + ), + ); + const setParams = result.current[1]; + + expect(result.current[0]).toEqual({ page: '2', pageSize: '10' }); + + act(() => { + setParams({ page: '10', pageSize: undefined }); + }); + + expect(result.current[0]).toEqual({ page: '10' }); + + expect(querySetter).toHaveBeenCalledWith({ + page: '10', + }); + }); + + it('removes params from local storage', () => { + const storageSetter = vi.fn(); + mockStorage.mockReturnValue({ + value: { sortBy: 'type' }, + setValue: storageSetter, + }); + + const { result } = renderHook(() => + useTableState( + { sortBy: 'createdAt', pageSize: '10' }, + 'test', + [], + ['sortBy', 'pageSize'], + ), + ); + + expect(result.current[0]).toEqual({ sortBy: 'type', pageSize: '10' }); + + act(() => { + result.current[1]({ pageSize: undefined }); + }); + + expect(result.current[0]).toEqual({ sortBy: 'type' }); + + expect(storageSetter).toHaveBeenCalledWith({ + sortBy: 'type', + }); + }); + + test('saves default parameters if not explicitly provided', (key) => { + const querySetter = vi.fn(); + const storageSetter = vi.fn(); + mockQuery.mockReturnValue([new URLSearchParams(), querySetter]); + mockStorage.mockReturnValue({ + value: {}, + setValue: storageSetter, + }); + + const { result } = renderHook(() => useTableState({}, 'test')); + + act(() => { + result.current[1]({ + unspecified: 'test', + page: '2', + pageSize: '10', + search: 'test', + sortBy: 'type', + sortOrder: 'favorites', + favorites: 'false', + columns: ['test', 'id'], + }); + }); + + expect(storageSetter).toHaveBeenCalledTimes(1); + expect(storageSetter).toHaveBeenCalledWith({ + pageSize: '10', + search: 'test', + sortBy: 'type', + sortOrder: 'favorites', + favorites: 'false', + columns: ['test', 'id'], + }); + expect(querySetter).toHaveBeenCalledTimes(1); + expect(querySetter).toHaveBeenCalledWith({ + page: '2', + pageSize: '10', + search: 'test', + sortBy: 'type', + sortOrder: 'favorites', + favorites: 'false', + columns: ['test', 'id'], + }); + }); + + it("doesn't save default params if explicitly specified", () => { + const storageSetter = vi.fn(); + mockStorage.mockReturnValue({ + value: {}, + setValue: storageSetter, + }); + const querySetter = vi.fn(); + mockQuery.mockReturnValue([new URLSearchParams(), querySetter]); + + const { result } = renderHook(() => + useTableState<{ + [key: string]: string | string[]; + }>({}, 'test', ['saveOnlyThisToUrl'], ['page']), + ); + const setParams = result.current[1]; + + act(() => { + setParams({ + saveOnlyThisToUrl: 'test', + page: '2', + pageSize: '10', + search: 'test', + sortBy: 'type', + sortOrder: 'favorites', + favorites: 'false', + columns: ['test', 'id'], + }); + }); + + expect(querySetter).toHaveBeenCalledWith({ saveOnlyThisToUrl: 'test' }); + expect(storageSetter).toHaveBeenCalledWith({ page: '2' }); + }); + + it('can reset state to the default instead of overwriting', () => { + mockStorage.mockReturnValue({ + value: { pageSize: 25 }, + setValue: vi.fn(), + }); + mockQuery.mockReturnValue([new URLSearchParams('page=4'), vi.fn()]); + + const { result } = renderHook(() => + useTableState<{ + page: string; + pageSize?: string; + sortBy?: string; + }>({ page: '1', pageSize: '10' }, 'test'), + ); + + const setParams = result.current[1]; + + act(() => { + setParams({ sortBy: 'type' }); + }); + expect(result.current[0]).toEqual({ + page: '4', + pageSize: '10', + sortBy: 'type', + }); + + act(() => { + setParams({ pageSize: '50' }, true); + }); + + expect(result.current[0]).toEqual({ + page: '1', + pageSize: '50', + }); + }); +}); diff --git a/frontend/src/hooks/useTableState.ts b/frontend/src/hooks/useTableState.ts new file mode 100644 index 0000000000..8e2ad79351 --- /dev/null +++ b/frontend/src/hooks/useTableState.ts @@ -0,0 +1,82 @@ +import { useState } from 'react'; +import { useSearchParams } from 'react-router-dom'; +import { createLocalStorage } from '../utils/createLocalStorage'; + +const filterObjectKeys = >( + obj: T, + keys: Array, +) => + Object.fromEntries( + Object.entries(obj).filter(([key]) => keys.includes(key as keyof T)), + ) as T; + +const defaultStoredKeys = [ + 'pageSize', + 'search', + 'sortBy', + 'sortOrder', + 'favorites', + 'columns', +]; +const defaultQueryKeys = [...defaultStoredKeys, 'page']; + +/** + * There are 3 sources of params, in order of priority: + * 1. local state + * 2. search params from the url + * 3. stored params in local storage + * 4. default parameters + * + * `queryKeys` will be saved in the url + * `storedKeys` will be saved in local storage + * + * @param defaultParams initial state + * @param storageId identifier for the local storage + * @param queryKeys array of elements to be saved in the url + * @param storageKeys array of elements to be saved in local storage + */ +export const useTableState = >( + defaultParams: Params, + storageId: string, + queryKeys?: Array, + storageKeys?: Array, +) => { + const [searchParams, setSearchParams] = useSearchParams(); + const { value: storedParams, setValue: setStoredParams } = + createLocalStorage(`${storageId}:tableQuery`, defaultParams); + + const searchQuery = Object.fromEntries(searchParams.entries()); + const [params, setParams] = useState({ + ...defaultParams, + ...(Object.keys(searchQuery).length ? {} : storedParams), + ...searchQuery, + } as Params); + + const updateParams = (value: Partial, reset = false) => { + const newState: Params = reset + ? { ...defaultParams, ...value } + : { + ...params, + ...value, + }; + + // remove keys with undefined values + Object.keys(newState).forEach((key) => { + if (newState[key] === undefined) { + delete newState[key]; + } + }); + + setParams(newState); + setSearchParams( + filterObjectKeys(newState, queryKeys || defaultQueryKeys), + ); + setStoredParams( + filterObjectKeys(newState, storageKeys || defaultStoredKeys), + ); + + return params; + }; + + return [params, updateParams] as const; +}; diff --git a/frontend/src/interfaces/project.ts b/frontend/src/interfaces/project.ts index 06d4a3a9d2..b17f6fb86d 100644 --- a/frontend/src/interfaces/project.ts +++ b/frontend/src/interfaces/project.ts @@ -10,6 +10,7 @@ export interface IProjectCard { health: number; description: string; featureCount: number; + mode: string; memberCount?: number; favorite?: boolean; } diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index ffb6884c5c..41be2d88d3 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -61,7 +61,6 @@ export type UiFlags = { customRootRolesKillSwitch?: boolean; strategyVariant?: boolean; lastSeenByEnvironment?: boolean; - featureNamingPattern?: boolean; doraMetrics?: boolean; variantTypeNumber?: boolean; privateProjects?: boolean; diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 6cb1885dc1..e495b69df9 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -81,10 +81,8 @@ exports[`should create default config 1`] = ` "disableEnvsOnRevive": false, "disableMetrics": false, "disableNotifications": false, - "doraMetrics": false, "embedProxy": true, "embedProxyFrontend": true, - "featureNamingPattern": false, "featureSearchAPI": false, "featureSearchFrontend": false, "featuresExportImport": true, 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 9095b48ac1..06f49a6103 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 @@ -159,7 +159,6 @@ beforeAll(async () => { experimental: { flags: { featuresExportImport: true, - featureNamingPattern: true, dependentFeatures: true, }, }, diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 9e665cde69..4bc977e6e8 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -1168,8 +1168,9 @@ class FeatureToggleService { projectId: string, featureNames: string[], ): Promise { - if (this.flagResolver.isEnabled('featureNamingPattern')) { + try { const project = await this.projectStore.get(projectId); + const patternData = project.featureNaming; const namingPattern = patternData?.pattern; @@ -1183,7 +1184,17 @@ class FeatureToggleService { return { ...result, featureNaming: patternData }; } } + } catch (error) { + // the project doesn't exist, so there's nothing to + // validate against + this.logger.info( + "Got an error when trying to validate flag naming patterns. It is probably because the target project doesn't exist. Here's the error:", + error.message, + ); + + return { state: 'valid' }; } + return { state: 'valid' }; } diff --git a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts index 92a39ea0f5..c3a5674233 100644 --- a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts @@ -709,7 +709,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { const [, envName] = sortBy.split(':'); rankingSql += this.db .raw( - `CASE WHEN feature_environments.environment = ? THEN feature_environments.enabled ELSE NULL END ${validatedSortOrder} NULLS LAST, features.created_at asc`, + `CASE WHEN feature_environments.environment = ? THEN feature_environments.enabled ELSE NULL END ${validatedSortOrder} NULLS LAST, features.created_at asc, features.name asc`, [envName], ) .toString(); @@ -718,9 +718,9 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { .raw(`?? ${validatedSortOrder}`, [ sortByMapping[sortBy], ]) - .toString()}, features.created_at asc`; + .toString()}, features.created_at asc, features.name asc`; } else { - rankingSql += `features.created_at ${validatedSortOrder}`; + rankingSql += `features.created_at ${validatedSortOrder}, features.name asc`; } query diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts index 974c51c45e..c27e36f686 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts @@ -39,7 +39,7 @@ const irrelevantDate = new Date(); beforeAll(async () => { const config = createTestConfig({ experimental: { - flags: { featureNamingPattern: true, playgroundImprovements: true }, + flags: { playgroundImprovements: true }, }, }); db = await dbInit( @@ -641,6 +641,20 @@ describe('flag name validation', () => { ).resolves.toBeFalsy(); } }); + + test("should allow anything if the project doesn't exist", async () => { + const projectId = 'project-that-doesnt-exist'; + const validFeatures = ['testpattern-feature', 'testpattern-feature2']; + + for (const feature of validFeatures) { + await expect( + service.validateFeatureFlagNameAgainstPattern( + feature, + projectId, + ), + ).resolves.toBeFalsy(); + } + }); }); test('Should return last seen at per environment', async () => { diff --git a/src/lib/features/scheduler/schedule-services.ts b/src/lib/features/scheduler/schedule-services.ts new file mode 100644 index 0000000000..0e276270c9 --- /dev/null +++ b/src/lib/features/scheduler/schedule-services.ts @@ -0,0 +1,153 @@ +import { + hoursToMilliseconds, + minutesToMilliseconds, + secondsToMilliseconds, +} from 'date-fns'; +import { IUnleashServices } from '../../server-impl'; + +/** + * Schedules service methods. + * + * In order to promote runtime control, you should **not use** a flagResolver inside this method. Instead, implement your flag usage inside the scheduled methods themselves. + * @param services + */ +export const scheduleServices = async ( + services: IUnleashServices, +): Promise => { + const { + accountService, + schedulerService, + apiTokenService, + instanceStatsService, + clientInstanceService, + projectService, + projectHealthService, + configurationRevisionService, + eventAnnouncerService, + featureToggleService, + versionService, + lastSeenService, + proxyService, + clientMetricsServiceV2, + } = services; + + schedulerService.schedule( + lastSeenService.cleanLastSeen.bind(lastSeenService), + hoursToMilliseconds(1), + 'cleanLastSeen', + ); + + schedulerService.schedule( + lastSeenService.store.bind(lastSeenService), + secondsToMilliseconds(30), + 'storeLastSeen', + ); + + schedulerService.schedule( + apiTokenService.fetchActiveTokens.bind(apiTokenService), + minutesToMilliseconds(1), + 'fetchActiveTokens', + ); + + schedulerService.schedule( + apiTokenService.updateLastSeen.bind(apiTokenService), + minutesToMilliseconds(3), + 'updateLastSeen', + ); + + schedulerService.schedule( + instanceStatsService.refreshStatsSnapshot.bind(instanceStatsService), + minutesToMilliseconds(5), + 'refreshStatsSnapshot', + ); + + schedulerService.schedule( + clientInstanceService.removeInstancesOlderThanTwoDays.bind( + clientInstanceService, + ), + hoursToMilliseconds(24), + 'removeInstancesOlderThanTwoDays', + ); + + schedulerService.schedule( + clientInstanceService.bulkAdd.bind(clientInstanceService), + secondsToMilliseconds(5), + 'bulkAddInstances', + ); + + schedulerService.schedule( + clientInstanceService.announceUnannounced.bind(clientInstanceService), + minutesToMilliseconds(5), + 'announceUnannounced', + ); + + schedulerService.schedule( + projectService.statusJob.bind(projectService), + hoursToMilliseconds(24), + 'statusJob', + ); + + schedulerService.schedule( + projectHealthService.setHealthRating.bind(projectHealthService), + hoursToMilliseconds(1), + 'setHealthRating', + ); + + schedulerService.schedule( + configurationRevisionService.updateMaxRevisionId.bind( + configurationRevisionService, + ), + secondsToMilliseconds(1), + 'updateMaxRevisionId', + ); + + schedulerService.schedule( + eventAnnouncerService.publishUnannouncedEvents.bind( + eventAnnouncerService, + ), + secondsToMilliseconds(1), + 'publishUnannouncedEvents', + ); + + schedulerService.schedule( + featureToggleService.updatePotentiallyStaleFeatures.bind( + featureToggleService, + ), + minutesToMilliseconds(1), + 'updatePotentiallyStaleFeatures', + ); + + schedulerService.schedule( + versionService.checkLatestVersion.bind(versionService), + hoursToMilliseconds(48), + 'checkLatestVersion', + ); + + schedulerService.schedule( + proxyService.fetchFrontendSettings.bind(proxyService), + minutesToMilliseconds(2), + 'fetchFrontendSettings', + ); + + schedulerService.schedule( + () => { + clientMetricsServiceV2.bulkAdd().catch(console.error); + }, + secondsToMilliseconds(5), + 'bulkAddMetrics', + ); + + schedulerService.schedule( + () => { + clientMetricsServiceV2.clearMetrics(48).catch(console.error); + }, + hoursToMilliseconds(12), + 'clearMetrics', + ); + + schedulerService.schedule( + accountService.updateLastSeen.bind(accountService), + minutesToMilliseconds(3), + 'updateAccountLastSeen', + ); +}; diff --git a/src/lib/features/scheduler/scheduler-service.test.ts b/src/lib/features/scheduler/scheduler-service.test.ts new file mode 100644 index 0000000000..55f0fdd7f7 --- /dev/null +++ b/src/lib/features/scheduler/scheduler-service.test.ts @@ -0,0 +1,239 @@ +import { SchedulerService } from './scheduler-service'; +import { LogProvider } from '../../logger'; +import MaintenanceService from '../../services/maintenance-service'; +import { createTestConfig } from '../../../test/config/test-config'; +import SettingService from '../../services/setting-service'; +import FakeSettingStore from '../../../test/fixtures/fake-setting-store'; +import EventService from '../../services/event-service'; + +function ms(timeMs) { + return new Promise((resolve) => setTimeout(resolve, timeMs)); +} + +const getLogger = () => { + const records: any[] = []; + const logger: LogProvider = () => ({ + error(...args: any[]) { + records.push(args); + }, + debug() {}, + info() {}, + warn() {}, + fatal() {}, + }); + const getRecords = () => records; + + return { logger, getRecords }; +}; + +const toggleMaintenanceMode = async ( + maintenanceService: MaintenanceService, + enabled: boolean, +) => { + await maintenanceService.toggleMaintenanceMode( + { enabled }, + 'irrelevant user', + ); +}; + +test('Schedules job immediately', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await schedulerService.schedule(job, 10, 'test-id'); + + expect(job).toBeCalledTimes(1); + schedulerService.stop(); +}); + +test('Does not schedule job immediately when paused', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await toggleMaintenanceMode(maintenanceService, true); + await schedulerService.schedule(job, 10, 'test-id-2'); + + expect(job).toBeCalledTimes(0); + schedulerService.stop(); +}); + +test('Can schedule a single regular job', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await schedulerService.schedule(job, 50, 'test-id-3'); + await ms(75); + + expect(job).toBeCalledTimes(2); + schedulerService.stop(); +}); + +test('Scheduled job ignored in a paused mode', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await toggleMaintenanceMode(maintenanceService, true); + await schedulerService.schedule(job, 50, 'test-id-4'); + await ms(75); + + expect(job).toBeCalledTimes(0); + schedulerService.stop(); +}); + +test('Can resume paused job', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await toggleMaintenanceMode(maintenanceService, true); + await schedulerService.schedule(job, 50, 'test-id-5'); + await toggleMaintenanceMode(maintenanceService, false); + await ms(75); + + expect(job).toBeCalledTimes(1); + schedulerService.stop(); +}); + +test('Can schedule multiple jobs at the same interval', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + const anotherJob = jest.fn(); + + await schedulerService.schedule(job, 50, 'test-id-6'); + await schedulerService.schedule(anotherJob, 50, 'test-id-7'); + await ms(75); + + expect(job).toBeCalledTimes(2); + expect(anotherJob).toBeCalledTimes(2); + schedulerService.stop(); +}); + +test('Can schedule multiple jobs at the different intervals', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + const job = jest.fn(); + const anotherJob = jest.fn(); + + await schedulerService.schedule(job, 100, 'test-id-8'); + await schedulerService.schedule(anotherJob, 200, 'test-id-9'); + await ms(250); + + expect(job).toBeCalledTimes(3); + expect(anotherJob).toBeCalledTimes(2); + schedulerService.stop(); +}); + +test('Can handle crash of a async job', async () => { + const { logger, getRecords } = getLogger(); + const config = { ...createTestConfig(), logger }; + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService(logger, maintenanceService); + + const job = async () => { + await Promise.reject('async reason'); + }; + + await schedulerService.schedule(job, 50, 'test-id-10'); + await ms(75); + + schedulerService.stop(); + expect(getRecords()).toEqual([ + ['scheduled job failed | id: test-id-10 | async reason'], + ['scheduled job failed | id: test-id-10 | async reason'], + ]); +}); + +test('Can handle crash of a sync job', async () => { + const { logger, getRecords } = getLogger(); + const config = { ...createTestConfig(), logger }; + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService(logger, maintenanceService); + + const job = () => { + throw new Error('sync reason'); + }; + + await schedulerService.schedule(job, 50, 'test-id-11'); + await ms(75); + + schedulerService.stop(); + expect(getRecords()).toEqual([ + ['scheduled job failed | id: test-id-11 | Error: sync reason'], + ['scheduled job failed | id: test-id-11 | Error: sync reason'], + ]); +}); diff --git a/src/lib/services/scheduler-service.ts b/src/lib/features/scheduler/scheduler-service.ts similarity index 60% rename from src/lib/services/scheduler-service.ts rename to src/lib/features/scheduler/scheduler-service.ts index c2f8ae0478..20f4a94a3c 100644 --- a/src/lib/services/scheduler-service.ts +++ b/src/lib/features/scheduler/scheduler-service.ts @@ -1,17 +1,19 @@ -import { Logger, LogProvider } from '../logger'; - -export type SchedulerMode = 'active' | 'paused'; +import { Logger, LogProvider } from '../../logger'; +import MaintenanceService from '../../services/maintenance-service'; export class SchedulerService { private intervalIds: NodeJS.Timer[] = []; - private mode: SchedulerMode; - private logger: Logger; - constructor(getLogger: LogProvider) { + private maintenanceService: MaintenanceService; + + constructor( + getLogger: LogProvider, + maintenanceService: MaintenanceService, + ) { this.logger = getLogger('/services/scheduler-service.ts'); - this.mode = 'active'; + this.maintenanceService = maintenanceService; } async schedule( @@ -22,7 +24,9 @@ export class SchedulerService { this.intervalIds.push( setInterval(async () => { try { - if (this.mode === 'active') { + const maintenanceMode = + await this.maintenanceService.isMaintenanceMode(); + if (!maintenanceMode) { await scheduledFunction(); } } catch (e) { @@ -33,7 +37,9 @@ export class SchedulerService { }, timeMs).unref(), ); try { - if (this.mode === 'active') { + const maintenanceMode = + await this.maintenanceService.isMaintenanceMode(); + if (!maintenanceMode) { await scheduledFunction(); } } catch (e) { @@ -44,16 +50,4 @@ export class SchedulerService { stop(): void { this.intervalIds.forEach(clearInterval); } - - pause(): void { - this.mode = 'paused'; - } - - resume(): void { - this.mode = 'active'; - } - - getMode(): SchedulerMode { - return this.mode; - } } diff --git a/src/lib/middleware/cors-origin-middleware.test.ts b/src/lib/middleware/cors-origin-middleware.test.ts index 2e81ebdbda..abd44622da 100644 --- a/src/lib/middleware/cors-origin-middleware.test.ts +++ b/src/lib/middleware/cors-origin-middleware.test.ts @@ -4,15 +4,9 @@ import { createTestConfig } from '../../test/config/test-config'; import FakeEventStore from '../../test/fixtures/fake-event-store'; import { randomId } from '../util/random-id'; import FakeProjectStore from '../../test/fixtures/fake-project-store'; -import { - EventService, - ProxyService, - SchedulerService, - SettingService, -} from '../../lib/services'; +import { EventService, ProxyService, SettingService } from '../../lib/services'; import { ISettingStore } from '../../lib/types'; import { frontendSettingsKey } from '../../lib/types/settings/frontend-settings'; -import { minutesToMilliseconds } from 'date-fns'; import FakeFeatureTagStore from '../../test/fixtures/fake-feature-tag-store'; const createSettingService = ( diff --git a/src/lib/routes/admin-api/project/index.ts b/src/lib/routes/admin-api/project/index.ts index 720c0e5634..51b3fe6f29 100644 --- a/src/lib/routes/admin-api/project/index.ts +++ b/src/lib/routes/admin-api/project/index.ts @@ -173,21 +173,15 @@ export default class ProjectApi extends Controller { req: IAuthRequest, res: Response, ): Promise { - if (this.config.flagResolver.isEnabled('doraMetrics')) { - const { projectId } = req.params; + const { projectId } = req.params; - const dora = await this.projectService.getDoraMetrics(projectId); + const dora = await this.projectService.getDoraMetrics(projectId); - this.openApiService.respondWithValidation( - 200, - res, - projectDoraMetricsSchema.$id, - dora, - ); - } else { - throw new InvalidOperationError( - 'Feature dora metrics is not enabled', - ); - } + this.openApiService.respondWithValidation( + 200, + res, + projectDoraMetricsSchema.$id, + dora, + ); } } diff --git a/src/lib/server-impl.ts b/src/lib/server-impl.ts index dd3fe82ec9..93d99c2750 100644 --- a/src/lib/server-impl.ts +++ b/src/lib/server-impl.ts @@ -5,7 +5,7 @@ import { migrateDb } from '../migrator'; import getApp from './app'; import { createMetricsMonitor } from './metrics'; import { createStores } from './db'; -import { createServices, scheduleServices } from './services'; +import { createServices } from './services'; import { createConfig } from './create-config'; import registerGracefulShutdown from './util/graceful-shutdown'; import { createDb } from './db/db-pool'; @@ -33,6 +33,7 @@ import * as permissions from './types/permissions'; import * as eventType from './types/events'; import { Db } from './db/db'; import { defaultLockKey, defaultTimeout, withDbLock } from './util/db-lock'; +import { scheduleServices } from './features/scheduler/schedule-services'; async function createApp( config: IUnleashConfig, @@ -45,7 +46,7 @@ async function createApp( const stores = createStores(config, db); const services = createServices(stores, config, db); if (!config.disableScheduler) { - await scheduleServices(services, config.flagResolver); + await scheduleServices(services); } const metricsMonitor = createMetricsMonitor(); diff --git a/src/lib/services/account-service.ts b/src/lib/services/account-service.ts index 2f13b3403b..78aecaf48c 100644 --- a/src/lib/services/account-service.ts +++ b/src/lib/services/account-service.ts @@ -18,8 +18,6 @@ export class AccountService { private accessService: AccessService; - private seenTimer: NodeJS.Timeout; - private lastSeenSecrets: Set = new Set(); constructor( @@ -32,7 +30,6 @@ export class AccountService { this.logger = getLogger('service/account-service.ts'); this.store = stores.accountStore; this.accessService = services.accessService; - this.updateLastSeen(); } async getAll(): Promise { @@ -63,19 +60,9 @@ export class AccountService { this.lastSeenSecrets = new Set(); await this.store.markSeenAt(toStore); } - - this.seenTimer = setTimeout( - async () => this.updateLastSeen(), - minutesToMilliseconds(3), - ).unref(); } addPATSeen(secret: string): void { this.lastSeenSecrets.add(secret); } - - destroy(): void { - clearTimeout(this.seenTimer); - this.seenTimer = null; - } } diff --git a/src/lib/services/client-metrics/last-seen/last-seen-service.ts b/src/lib/services/client-metrics/last-seen/last-seen-service.ts index 4bb6d5a282..c211c4dbf9 100644 --- a/src/lib/services/client-metrics/last-seen/last-seen-service.ts +++ b/src/lib/services/client-metrics/last-seen/last-seen-service.ts @@ -1,9 +1,12 @@ -import { secondsToMilliseconds } from 'date-fns'; import { Logger } from '../../../logger'; import { IUnleashConfig } from '../../../server-impl'; import { IClientMetricsEnv } from '../../../types/stores/client-metrics-store-v2'; import { ILastSeenStore } from './types/last-seen-store-type'; -import { IFeatureToggleStore, IUnleashStores } from '../../../../lib/types'; +import { + IFeatureToggleStore, + IFlagResolver, + IUnleashStores, +} from '../../../../lib/types'; export type LastSeenInput = { featureName: string; @@ -21,6 +24,8 @@ export class LastSeenService { private config: IUnleashConfig; + private flagResolver: IFlagResolver; + constructor( { featureToggleStore, @@ -33,6 +38,7 @@ export class LastSeenService { this.logger = config.getLogger( '/services/client-metrics/last-seen-service.ts', ); + this.flagResolver = config.flagResolver; this.config = config; } @@ -75,6 +81,8 @@ export class LastSeenService { } async cleanLastSeen() { - await this.lastSeenStore.cleanLastSeen(); + if (this.flagResolver.isEnabled('useLastSeenRefactor')) { + await this.lastSeenStore.cleanLastSeen(); + } } } diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 36c2cbb752..db7ef63b27 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -1,9 +1,4 @@ -import { - IUnleashConfig, - IUnleashStores, - IUnleashServices, - IFlagResolver, -} from '../types'; +import { IUnleashConfig, IUnleashStores, IUnleashServices } from '../types'; import FeatureTypeService from './feature-type-service'; import EventService from './event-service'; import HealthService from './health-service'; @@ -44,13 +39,8 @@ import { LastSeenService } from './client-metrics/last-seen/last-seen-service'; import { InstanceStatsService } from '../features/instance-stats/instance-stats-service'; import { FavoritesService } from './favorites-service'; import MaintenanceService from './maintenance-service'; -import { - hoursToMilliseconds, - minutesToMilliseconds, - secondsToMilliseconds, -} from 'date-fns'; import { AccountService } from './account-service'; -import { SchedulerService } from './scheduler-service'; +import { SchedulerService } from '../features/scheduler/scheduler-service'; import { Knex } from 'knex'; import { createExportImportTogglesService, @@ -109,149 +99,6 @@ import { } from '../features/feature-search/createFeatureSearchService'; import { FeatureSearchService } from '../features/feature-search/feature-search-service'; -// TODO: will be moved to scheduler feature directory -export const scheduleServices = async ( - services: IUnleashServices, - flagResolver: IFlagResolver, -): Promise => { - const { - schedulerService, - apiTokenService, - instanceStatsService, - clientInstanceService, - projectService, - projectHealthService, - configurationRevisionService, - maintenanceService, - eventAnnouncerService, - featureToggleService, - versionService, - lastSeenService, - proxyService, - clientMetricsServiceV2, - } = services; - - if (await maintenanceService.isMaintenanceMode()) { - schedulerService.pause(); - } - - if (flagResolver.isEnabled('useLastSeenRefactor')) { - schedulerService.schedule( - lastSeenService.cleanLastSeen.bind(lastSeenService), - hoursToMilliseconds(1), - 'cleanLastSeen', - ); - } - - schedulerService.schedule( - lastSeenService.store.bind(lastSeenService), - secondsToMilliseconds(30), - 'storeLastSeen', - ); - - schedulerService.schedule( - apiTokenService.fetchActiveTokens.bind(apiTokenService), - minutesToMilliseconds(1), - 'fetchActiveTokens', - ); - - schedulerService.schedule( - apiTokenService.updateLastSeen.bind(apiTokenService), - minutesToMilliseconds(3), - 'updateLastSeen', - ); - - schedulerService.schedule( - instanceStatsService.refreshStatsSnapshot.bind(instanceStatsService), - minutesToMilliseconds(5), - 'refreshStatsSnapshot', - ); - - schedulerService.schedule( - clientInstanceService.removeInstancesOlderThanTwoDays.bind( - clientInstanceService, - ), - hoursToMilliseconds(24), - 'removeInstancesOlderThanTwoDays', - ); - - schedulerService.schedule( - clientInstanceService.bulkAdd.bind(clientInstanceService), - secondsToMilliseconds(5), - 'bulkAddInstances', - ); - - schedulerService.schedule( - clientInstanceService.announceUnannounced.bind(clientInstanceService), - minutesToMilliseconds(5), - 'announceUnannounced', - ); - - schedulerService.schedule( - projectService.statusJob.bind(projectService), - hoursToMilliseconds(24), - 'statusJob', - ); - - schedulerService.schedule( - projectHealthService.setHealthRating.bind(projectHealthService), - hoursToMilliseconds(1), - 'setHealthRating', - ); - - schedulerService.schedule( - configurationRevisionService.updateMaxRevisionId.bind( - configurationRevisionService, - ), - secondsToMilliseconds(1), - 'updateMaxRevisionId', - ); - - schedulerService.schedule( - eventAnnouncerService.publishUnannouncedEvents.bind( - eventAnnouncerService, - ), - secondsToMilliseconds(1), - 'publishUnannouncedEvents', - ); - - schedulerService.schedule( - featureToggleService.updatePotentiallyStaleFeatures.bind( - featureToggleService, - ), - minutesToMilliseconds(1), - 'updatePotentiallyStaleFeatures', - ); - - schedulerService.schedule( - versionService.checkLatestVersion.bind(versionService), - hoursToMilliseconds(48), - 'checkLatestVersion', - ); - - schedulerService.schedule( - proxyService.fetchFrontendSettings.bind(proxyService), - minutesToMilliseconds(2), - 'fetchFrontendSettings', - ); - - schedulerService.schedule( - () => { - clientMetricsServiceV2.bulkAdd().catch(console.error); - }, - secondsToMilliseconds(5), - 'bulkAddMetrics', - ); - - schedulerService.schedule( - () => { - clientMetricsServiceV2.clearMetrics(48).catch(console.error); - }, - hoursToMilliseconds(12), - 'clearMetrics', - ); -}; - export const createServices = ( stores: IUnleashStores, config: IUnleashConfig, @@ -438,13 +285,11 @@ export const createServices = ( db ? createGetProductionChanges(db) : createFakeGetProductionChanges(), ); - const schedulerService = new SchedulerService(config.getLogger); + const maintenanceService = new MaintenanceService(config, settingService); - const maintenanceService = new MaintenanceService( - stores, - config, - settingService, - schedulerService, + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, ); const eventAnnouncerService = new EventAnnouncerService(stores, config); diff --git a/src/lib/services/maintenance-service.test.ts b/src/lib/services/maintenance-service.test.ts index 4eab1c0adc..fc4be7e720 100644 --- a/src/lib/services/maintenance-service.test.ts +++ b/src/lib/services/maintenance-service.test.ts @@ -1,17 +1,40 @@ -import { SchedulerService } from './scheduler-service'; +import { SchedulerService } from '../features/scheduler/scheduler-service'; import MaintenanceService from './maintenance-service'; -import { IUnleashStores } from '../types'; import SettingService from './setting-service'; import { createTestConfig } from '../../test/config/test-config'; +import FakeSettingStore from '../../test/fixtures/fake-setting-store'; +import EventService from './event-service'; -test('Maintenance on should pause scheduler', async () => { +test('Scheduler should run scheduled functions if maintenance mode is off', async () => { const config = createTestConfig(); - const schedulerService = new SchedulerService(config.getLogger); - const maintenanceService = new MaintenanceService( - {} as IUnleashStores, - config, - { insert() {} } as unknown as SettingService, - schedulerService, + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, + ); + + const job = jest.fn(); + + await schedulerService.schedule(job, 10, 'test-id'); + + expect(job).toBeCalledTimes(1); + schedulerService.stop(); +}); + +test('Scheduler should not run scheduled functions if maintenance mode is on', async () => { + const config = createTestConfig(); + const settingStore = new FakeSettingStore(); + const settingService = new SettingService({ settingStore }, config, { + storeEvent() {}, + } as unknown as EventService); + const maintenanceService = new MaintenanceService(config, settingService); + const schedulerService = new SchedulerService( + config.getLogger, + maintenanceService, ); await maintenanceService.toggleMaintenanceMode( @@ -19,26 +42,10 @@ test('Maintenance on should pause scheduler', async () => { 'irrelevant user', ); - expect(schedulerService.getMode()).toBe('paused'); - schedulerService.stop(); -}); - -test('Maintenance off should resume scheduler', async () => { - const config = createTestConfig({ disableScheduler: false }); - const schedulerService = new SchedulerService(config.getLogger); - schedulerService.pause(); - const maintenanceService = new MaintenanceService( - {} as IUnleashStores, - config, - { insert() {} } as unknown as SettingService, - schedulerService, - ); - - await maintenanceService.toggleMaintenanceMode( - { enabled: false }, - 'irrelevant user', - ); - - expect(schedulerService.getMode()).toBe('active'); + const job = jest.fn(); + + await schedulerService.schedule(job, 10, 'test-id'); + + expect(job).toBeCalledTimes(0); schedulerService.stop(); }); diff --git a/src/lib/services/maintenance-service.ts b/src/lib/services/maintenance-service.ts index 21be7f1249..78c2ad7d1e 100644 --- a/src/lib/services/maintenance-service.ts +++ b/src/lib/services/maintenance-service.ts @@ -1,40 +1,20 @@ -import { IUnleashConfig, IUnleashStores } from '../types'; +import { IUnleashConfig } from '../types'; import { Logger } from '../logger'; -import { IPatStore } from '../types/stores/pat-store'; -import { IEventStore } from '../types/stores/event-store'; import SettingService from './setting-service'; import { maintenanceSettingsKey } from '../types/settings/maintenance-settings'; import { MaintenanceSchema } from '../openapi/spec/maintenance-schema'; -import { SchedulerService } from './scheduler-service'; export default class MaintenanceService { private config: IUnleashConfig; private logger: Logger; - private patStore: IPatStore; - - private eventStore: IEventStore; - private settingService: SettingService; - private schedulerService: SchedulerService; - - constructor( - { - patStore, - eventStore, - }: Pick, - config: IUnleashConfig, - settingService: SettingService, - schedulerService: SchedulerService, - ) { + constructor(config: IUnleashConfig, settingService: SettingService) { this.config = config; this.logger = config.getLogger('services/pat-service.ts'); - this.patStore = patStore; - this.eventStore = eventStore; this.settingService = settingService; - this.schedulerService = schedulerService; } async isMaintenanceMode(): Promise { @@ -56,11 +36,6 @@ export default class MaintenanceService { setting: MaintenanceSchema, user: string, ): Promise { - if (setting.enabled) { - this.schedulerService.pause(); - } else if (!this.config.disableScheduler) { - this.schedulerService.resume(); - } return this.settingService.insert( maintenanceSettingsKey, setting, diff --git a/src/lib/services/scheduler-service.test.ts b/src/lib/services/scheduler-service.test.ts deleted file mode 100644 index 0a03a7b0f8..0000000000 --- a/src/lib/services/scheduler-service.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -import { SchedulerService } from './scheduler-service'; -import { LogProvider } from '../logger'; - -function ms(timeMs) { - return new Promise((resolve) => setTimeout(resolve, timeMs)); -} - -const getLogger = () => { - const records: any[] = []; - const logger: LogProvider = () => ({ - error(...args: any[]) { - records.push(args); - }, - debug() {}, - info() {}, - warn() {}, - fatal() {}, - }); - const getRecords = () => records; - - return { logger, getRecords }; -}; - -test('Schedules job immediately', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - - schedulerService.schedule(job, 10, 'test-id'); - - expect(job).toBeCalledTimes(1); - schedulerService.stop(); -}); - -test('Does not schedule job immediately when paused', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - - schedulerService.pause(); - schedulerService.schedule(job, 10, 'test-id-2'); - - expect(job).toBeCalledTimes(0); - schedulerService.stop(); -}); - -test('Can schedule a single regular job', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - - schedulerService.schedule(job, 50, 'test-id-3'); - await ms(75); - - expect(job).toBeCalledTimes(2); - schedulerService.stop(); -}); - -test('Scheduled job ignored in a paused mode', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - - schedulerService.pause(); - schedulerService.schedule(job, 50, 'test-id-4'); - await ms(75); - - expect(job).toBeCalledTimes(0); - schedulerService.stop(); -}); - -test('Can resume paused job', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - - schedulerService.pause(); - schedulerService.schedule(job, 50, 'test-id-5'); - schedulerService.resume(); - await ms(75); - - expect(job).toBeCalledTimes(1); - schedulerService.stop(); -}); - -test('Can schedule multiple jobs at the same interval', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - const anotherJob = jest.fn(); - - schedulerService.schedule(job, 50, 'test-id-6'); - schedulerService.schedule(anotherJob, 50, 'test-id-7'); - await ms(75); - - expect(job).toBeCalledTimes(2); - expect(anotherJob).toBeCalledTimes(2); - schedulerService.stop(); -}); - -test('Can schedule multiple jobs at the different intervals', async () => { - const { logger } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = jest.fn(); - const anotherJob = jest.fn(); - - schedulerService.schedule(job, 100, 'test-id-8'); - schedulerService.schedule(anotherJob, 200, 'test-id-9'); - await ms(250); - - expect(job).toBeCalledTimes(3); - expect(anotherJob).toBeCalledTimes(2); - schedulerService.stop(); -}); - -test('Can handle crash of a async job', async () => { - const { logger, getRecords } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = async () => { - await Promise.reject('async reason'); - }; - - schedulerService.schedule(job, 50, 'test-id-10'); - await ms(75); - - schedulerService.stop(); - expect(getRecords()).toEqual([ - ['scheduled job failed | id: test-id-10 | async reason'], - ['scheduled job failed | id: test-id-10 | async reason'], - ]); -}); - -test('Can handle crash of a sync job', async () => { - const { logger, getRecords } = getLogger(); - const schedulerService = new SchedulerService(logger); - const job = () => { - throw new Error('sync reason'); - }; - - schedulerService.schedule(job, 50, 'test-id-11'); - await ms(75); - - schedulerService.stop(); - expect(getRecords()).toEqual([ - ['scheduled job failed | id: test-id-11 | Error: sync reason'], - ['scheduled job failed | id: test-id-11 | Error: sync reason'], - ]); -}); diff --git a/src/lib/services/segment-service.ts b/src/lib/services/segment-service.ts index 0b9657422b..7441cff49c 100644 --- a/src/lib/services/segment-service.ts +++ b/src/lib/services/segment-service.ts @@ -125,7 +125,9 @@ export class SegmentService implements ISegmentService { await this.changeRequestSegmentUsageReadModel.getStrategiesUsedInActiveChangeRequests( id, ) - ).filter((strategy) => !strategyIds.has(strategy.id)); + ).filter( + (strategy) => !('id' in strategy && strategyIds.has(strategy.id)), + ); return { strategies, changeRequestStrategies }; } diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index d6825d5eb7..f6810809a7 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -23,8 +23,6 @@ export type IFlagKey = | 'filterInvalidClientMetrics' | 'lastSeenByEnvironment' | 'customRootRolesKillSwitch' - | 'featureNamingPattern' - | 'doraMetrics' | 'variantTypeNumber' | 'privateProjects' | 'dependentFeatures' @@ -116,14 +114,6 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_CUSTOM_ROOT_ROLES_KILL_SWITCH, false, ), - featureNamingPattern: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_FEATURE_NAMING_PATTERN, - false, - ), - doraMetrics: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_DORA_METRICS, - false, - ), dependentFeatures: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_DEPENDENT_FEATURES, false, diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index a5c15769c7..51ec2bf626 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -37,7 +37,7 @@ import { InstanceStatsService } from '../features/instance-stats/instance-stats- import { FavoritesService } from '../services/favorites-service'; import MaintenanceService from '../services/maintenance-service'; import { AccountService } from '../services/account-service'; -import { SchedulerService } from '../services/scheduler-service'; +import { SchedulerService } from '../features/scheduler/scheduler-service'; import { Knex } from 'knex'; import { IExportService, diff --git a/src/server-dev.ts b/src/server-dev.ts index 5755f862a3..7920390cf0 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -38,8 +38,6 @@ process.nextTick(async () => { anonymiseEventLog: false, responseTimeWithAppNameKillSwitch: false, lastSeenByEnvironment: true, - featureNamingPattern: true, - doraMetrics: true, variantTypeNumber: true, privateProjects: true, dependentFeatures: true, diff --git a/src/test/e2e/api/client/feature.e2e.test.ts b/src/test/e2e/api/client/feature.e2e.test.ts index b2fe54eec0..c68de4738a 100644 --- a/src/test/e2e/api/client/feature.e2e.test.ts +++ b/src/test/e2e/api/client/feature.e2e.test.ts @@ -21,7 +21,6 @@ beforeAll(async () => { experimental: { flags: { strictSchemaValidation: true, - featureNamingPattern: true, dependentFeatures: true, }, }, diff --git a/website/docs/reference/strategy-variants.md b/website/docs/reference/strategy-variants.md index f7367aaee3..ce62857256 100644 --- a/website/docs/reference/strategy-variants.md +++ b/website/docs/reference/strategy-variants.md @@ -75,6 +75,8 @@ Unleash currently supports these payload types: When you have only one variant in a strategy, stickiness does not matter. If you decide to add multiple variants to the strategy, then variant stickiness is derived from the strategy stickiness. Strategy stickiness is calculated on the received user and context, as described in [the stickiness chapter](./stickiness.md). This ensures that the same user will consistently see the same variant. If no context data is provided, the traffic will be spread randomly for each request. +If you would like to reassign users to different variants using existing stickiness parameter then you can change the groupId of the strategy. This will provide different input to the stickiness calculation. + ### Strategy variants vs feature toggle variants Strategy variants take precedence over the [feature toggle variants](./feature-toggle-variants.md). If your matching activation strategy doesn't have any variants configured you will fall back to the [feature toggle variants](./feature-toggle-variants.md).