diff --git a/frontend/src/component/App.tsx b/frontend/src/component/App.tsx index ede602779a0e..778ca85f065c 100644 --- a/frontend/src/component/App.tsx +++ b/frontend/src/component/App.tsx @@ -20,8 +20,8 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; import MaintenanceBanner from './maintenance/MaintenanceBanner'; import { styled } from '@mui/material'; import { InitialRedirect } from './InitialRedirect'; -import { InternalMessageBanners } from './messageBanners/internalMessageBanners/InternalMessageBanners'; -import { ExternalMessageBanners } from './messageBanners/externalMessageBanners/ExternalMessageBanners'; +import { InternalBanners } from './banners/internalBanners/InternalBanners'; +import { ExternalBanners } from './banners/externalBanners/ExternalBanners'; const StyledContainer = styled('div')(() => ({ '& ul': { @@ -65,8 +65,8 @@ export const App = () => { )} show={} /> - - + + diff --git a/frontend/src/component/messageBanners/MessageBanner/MessageBanner.tsx b/frontend/src/component/banners/Banner/Banner.tsx similarity index 71% rename from frontend/src/component/messageBanners/MessageBanner/MessageBanner.tsx rename to frontend/src/component/banners/Banner/Banner.tsx index 2e7a3783291b..454085973565 100644 --- a/frontend/src/component/messageBanners/MessageBanner/MessageBanner.tsx +++ b/frontend/src/component/banners/Banner/Banner.tsx @@ -7,33 +7,26 @@ import { import { styled, Icon, Link } from '@mui/material'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { useNavigate } from 'react-router-dom'; -import { MessageBannerDialog } from './MessageBannerDialog/MessageBannerDialog'; +import { BannerDialog } from './BannerDialog/BannerDialog'; import { useState } from 'react'; import ReactMarkdown from 'react-markdown'; -import { BannerVariant, IMessageBanner } from 'interfaces/messageBanner'; +import { BannerVariant, IBanner } from 'interfaces/banner'; +import { Sticky } from 'component/common/Sticky/Sticky'; const StyledBar = styled('aside', { - shouldForwardProp: (prop) => prop !== 'variant' && prop !== 'sticky', -})<{ variant: BannerVariant; sticky?: boolean }>( - ({ theme, variant, sticky }) => ({ - position: sticky ? 'sticky' : 'relative', - zIndex: 1, - display: 'flex', - alignItems: 'center', - justifyContent: 'center', - padding: theme.spacing(1), - gap: theme.spacing(1), - borderBottom: '1px solid', - borderColor: theme.palette[variant].border, - background: theme.palette[variant].light, - color: theme.palette[variant].dark, - fontSize: theme.fontSizes.smallBody, - ...(sticky && { - top: 0, - zIndex: theme.zIndex.sticky - 100, - }), - }), -); + shouldForwardProp: (prop) => prop !== 'variant', +})<{ variant: BannerVariant }>(({ theme, variant }) => ({ + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + padding: theme.spacing(1), + gap: theme.spacing(1), + borderBottom: '1px solid', + borderColor: theme.palette[variant].border, + background: theme.palette[variant].light, + color: theme.palette[variant].dark, + fontSize: theme.fontSizes.smallBody, +})); const StyledIcon = styled('div', { shouldForwardProp: (prop) => prop !== 'variant', @@ -43,11 +36,11 @@ const StyledIcon = styled('div', { color: theme.palette[variant].main, })); -interface IMessageBannerProps { - messageBanner: IMessageBanner; +interface IBannerProps { + banner: IBanner; } -export const MessageBanner = ({ messageBanner }: IMessageBannerProps) => { +export const Banner = ({ banner }: IBannerProps) => { const [open, setOpen] = useState(false); const { @@ -60,10 +53,10 @@ export const MessageBanner = ({ messageBanner }: IMessageBannerProps) => { plausibleEvent, dialogTitle, dialog, - } = messageBanner; + } = banner; - return ( - + const bannerBar = ( + @@ -75,15 +68,21 @@ export const MessageBanner = ({ messageBanner }: IMessageBannerProps) => { > {linkText} - {dialog!} - + ); + + if (sticky) { + return {bannerBar}; + } + + return bannerBar; }; const VariantIcons = { @@ -127,7 +126,7 @@ const BannerButton = ({ const trackEvent = () => { if (!plausibleEvent) return; - tracker.trackEvent('message_banner', { + tracker.trackEvent('banner', { props: { event: plausibleEvent }, }); }; diff --git a/frontend/src/component/messageBanners/MessageBanner/MessageBannerDialog/MessageBannerDialog.tsx b/frontend/src/component/banners/Banner/BannerDialog/BannerDialog.tsx similarity index 87% rename from frontend/src/component/messageBanners/MessageBanner/MessageBannerDialog/MessageBannerDialog.tsx rename to frontend/src/component/banners/Banner/BannerDialog/BannerDialog.tsx index 49a32e6b6c84..8d92557cb0cf 100644 --- a/frontend/src/component/messageBanners/MessageBanner/MessageBannerDialog/MessageBannerDialog.tsx +++ b/frontend/src/component/banners/Banner/BannerDialog/BannerDialog.tsx @@ -8,19 +8,19 @@ const StyledReactMarkdown = styled(ReactMarkdown)(({ theme }) => ({ }, })); -interface IMessageBannerDialogProps { +interface IBannerDialogProps { title: string; open: boolean; setOpen: React.Dispatch>; children: string; } -export const MessageBannerDialog = ({ +export const BannerDialog = ({ open, setOpen, title, children, -}: IMessageBannerDialogProps) => { +}: IBannerDialogProps) => { return ( { + const { uiConfig } = useUiConfig(); + + const bannerVariantFromMessageBannerFlag = useVariant( + uiConfig.flags.messageBanner, + ); + const bannerVariantFromBannerFlag = useVariant( + uiConfig.flags.banner, + ); + + const bannerVariant = + bannerVariantFromMessageBannerFlag || bannerVariantFromBannerFlag || []; + + const banners: IBanner[] = Array.isArray(bannerVariant) + ? bannerVariant + : [bannerVariant]; + + return ( + <> + {banners.map((banner) => ( + + ))} + + ); +}; diff --git a/frontend/src/component/banners/internalBanners/InternalBanners.tsx b/frontend/src/component/banners/internalBanners/InternalBanners.tsx new file mode 100644 index 000000000000..3db8dbd90aa1 --- /dev/null +++ b/frontend/src/component/banners/internalBanners/InternalBanners.tsx @@ -0,0 +1,14 @@ +import { Banner } from 'component/banners/Banner/Banner'; +import { useBanners } from 'hooks/api/getters/useBanners/useBanners'; + +export const InternalBanners = () => { + const { banners } = useBanners(); + + return ( + <> + {banners.map((banner) => ( + + ))} + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest.test.tsx b/frontend/src/component/changeRequest/ChangeRequest.test.tsx index 7075f523cc90..408a8f5ea1a4 100644 --- a/frontend/src/component/changeRequest/ChangeRequest.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest.test.tsx @@ -8,6 +8,7 @@ import { AccessProvider } from '../providers/AccessProvider/AccessProvider'; import { AnnouncerProvider } from '../common/Announcer/AnnouncerProvider/AnnouncerProvider'; import { testServerRoute, testServerSetup } from '../../utils/testServer'; import { UIProviderContainer } from '../providers/UIProvider/UIProviderContainer'; +import { StickyProvider } from 'component/common/Sticky/StickyProvider'; const server = testServerSetup(); @@ -227,12 +228,16 @@ const UnleashUiSetup: FC<{ path: string; pathTemplate: string }> = ({ - - {children}} - /> - + + + {children} + } + /> + + diff --git a/frontend/src/component/changeRequest/ChangeRequestPermissions.test.tsx b/frontend/src/component/changeRequest/ChangeRequestPermissions.test.tsx index a777c092dcb6..21e70c584e89 100644 --- a/frontend/src/component/changeRequest/ChangeRequestPermissions.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestPermissions.test.tsx @@ -10,6 +10,7 @@ import { FC } from 'react'; import { IPermission } from '../../interfaces/user'; import { SWRConfig } from 'swr'; import { ProjectMode } from '../project/Project/hooks/useProjectEnterpriseSettingsForm'; +import { StickyProvider } from 'component/common/Sticky/StickyProvider'; const server = testServerSetup(); @@ -186,9 +187,14 @@ const UnleashUiSetup: FC<{ path: string; pathTemplate: string }> = ({ - - - + + + + + diff --git a/frontend/src/component/common/Sticky/Sticky.test.tsx b/frontend/src/component/common/Sticky/Sticky.test.tsx new file mode 100644 index 000000000000..4b01c04bc4df --- /dev/null +++ b/frontend/src/component/common/Sticky/Sticky.test.tsx @@ -0,0 +1,112 @@ +import { render, screen, cleanup } from '@testing-library/react'; +import { Sticky } from './Sticky'; +import { IStickyContext, StickyContext } from './StickyContext'; +import { vi, expect } from 'vitest'; + +describe('Sticky component', () => { + let originalConsoleError: () => void; + let mockRegisterStickyItem: () => void; + let mockUnregisterStickyItem: () => void; + let mockGetTopOffset: () => number; + let mockContextValue: IStickyContext; + + beforeEach(() => { + originalConsoleError = console.error; + console.error = vi.fn(); + + mockRegisterStickyItem = vi.fn(); + mockUnregisterStickyItem = vi.fn(); + mockGetTopOffset = vi.fn(() => 10); + + mockContextValue = { + registerStickyItem: mockRegisterStickyItem, + unregisterStickyItem: mockUnregisterStickyItem, + getTopOffset: mockGetTopOffset, + stickyItems: [], + }; + }); + + afterEach(() => { + cleanup(); + console.error = originalConsoleError; + }); + + it('renders correctly within StickyContext', () => { + render( + + Content + , + ); + + expect(screen.getByText('Content')).toBeInTheDocument(); + }); + + it('throws error when not wrapped in StickyContext', () => { + console.error = vi.fn(); + + expect(() => render(Content)).toThrow( + 'Sticky component must be used within a StickyProvider', + ); + }); + + it('applies sticky positioning', () => { + render( + + Content + , + ); + + const stickyElement = screen.getByText('Content'); + expect(stickyElement).toHaveStyle({ position: 'sticky' }); + }); + + it('registers and unregisters sticky item on mount/unmount', () => { + const { unmount } = render( + + Content + , + ); + + expect(mockRegisterStickyItem).toHaveBeenCalledTimes(1); + + unmount(); + + expect(mockUnregisterStickyItem).toHaveBeenCalledTimes(1); + }); + + it('correctly sets the top value when mounted', async () => { + render( + + Content + , + ); + + const stickyElement = await screen.findByText('Content'); + expect(stickyElement).toHaveStyle({ top: '10px' }); + }); + + it('updates top offset when stickyItems changes', async () => { + const { rerender } = render( + + Content + , + ); + + let stickyElement = await screen.findByText('Content'); + expect(stickyElement).toHaveStyle({ top: '10px' }); + + const updatedMockContextValue = { + ...mockContextValue, + getTopOffset: vi.fn(() => 20), + }; + + rerender( + + Content + , + ); + + stickyElement = await screen.findByText('Content'); + expect(stickyElement).toHaveStyle({ top: '20px' }); + }); +}); diff --git a/frontend/src/component/common/Sticky/Sticky.tsx b/frontend/src/component/common/Sticky/Sticky.tsx new file mode 100644 index 000000000000..1a8d8c6b19bf --- /dev/null +++ b/frontend/src/component/common/Sticky/Sticky.tsx @@ -0,0 +1,80 @@ +import { + HTMLAttributes, + ReactNode, + useContext, + useEffect, + useRef, + useState, +} from 'react'; +import { StickyContext } from './StickyContext'; +import { styled } from '@mui/material'; + +const StyledSticky = styled('div', { + shouldForwardProp: (prop) => prop !== 'top', +})<{ top?: number }>(({ theme, top }) => ({ + position: 'sticky', + zIndex: theme.zIndex.sticky - 100, + ...(top !== undefined + ? { + '&': { + top, + }, + } + : {}), +})); + +interface IStickyProps extends HTMLAttributes { + children: ReactNode; +} + +export const Sticky = ({ children, ...props }: IStickyProps) => { + const context = useContext(StickyContext); + const ref = useRef(null); + const [initialTopOffset, setInitialTopOffset] = useState( + null, + ); + const [top, setTop] = useState(); + + if (!context) { + throw new Error( + 'Sticky component must be used within a StickyProvider', + ); + } + + const { registerStickyItem, unregisterStickyItem, getTopOffset } = context; + + useEffect(() => { + // We should only set the initial top offset once - when the component is mounted + // This value will be set based on the initial top that was set for this component + // After that, the top will be calculated based on the height of the previous sticky items + this initial top offset + if (ref.current && initialTopOffset === null) { + setInitialTopOffset( + parseInt(getComputedStyle(ref.current).getPropertyValue('top')), + ); + } + }, []); + + useEffect(() => { + // (Re)calculate the top offset based on the sticky items + setTop(getTopOffset(ref) + (initialTopOffset || 0)); + }, [getTopOffset, initialTopOffset]); + + useEffect(() => { + // We should register the sticky item when it is mounted and unregister it when it is unmounted + if (!ref.current) { + return; + } + + registerStickyItem(ref); + + return () => { + unregisterStickyItem(ref); + }; + }, [ref, registerStickyItem, unregisterStickyItem]); + + return ( + + {children} + + ); +}; diff --git a/frontend/src/component/common/Sticky/StickyContext.tsx b/frontend/src/component/common/Sticky/StickyContext.tsx new file mode 100644 index 000000000000..b6257162851c --- /dev/null +++ b/frontend/src/component/common/Sticky/StickyContext.tsx @@ -0,0 +1,12 @@ +import { RefObject, createContext } from 'react'; + +export interface IStickyContext { + stickyItems: RefObject[]; + registerStickyItem: (ref: RefObject) => void; + unregisterStickyItem: (ref: RefObject) => void; + getTopOffset: (ref: RefObject) => number; +} + +export const StickyContext = createContext( + undefined, +); diff --git a/frontend/src/component/common/Sticky/StickyProvider.test.tsx b/frontend/src/component/common/Sticky/StickyProvider.test.tsx new file mode 100644 index 000000000000..ebe51a24e5a9 --- /dev/null +++ b/frontend/src/component/common/Sticky/StickyProvider.test.tsx @@ -0,0 +1,160 @@ +import { render, cleanup } from '@testing-library/react'; +import { StickyProvider } from './StickyProvider'; +import { IStickyContext, StickyContext } from './StickyContext'; +import { expect } from 'vitest'; + +const defaultGetBoundingClientRect = { + width: 0, + height: 0, + top: 0, + left: 0, + bottom: 0, + right: 0, + x: 0, + y: 0, + toJSON() {}, +}; + +describe('StickyProvider component', () => { + afterEach(cleanup); + + it('provides the sticky context with expected functions', () => { + let receivedContext = null; + render( + + + {(context) => { + receivedContext = context; + return null; + }} + + , + ); + + expect(receivedContext).not.toBeNull(); + expect(receivedContext).toHaveProperty('stickyItems'); + expect(receivedContext).toHaveProperty('registerStickyItem'); + expect(receivedContext).toHaveProperty('unregisterStickyItem'); + expect(receivedContext).toHaveProperty('getTopOffset'); + }); + + it('registers and unregisters sticky items', () => { + let contextValues: IStickyContext | undefined; + const refMock = { current: document.createElement('div') }; + + const { rerender } = render( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + contextValues?.registerStickyItem(refMock); + rerender( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + expect(contextValues?.stickyItems).toContain(refMock); + + contextValues?.unregisterStickyItem(refMock); + rerender( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + expect(contextValues?.stickyItems).not.toContain(refMock); + }); + + it('sorts sticky items based on their DOM position', () => { + let contextValues: IStickyContext | undefined; + + const refMockA = { current: document.createElement('div') }; + const refMockB = { current: document.createElement('div') }; + + refMockA.current.getBoundingClientRect = () => ({ + ...defaultGetBoundingClientRect, + top: 200, + }); + refMockB.current.getBoundingClientRect = () => ({ + ...defaultGetBoundingClientRect, + top: 100, + }); + + render( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + contextValues?.registerStickyItem(refMockA); + contextValues?.registerStickyItem(refMockB); + + expect(contextValues?.stickyItems[0]).toBe(refMockB); + expect(contextValues?.stickyItems[1]).toBe(refMockA); + }); + + it('calculates top offset correctly', () => { + let contextValues: IStickyContext | undefined; + const refMockA = { current: document.createElement('div') }; + const refMockB = { current: document.createElement('div') }; + + refMockA.current.getBoundingClientRect = () => ({ + ...defaultGetBoundingClientRect, + height: 100, + }); + + refMockB.current.getBoundingClientRect = () => ({ + ...defaultGetBoundingClientRect, + height: 200, + }); + + const { rerender } = render( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + contextValues?.registerStickyItem(refMockA); + contextValues?.registerStickyItem(refMockB); + rerender( + + + {(context) => { + contextValues = context; + return null; + }} + + , + ); + + const topOffset = contextValues?.getTopOffset(refMockB); + expect(topOffset).toBe(100); + }); +}); diff --git a/frontend/src/component/common/Sticky/StickyProvider.tsx b/frontend/src/component/common/Sticky/StickyProvider.tsx new file mode 100644 index 000000000000..7e61d6fb3b2b --- /dev/null +++ b/frontend/src/component/common/Sticky/StickyProvider.tsx @@ -0,0 +1,133 @@ +import { useState, useCallback, ReactNode, RefObject, useEffect } from 'react'; +import { StickyContext } from './StickyContext'; + +interface IStickyProviderProps { + children: ReactNode; +} + +export const StickyProvider = ({ children }: IStickyProviderProps) => { + const [stickyItems, setStickyItems] = useState[]>( + [], + ); + const [resizeListeners, setResizeListeners] = useState( + new Set>(), + ); + + const registerStickyItem = useCallback( + (item: RefObject) => { + setStickyItems((prevItems) => { + // We should only register a new item if it is not already registered + if (!prevItems.includes(item)) { + // Register resize listener for the item + registerResizeListener(item); + + const newItems = [...prevItems, item]; + // We should try to sort the items by their top on the viewport, so that their order in the DOM is the same as their order in the array + return newItems.sort((a, b) => { + const elementA = a.current; + const elementB = b.current; + if (elementA && elementB) { + return ( + elementA.getBoundingClientRect().top - + elementB.getBoundingClientRect().top + ); + } + return 0; + }); + } + + return prevItems; + }); + }, + [], + ); + + const unregisterStickyItem = useCallback( + (ref: RefObject) => { + unregisterResizeListener(ref); + setStickyItems((prev) => prev.filter((item) => item !== ref)); + }, + [], + ); + + const registerResizeListener = useCallback( + (ref: RefObject) => { + setResizeListeners((prev) => new Set(prev).add(ref)); + }, + [], + ); + + const unregisterResizeListener = useCallback( + (ref: RefObject) => { + setResizeListeners((prev) => { + const newListeners = new Set(prev); + newListeners.delete(ref); + return newListeners; + }); + }, + [], + ); + + const getTopOffset = useCallback( + (ref: RefObject) => { + if (!stickyItems.some((item) => item === ref)) { + // Return 0 in case the item is not registered yet + return 0; + } + const stickyItemsUpToOurItem = stickyItems.slice( + 0, + stickyItems.findIndex((item) => item === ref), + ); + return stickyItemsUpToOurItem.reduce((acc, item) => { + if (item === ref) { + // We should not include the current item in the calculation + return acc; + } + + // Accumulate the height of all sticky items above our item + const itemHeight = + item.current?.getBoundingClientRect().height || 0; + + return acc + itemHeight; + }, 0); + }, + [stickyItems], + ); + + useEffect(() => { + const resizeObserver = new ResizeObserver(() => { + // We should recalculate top offsets whenever there's a resize + // This will trigger the dependency in `getTopOffset` and recalculate the top offsets in the Sticky components + setStickyItems((prev) => [...prev]); + }); + + resizeListeners.forEach((item) => { + if (item.current) { + resizeObserver.observe(item.current); + } + }); + + return () => { + if (resizeListeners.size > 0) { + resizeListeners.forEach((item) => { + if (item.current) { + resizeObserver.unobserve(item.current); + } + }); + } + }; + }, [resizeListeners]); + + return ( + + {children} + + ); +}; diff --git a/frontend/src/component/demo/DemoBanner/DemoBanner.tsx b/frontend/src/component/demo/DemoBanner/DemoBanner.tsx index 5d4fdb4349d0..2f09b002eb0d 100644 --- a/frontend/src/component/demo/DemoBanner/DemoBanner.tsx +++ b/frontend/src/component/demo/DemoBanner/DemoBanner.tsx @@ -1,9 +1,8 @@ import { Button, styled } from '@mui/material'; +import { Sticky } from 'component/common/Sticky/Sticky'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; -const StyledBanner = styled('div')(({ theme }) => ({ - position: 'sticky', - top: 0, +const StyledBanner = styled(Sticky)(({ theme }) => ({ zIndex: theme.zIndex.sticky, display: 'flex', gap: theme.spacing(1), diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanel.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanel.tsx index 0853ead22718..2b2587018141 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanel.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanel.tsx @@ -5,9 +5,9 @@ import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { FeatureOverviewSidePanelDetails } from './FeatureOverviewSidePanelDetails/FeatureOverviewSidePanelDetails'; import { FeatureOverviewSidePanelEnvironmentSwitches } from './FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitches'; import { FeatureOverviewSidePanelTags } from './FeatureOverviewSidePanelTags/FeatureOverviewSidePanelTags'; +import { Sticky } from 'component/common/Sticky/Sticky'; -const StyledContainer = styled('div')(({ theme }) => ({ - position: 'sticky', +const StyledContainer = styled(Sticky)(({ theme }) => ({ top: theme.spacing(2), borderRadius: theme.shape.borderRadiusLarge, backgroundColor: theme.palette.background.paper, diff --git a/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx b/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx index 95f3d9be6c75..1ef42e91301b 100644 --- a/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx +++ b/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx @@ -5,6 +5,7 @@ import { ChangeRequestSidebar } from 'component/changeRequest/ChangeRequestSideb import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests'; import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; import { changesCount } from 'component/changeRequest/changesCount'; +import { Sticky } from 'component/common/Sticky/Sticky'; interface IDraftBannerProps { project: string; @@ -98,10 +99,7 @@ const DraftBannerContent: FC<{ ); }; -const StickyBanner = styled(Box)(({ theme }) => ({ - position: 'sticky', - top: -1, - zIndex: 250 /* has to lower than header.zIndex and higher than body.zIndex */, +const StickyBanner = styled(Sticky)(({ theme }) => ({ borderTop: `1px solid ${theme.palette.warning.border}`, borderBottom: `1px solid ${theme.palette.warning.border}`, color: theme.palette.warning.contrastText, diff --git a/frontend/src/component/maintenance/MaintenanceBanner.tsx b/frontend/src/component/maintenance/MaintenanceBanner.tsx index 2298aae03ab0..23041dbc35ae 100644 --- a/frontend/src/component/maintenance/MaintenanceBanner.tsx +++ b/frontend/src/component/maintenance/MaintenanceBanner.tsx @@ -1,5 +1,6 @@ import { styled } from '@mui/material'; import { ErrorOutlineRounded } from '@mui/icons-material'; +import { Sticky } from 'component/common/Sticky/Sticky'; const StyledErrorRoundedIcon = styled(ErrorOutlineRounded)(({ theme }) => ({ color: theme.palette.error.main, @@ -8,7 +9,7 @@ const StyledErrorRoundedIcon = styled(ErrorOutlineRounded)(({ theme }) => ({ marginRight: theme.spacing(1), })); -const StyledDiv = styled('div')(({ theme }) => ({ +const StyledDiv = styled(Sticky)(({ theme }) => ({ display: 'flex', fontSize: theme.fontSizes.smallBody, justifyContent: 'center', @@ -18,8 +19,6 @@ const StyledDiv = styled('div')(({ theme }) => ({ height: '65px', borderBottom: `1px solid ${theme.palette.error.border}`, whiteSpace: 'pre-wrap', - position: 'sticky', - top: 0, zIndex: theme.zIndex.sticky - 100, })); diff --git a/frontend/src/component/messageBanners/externalMessageBanners/ExternalMessageBanners.tsx b/frontend/src/component/messageBanners/externalMessageBanners/ExternalMessageBanners.tsx deleted file mode 100644 index ee991ca89c96..000000000000 --- a/frontend/src/component/messageBanners/externalMessageBanners/ExternalMessageBanners.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { MessageBanner } from 'component/messageBanners/MessageBanner/MessageBanner'; -import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; -import { useVariant } from 'hooks/useVariant'; -import { IMessageBanner } from 'interfaces/messageBanner'; - -export const ExternalMessageBanners = () => { - const { uiConfig } = useUiConfig(); - - const messageBannerVariant = - useVariant( - uiConfig.flags.messageBanner, - ) || []; - - const messageBanners: IMessageBanner[] = Array.isArray(messageBannerVariant) - ? messageBannerVariant - : [messageBannerVariant]; - - return ( - <> - {messageBanners.map((messageBanner) => ( - - ))} - - ); -}; diff --git a/frontend/src/component/messageBanners/internalMessageBanners/InternalMessageBanners.tsx b/frontend/src/component/messageBanners/internalMessageBanners/InternalMessageBanners.tsx deleted file mode 100644 index f84d17c597fa..000000000000 --- a/frontend/src/component/messageBanners/internalMessageBanners/InternalMessageBanners.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import { MessageBanner } from 'component/messageBanners/MessageBanner/MessageBanner'; -import { useMessageBanners } from 'hooks/api/getters/useMessageBanners/useMessageBanners'; - -export const InternalMessageBanners = () => { - const { messageBanners } = useMessageBanners(); - - return ( - <> - {messageBanners.map((messageBanner) => ( - - ))} - - ); -}; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx index d181431a1f65..3f63cbd41ef7 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx @@ -3,6 +3,7 @@ import { render } from 'utils/testRenderer'; import React from 'react'; import { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; import { PlaygroundResultFeatureStrategyList } from './PlaygroundResultFeatureStrategyList'; +import { vi } from 'vitest'; const testCases = [ { @@ -62,8 +63,72 @@ const testCases = [ expectedText: 'If environment was enabled, then this feature toggle would be TRUE with strategies evaluated like so:', }, + { + name: 'Has disabled strategies and is enabled in environment', + feature: { + strategies: { + result: true, + data: [ + { + name: 'default', + parameters: {}, + result: { enabled: true, evaluationStatus: 'complete' }, + }, + { + name: 'default', + parameters: {}, + disabled: true, + result: { + enabled: 'unknown', + evaluationStatus: 'unevaluated', + }, + }, + ], + }, + isEnabledInCurrentEnvironment: true, + hasUnsatisfiedDependency: false, + } as PlaygroundFeatureSchema, + expectedText: + 'Disabled strategies are not evaluated for the overall result.', + }, + { + name: 'Has disabled strategies and is disabled in environment', + feature: { + strategies: { + result: true, + data: [ + { + name: 'default', + parameters: {}, + result: { enabled: true, evaluationStatus: 'complete' }, + }, + { + name: 'default', + parameters: {}, + disabled: true, + result: { + enabled: 'unknown', + evaluationStatus: 'unevaluated', + }, + }, + ], + }, + isEnabledInCurrentEnvironment: false, + hasUnsatisfiedDependency: false, + } as PlaygroundFeatureSchema, + expectedText: + 'Disabled strategies are not evaluated for the overall result.', + }, ]; +vi.mock('../../../../../../hooks/useUiFlag', () => ({ + useUiFlag: vi.fn().mockImplementation(() => true), +})); + +afterAll(() => { + vi.clearAllMocks(); +}); + testCases.forEach(({ name, feature, expectedText }) => { test(name, async () => { render( diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx index c15efe19a917..2adbcdb49d2f 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx @@ -5,6 +5,7 @@ import { import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; import { Alert } from '@mui/material'; +import { useUiFlag } from '../../../../../../hooks/useUiFlag'; interface PlaygroundResultFeatureStrategyListProps { feature: PlaygroundFeatureSchema; @@ -15,6 +16,17 @@ export const PlaygroundResultFeatureStrategyList = ({ feature, input, }: PlaygroundResultFeatureStrategyListProps) => { + const playgroundImprovements = useUiFlag('playgroundImprovements'); + const enabledStrategies = feature.strategies?.data?.filter( + (strategy) => !strategy.disabled, + ); + const disabledStrategies = feature.strategies?.data?.filter( + (strategy) => strategy.disabled, + ); + + const showDisabledStrategies = + playgroundImprovements && disabledStrategies?.length > 0; + return ( <> } elseShow={ - + <> + + + } + /> + } /> diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/FeatureStrategyItem.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/FeatureStrategyItem.tsx index 9c6a9f87e4f1..63d07dd1f276 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/FeatureStrategyItem.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/FeatureStrategyItem.tsx @@ -4,6 +4,8 @@ import { PlaygroundStrategySchema, PlaygroundRequestSchema } from 'openapi'; import { StrategyExecution } from './StrategyExecution/StrategyExecution'; import { StrategyItemContainer } from 'component/common/StrategyItemContainer/StrategyItemContainer'; import { objectId } from 'utils/objectId'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { DisabledStrategyExecution } from './StrategyExecution/DisabledStrategyExecution'; interface IFeatureStrategyItemProps { strategy: PlaygroundStrategySchema; @@ -19,7 +21,8 @@ export const FeatureStrategyItem = ({ const { result } = strategy; const theme = useTheme(); const label = - result.evaluationStatus === 'incomplete' + result.evaluationStatus === 'incomplete' || + result.evaluationStatus === 'unevaluated' ? 'Unevaluated' : result.enabled ? 'True' @@ -28,9 +31,10 @@ export const FeatureStrategyItem = ({ return ( } > - + } + elseShow={ + + } /> ); diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/ConstraintExecution/ConstraintExecutionWithoutResults.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/ConstraintExecution/ConstraintExecutionWithoutResults.tsx new file mode 100644 index 000000000000..bcfbc342c070 --- /dev/null +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/ConstraintExecution/ConstraintExecutionWithoutResults.tsx @@ -0,0 +1,43 @@ +import { Fragment, VFC } from 'react'; +import { + PlaygroundConstraintSchema, + PlaygroundRequestSchema, + PlaygroundStrategySchemaResultAnyOfEvaluationStatus, +} from 'openapi'; +import { objectId } from 'utils/objectId'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { styled } from '@mui/material'; +import { ConstraintAccordionView } from 'component/common/ConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView'; +import { ConstraintError } from './ConstraintError/ConstraintError'; +import { ConstraintOk } from './ConstraintOk/ConstraintOk'; + +interface IConstraintExecutionWithoutResultsProps { + constraints?: PlaygroundConstraintSchema[]; +} + +export const ConstraintExecutionWrapper = styled('div')(() => ({ + width: '100%', + display: 'flex', + flexDirection: 'column', +})); + +export const ConstraintExecutionWithoutResults: VFC< + IConstraintExecutionWithoutResultsProps +> = ({ constraints }) => { + if (!constraints) return null; + + return ( + + {constraints?.map((constraint, index) => ( + + 0} + show={} + /> + + + ))} + + ); +}; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/DisabledStrategyExecution.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/DisabledStrategyExecution.tsx new file mode 100644 index 000000000000..f195bd8f0f9a --- /dev/null +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/DisabledStrategyExecution.tsx @@ -0,0 +1,85 @@ +import { Fragment, VFC } from 'react'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { styled } from '@mui/material'; +import { PlaygroundRequestSchema, PlaygroundStrategySchema } from 'openapi'; +import { ConstraintExecution } from './ConstraintExecution/ConstraintExecution'; +import { SegmentExecution } from './SegmentExecution/SegmentExecution'; +import { PlaygroundResultStrategyExecutionParameters } from './StrategyExecutionParameters/StrategyExecutionParameters'; +import { CustomStrategyParams } from './CustomStrategyParams/CustomStrategyParams'; +import { formattedStrategyNames } from 'utils/strategyNames'; +import { StyledBoxSummary } from './StrategyExecution.styles'; +import { Badge } from 'component/common/Badge/Badge'; +import { ConstraintExecutionWithoutResults } from './ConstraintExecution/ConstraintExecutionWithoutResults'; +import { SegmentExecutionWithoutResult } from './SegmentExecution/SegmentExecutionWithoutResult'; + +interface IDisabledStrategyExecutionProps { + strategyResult: PlaygroundStrategySchema; + percentageFill?: string; + input?: PlaygroundRequestSchema; +} + +const StyledStrategyExecutionWrapper = styled('div')(({ theme }) => ({ + padding: theme.spacing(0), +})); + +export const DisabledStrategyExecution: VFC = + ({ strategyResult, input, percentageFill }) => { + const { name, constraints, segments, parameters } = strategyResult; + + const hasSegments = Boolean(segments && segments.length > 0); + const hasConstraints = Boolean(constraints && constraints?.length > 0); + const hasExecutionParameters = + name !== 'default' && + Object.keys(formattedStrategyNames).includes(name); + const hasCustomStrategyParameters = + Object.keys(parameters).length > 0 && + strategyResult.result.evaluationStatus === 'incomplete'; // Use of custom strategy can be more explicit from the API + + if (!parameters) { + return null; + } + + const items = [ + hasSegments && ( + + ), + hasConstraints && ( + + ), + hasExecutionParameters && ( + + ), + hasCustomStrategyParameters && ( + + ), + name === 'default' && ( + + The standard strategy is ON{' '} + for all users. + + ), + ].filter(Boolean); + + return ( + + {items.map((item, index) => ( + // biome-ignore lint/suspicious/noArrayIndexKey: + + 0} + show={} + /> + {item} + + ))} + + ); + }; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/SegmentExecution/SegmentExecutionWithoutResult.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/SegmentExecution/SegmentExecutionWithoutResult.tsx new file mode 100644 index 000000000000..33e40ac84c38 --- /dev/null +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/SegmentExecution/SegmentExecutionWithoutResult.tsx @@ -0,0 +1,47 @@ +import { Fragment, VFC } from 'react'; +import { PlaygroundSegmentSchema, PlaygroundRequestSchema } from 'openapi'; +import { ConstraintExecution } from '../ConstraintExecution/ConstraintExecution'; +import { CancelOutlined } from '@mui/icons-material'; +import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { styled, Typography } from '@mui/material'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { SegmentItem } from 'component/common/SegmentItem/SegmentItem'; +import { ConstraintExecutionWithoutResults } from '../ConstraintExecution/ConstraintExecutionWithoutResults'; + +interface ISegmentExecutionWithoutResultProps { + segments?: PlaygroundSegmentSchema[]; +} + +export const SegmentExecutionWithoutResult: VFC< + ISegmentExecutionWithoutResultProps +> = ({ segments }) => { + if (!segments) return null; + + return ( + <> + {segments.map((segment, index) => ( + + + } + isExpanded + /> + = 0 && + segments.length > 1 && + // Don't add if it's the last segment item + index !== segments.length - 1 + } + show={} + /> + + ))} + + ); +}; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/StrategyExecution.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/StrategyExecution.tsx index 78d542ae8410..7a0214f36905 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/StrategyExecution.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/StrategyItem/StrategyExecution/StrategyExecution.tsx @@ -10,6 +10,8 @@ import { CustomStrategyParams } from './CustomStrategyParams/CustomStrategyParam import { formattedStrategyNames } from 'utils/strategyNames'; import { StyledBoxSummary } from './StrategyExecution.styles'; import { Badge } from 'component/common/Badge/Badge'; +import { ConstraintExecutionWithoutResults } from './ConstraintExecution/ConstraintExecutionWithoutResults'; +import { SegmentExecutionWithoutResult } from './SegmentExecution/SegmentExecutionWithoutResult'; interface IStrategyExecutionProps { strategyResult: PlaygroundStrategySchema; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/playgroundResultStrategyLists.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/playgroundResultStrategyLists.tsx index 308a7d8beb4c..aa08a59f9318 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/playgroundResultStrategyLists.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/playgroundResultStrategyLists.tsx @@ -8,6 +8,7 @@ import { import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { FeatureStrategyItem } from './StrategyItem/FeatureStrategyItem'; import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { useUiFlag } from '../../../../../../../hooks/useUiFlag'; const StyledAlertWrapper = styled('div')(({ theme }) => ({ display: 'flex', @@ -31,20 +32,38 @@ const StyledAlert = styled(Alert)(({ theme }) => ({ interface PlaygroundResultStrategyListProps { strategies: PlaygroundStrategySchema[]; input?: PlaygroundRequestSchema; + titlePrefix?: string; + infoText?: string; } +const StyledSubtitle = styled(Typography)(({ theme }) => ({ + margin: theme.spacing(2, 1, 2, 0), + color: 'text.secondary', +})); + export const PlaygroundResultStrategyLists = ({ strategies, input, + titlePrefix, + infoText, }: PlaygroundResultStrategyListProps) => ( 0} show={ <> - {`Strategies (${strategies?.length})`} + {`${ + titlePrefix + ? titlePrefix.concat(' strategies') + : 'Strategies' + } (${strategies?.length})`} + + {infoText} + + } + /> {strategies?.map((strategy, index) => ( @@ -91,6 +110,17 @@ export const WrappedPlaygroundResultStrategyList = ({ feature, input, }: IWrappedPlaygroundResultStrategyListProps) => { + const playgroundImprovements = useUiFlag('playgroundImprovements'); + const enabledStrategies = feature.strategies?.data?.filter( + (strategy) => !strategy.disabled, + ); + const disabledStrategies = feature.strategies?.data?.filter( + (strategy) => strategy.disabled, + ); + + const showDisabledStrategies = + playgroundImprovements && disabledStrategies?.length > 0; + return ( @@ -100,10 +130,26 @@ export const WrappedPlaygroundResultStrategyList = ({ + + + + } + /> ); }; diff --git a/frontend/src/hooks/api/actions/useMessageBannersApi/useMessageBannersApi.ts b/frontend/src/hooks/api/actions/useMessageBannersApi/useMessageBannersApi.ts index 384cba240269..c65e93cbb998 100644 --- a/frontend/src/hooks/api/actions/useMessageBannersApi/useMessageBannersApi.ts +++ b/frontend/src/hooks/api/actions/useMessageBannersApi/useMessageBannersApi.ts @@ -1,27 +1,22 @@ -import { IInternalMessageBanner } from 'interfaces/messageBanner'; +import { IInternalBanner } from 'interfaces/banner'; import useAPI from '../useApi/useApi'; -const ENDPOINT = 'api/admin/message-banners'; +const ENDPOINT = 'api/admin/banners'; -type AddOrUpdateMessageBanner = Omit< - IInternalMessageBanner, - 'id' | 'createdAt' ->; +type AddOrUpdateBanner = Omit; -export const useMessageBannersApi = () => { +export const useBannersApi = () => { const { loading, makeRequest, createRequest, errors } = useAPI({ propagateErrors: true, }); - const addMessageBanner = async ( - messageBanner: AddOrUpdateMessageBanner, - ) => { - const requestId = 'addMessageBanner'; + const addBanner = async (banner: AddOrUpdateBanner) => { + const requestId = 'addBanner'; const req = createRequest( ENDPOINT, { method: 'POST', - body: JSON.stringify(messageBanner), + body: JSON.stringify(banner), }, requestId, ); @@ -30,16 +25,16 @@ export const useMessageBannersApi = () => { return response.json(); }; - const updateMessageBanner = async ( - messageBannerId: number, - messageBanner: AddOrUpdateMessageBanner, + const updateBanner = async ( + bannerId: number, + banner: AddOrUpdateBanner, ) => { - const requestId = 'updateMessageBanner'; + const requestId = 'updateBanner'; const req = createRequest( - `${ENDPOINT}/${messageBannerId}`, + `${ENDPOINT}/${bannerId}`, { method: 'PUT', - body: JSON.stringify(messageBanner), + body: JSON.stringify(banner), }, requestId, ); @@ -47,10 +42,10 @@ export const useMessageBannersApi = () => { await makeRequest(req.caller, req.id); }; - const removeMessageBanner = async (messageBannerId: number) => { - const requestId = 'removeMessageBanner'; + const removeBanner = async (bannerId: number) => { + const requestId = 'removeBanner'; const req = createRequest( - `${ENDPOINT}/${messageBannerId}`, + `${ENDPOINT}/${bannerId}`, { method: 'DELETE' }, requestId, ); @@ -59,9 +54,9 @@ export const useMessageBannersApi = () => { }; return { - addMessageBanner, - updateMessageBanner, - removeMessageBanner, + addBanner, + updateBanner, + removeBanner, errors, loading, }; diff --git a/frontend/src/hooks/api/getters/useMessageBanners/useMessageBanners.ts b/frontend/src/hooks/api/getters/useBanners/useBanners.ts similarity index 62% rename from frontend/src/hooks/api/getters/useMessageBanners/useMessageBanners.ts rename to frontend/src/hooks/api/getters/useBanners/useBanners.ts index 0bfdbd92fa6e..f6e6399fb649 100644 --- a/frontend/src/hooks/api/getters/useMessageBanners/useMessageBanners.ts +++ b/frontend/src/hooks/api/getters/useBanners/useBanners.ts @@ -4,25 +4,24 @@ import handleErrorResponses from '../httpErrorResponseHandler'; import { useConditionalSWR } from '../useConditionalSWR/useConditionalSWR'; import useUiConfig from '../useUiConfig/useUiConfig'; import { useUiFlag } from 'hooks/useUiFlag'; -import { IInternalMessageBanner } from 'interfaces/messageBanner'; +import { IInternalBanner } from 'interfaces/banner'; -const ENDPOINT = 'api/admin/message-banners'; +const ENDPOINT = 'api/admin/banners'; -export const useMessageBanners = () => { +export const useBanners = () => { const { isEnterprise } = useUiConfig(); - const internalMessageBanners = useUiFlag('internalMessageBanners'); + const bannersEnabled = useUiFlag('banners'); const { data, error, mutate } = useConditionalSWR( - isEnterprise() && internalMessageBanners, - { messageBanners: [] }, + isEnterprise() && bannersEnabled, + { banners: [] }, formatApiPath(ENDPOINT), fetcher, ); return useMemo( () => ({ - messageBanners: (data?.messageBanners ?? - []) as IInternalMessageBanner[], + banners: (data?.banners ?? []) as IInternalBanner[], loading: !error && !data, refetch: () => mutate(), error, @@ -33,6 +32,6 @@ export const useMessageBanners = () => { const fetcher = (path: string) => { return fetch(path) - .then(handleErrorResponses('Message Banners')) + .then(handleErrorResponses('Banners')) .then((res) => res.json()); }; diff --git a/frontend/src/hooks/usePlausibleTracker.ts b/frontend/src/hooks/usePlausibleTracker.ts index 97a80cd18fd6..00f1a60a000a 100644 --- a/frontend/src/hooks/usePlausibleTracker.ts +++ b/frontend/src/hooks/usePlausibleTracker.ts @@ -15,7 +15,7 @@ export type CustomEvents = | 'change_request' | 'favorite' | 'maintenance' - | 'message_banner' + | 'banner' | 'hidden_environment' | 'project_overview' | 'suggest_tags' diff --git a/frontend/src/index.tsx b/frontend/src/index.tsx index 886c257df127..442313b697cf 100644 --- a/frontend/src/index.tsx +++ b/frontend/src/index.tsx @@ -13,6 +13,7 @@ import { FeedbackCESProvider } from 'component/feedback/FeedbackCESContext/Feedb import { AnnouncerProvider } from 'component/common/Announcer/AnnouncerProvider/AnnouncerProvider'; import { InstanceStatus } from 'component/common/InstanceStatus/InstanceStatus'; import { UIProviderContainer } from 'component/providers/UIProvider/UIProviderContainer'; +import { StickyProvider } from 'component/common/Sticky/StickyProvider'; window.global ||= window; @@ -23,10 +24,12 @@ ReactDOM.render( - - - - + + + + + + diff --git a/frontend/src/interfaces/messageBanner.ts b/frontend/src/interfaces/banner.ts similarity index 77% rename from frontend/src/interfaces/messageBanner.ts rename to frontend/src/interfaces/banner.ts index 4cd168c88a24..c7bbb8a38f6e 100644 --- a/frontend/src/interfaces/messageBanner.ts +++ b/frontend/src/interfaces/banner.ts @@ -1,6 +1,6 @@ export type BannerVariant = 'warning' | 'info' | 'error' | 'success'; -export interface IMessageBanner { +export interface IBanner { message: string; variant?: BannerVariant; sticky?: boolean; @@ -12,7 +12,7 @@ export interface IMessageBanner { dialog?: string; } -export interface IInternalMessageBanner extends IMessageBanner { +export interface IInternalBanner extends IBanner { id: number; enabled: boolean; createdAt: string; diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index e227653e3d65..2d825365c1e3 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -47,6 +47,7 @@ export type UiFlags = { embedProxyFrontend?: boolean; maintenanceMode?: boolean; messageBanner?: Variant; + banner?: Variant; featuresExportImport?: boolean; caseInsensitiveInOperators?: boolean; proPlanAutoCharge?: boolean; @@ -69,7 +70,7 @@ export type UiFlags = { accessOverview?: boolean; datadogJsonTemplate?: boolean; dependentFeatures?: boolean; - internalMessageBanners?: boolean; + banners?: boolean; disableEnvsOnRevive?: boolean; playgroundImprovements?: boolean; }; diff --git a/frontend/src/setupTests.ts b/frontend/src/setupTests.ts index d15f1080f900..25e7a167846f 100644 --- a/frontend/src/setupTests.ts +++ b/frontend/src/setupTests.ts @@ -2,4 +2,14 @@ import '@testing-library/jest-dom'; import 'whatwg-fetch'; import 'regenerator-runtime'; +class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +if (!window.ResizeObserver) { + window.ResizeObserver = ResizeObserver; +} + process.env.TZ = 'UTC'; diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index a2a9656ca223..8475acd9645f 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -77,6 +77,7 @@ exports[`should create default config 1`] = ` "flags": { "accessOverview": false, "anonymiseEventLog": false, + "banners": false, "caseInsensitiveInOperators": false, "customRootRolesKillSwitch": false, "datadogJsonTemplate": false, @@ -93,7 +94,6 @@ exports[`should create default config 1`] = ` "featuresExportImport": true, "filterInvalidClientMetrics": false, "googleAuthEnabled": false, - "internalMessageBanners": false, "lastSeenByEnvironment": false, "maintenanceMode": false, "messageBanner": { @@ -122,6 +122,7 @@ exports[`should create default config 1`] = ` "experiments": { "accessOverview": false, "anonymiseEventLog": false, + "banners": false, "caseInsensitiveInOperators": false, "customRootRolesKillSwitch": false, "datadogJsonTemplate": false, @@ -138,7 +139,6 @@ exports[`should create default config 1`] = ` "featuresExportImport": true, "filterInvalidClientMetrics": false, "googleAuthEnabled": false, - "internalMessageBanners": false, "lastSeenByEnvironment": false, "maintenanceMode": false, "messageBanner": { diff --git a/src/lib/addons/feature-event-formatter-md.ts b/src/lib/addons/feature-event-formatter-md.ts index 81c624de4134..f975a91679f9 100644 --- a/src/lib/addons/feature-event-formatter-md.ts +++ b/src/lib/addons/feature-event-formatter-md.ts @@ -41,9 +41,9 @@ import { GROUP_UPDATED, IConstraint, IEvent, - MESSAGE_BANNER_CREATED, - MESSAGE_BANNER_DELETED, - MESSAGE_BANNER_UPDATED, + BANNER_CREATED, + BANNER_DELETED, + BANNER_UPDATED, PROJECT_CREATED, PROJECT_DELETED, SEGMENT_CREATED, @@ -232,16 +232,16 @@ const EVENT_MAP: Record = { action: '*{{user}}* updated group *{{event.preData.name}}*', path: '/admin/groups', }, - [MESSAGE_BANNER_CREATED]: { - action: '*{{user}}* created message banner *{{event.data.message}}*', + [BANNER_CREATED]: { + action: '*{{user}}* created banner *{{event.data.message}}*', path: '/admin/message-banners', }, - [MESSAGE_BANNER_DELETED]: { - action: '*{{user}}* deleted message banner *{{event.preData.message}}*', + [BANNER_DELETED]: { + action: '*{{user}}* deleted banner *{{event.preData.message}}*', path: '/admin/message-banners', }, - [MESSAGE_BANNER_UPDATED]: { - action: '*{{user}}* updated message banner *{{event.preData.message}}*', + [BANNER_UPDATED]: { + action: '*{{user}}* updated banner *{{event.preData.message}}*', path: '/admin/message-banners', }, [PROJECT_CREATED]: { diff --git a/src/lib/addons/slack-app-definition.ts b/src/lib/addons/slack-app-definition.ts index 4ba83275a02c..1d97d2e96ffd 100644 --- a/src/lib/addons/slack-app-definition.ts +++ b/src/lib/addons/slack-app-definition.ts @@ -49,9 +49,9 @@ import { SERVICE_ACCOUNT_DELETED, SERVICE_ACCOUNT_UPDATED, GROUP_DELETED, - MESSAGE_BANNER_CREATED, - MESSAGE_BANNER_UPDATED, - MESSAGE_BANNER_DELETED, + BANNER_CREATED, + BANNER_UPDATED, + BANNER_DELETED, } from '../types/events'; import { IAddonDefinition } from '../types/model'; @@ -127,9 +127,9 @@ const slackAppDefinition: IAddonDefinition = { GROUP_CREATED, GROUP_DELETED, GROUP_UPDATED, - MESSAGE_BANNER_CREATED, - MESSAGE_BANNER_UPDATED, - MESSAGE_BANNER_DELETED, + BANNER_CREATED, + BANNER_UPDATED, + BANNER_DELETED, PROJECT_CREATED, PROJECT_DELETED, SEGMENT_CREATED, diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts index 7cc740945fc2..92a8d650f0e5 100644 --- a/src/lib/features/dependent-features/dependent-features-service.ts +++ b/src/lib/features/dependent-features/dependent-features-service.ts @@ -96,18 +96,26 @@ export class DependentFeaturesService { ); } - const [children, parentExists, sameProject] = await Promise.all([ - this.dependentFeaturesReadModel.getChildren([child]), - this.featuresReadModel.featureExists(parent), - this.featuresReadModel.featuresInTheSameProject(child, parent), - ]); - - if (children.length > 0) { + const [grandchildren, grandparents, parentExists, sameProject] = + await Promise.all([ + this.dependentFeaturesReadModel.getChildren([child]), + this.dependentFeaturesReadModel.getParents(parent), + this.featuresReadModel.featureExists(parent), + this.featuresReadModel.featuresInTheSameProject(child, parent), + ]); + + if (grandchildren.length > 0) { throw new InvalidOperationError( 'Transitive dependency detected. Cannot add a dependency to the feature that other features depend on.', ); } + if (grandparents.length > 0) { + throw new InvalidOperationError( + 'Transitive dependency detected. Cannot add a dependency to the feature that has parent dependency.', + ); + } + if (!parentExists) { throw new InvalidOperationError( `No active feature ${parent} exists`, 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 7aeb78867f4e..468a36906233 100644 --- a/src/lib/features/dependent-features/dependent.features.e2e.test.ts +++ b/src/lib/features/dependent-features/dependent.features.e2e.test.ts @@ -138,7 +138,7 @@ test('should add and delete feature dependencies', async () => { ]); }); -test('should not allow to add a parent dependency to a feature that already has children', async () => { +test('should not allow to add grandparent', async () => { const grandparent = uuidv4(); const parent = uuidv4(); const child = uuidv4(); @@ -158,8 +158,28 @@ test('should not allow to add a parent dependency to a feature that already has ); }); -test('should not allow to add non-existent parent dependency', async () => { +test('should not allow to add grandchild', async () => { const grandparent = uuidv4(); + const parent = uuidv4(); + const child = uuidv4(); + await app.createFeature(grandparent); + await app.createFeature(parent); + await app.createFeature(child); + + await addFeatureDependency(parent, { + feature: grandparent, + }); + + await addFeatureDependency( + child, + { + feature: parent, + }, + 403, + ); +}); + +test('should not allow to add non-existent parent dependency', async () => { const parent = uuidv4(); const child = uuidv4(); await app.createFeature(child); diff --git a/src/lib/features/feature-toggle/feature-toggle-store.ts b/src/lib/features/feature-toggle/feature-toggle-store.ts index 26d7d33cf589..1deb717200b8 100644 --- a/src/lib/features/feature-toggle/feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-store.ts @@ -198,6 +198,10 @@ export default class FeatureToggleStore implements IFeatureToggleStore { builder.addSelectColumn('df.enabled as parent_enabled'); } + if (featureQuery?.project) { + builder.forProject(featureQuery.project); + } + const rows = await builder.internalQuery.select( builder.getSelectColumns(), ); diff --git a/src/lib/features/feature-toggle/query-builders/feature-toggle-list-builder.ts b/src/lib/features/feature-toggle/query-builders/feature-toggle-list-builder.ts index da388a6db831..0fa22b4e90a7 100644 --- a/src/lib/features/feature-toggle/query-builders/feature-toggle-list-builder.ts +++ b/src/lib/features/feature-toggle/query-builders/feature-toggle-list-builder.ts @@ -129,7 +129,11 @@ export class FeatureToggleListBuilder { userId, ); }); - + return this; } -} \ No newline at end of file + + forProject = (project: string[]) => { + this.internalQuery.whereIn('features.project', project); + } +} diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-store.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-store.e2e.test.ts index beeca94b467d..1f3f43279c9d 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-store.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-store.e2e.test.ts @@ -1,16 +1,22 @@ import dbInit from '../../../../test/e2e/helpers/database-init'; import getLogger from '../../../../test/fixtures/no-logger'; -import { FeatureToggleDTO, IFeatureToggleStore } from '../../../types'; +import { + FeatureToggleDTO, + IFeatureToggleStore, + IProjectStore, +} from '../../../types'; let stores; let db; let featureToggleStore: IFeatureToggleStore; +let projectStore: IProjectStore; beforeAll(async () => { getLogger.setMuteError(true); db = await dbInit('feature_toggle_store_serial', getLogger); stores = db.stores; featureToggleStore = stores.featureToggleStore; + projectStore = stores.projectStore; }); afterAll(async () => { @@ -301,5 +307,24 @@ describe('potentially_stale marking', () => { expect(potentiallyStale).toBeFalsy(); }); + + test('it should filter projects for playground', async () => { + await projectStore.create({ + id: 'MyProject', + name: 'MyProject', + description: 'MyProject', + }); + await featureToggleStore.create('default', { name: 'featureA' }); + + await featureToggleStore.create('MyProject', { name: 'featureB' }); + + const playgroundFeatures = + await featureToggleStore.getPlaygroundFeatures({ + project: ['MyProject'], + }); + + expect(playgroundFeatures).toHaveLength(1); + expect(playgroundFeatures[0].project).toBe('MyProject'); + }); }); }); diff --git a/src/lib/features/playground/playground-service.ts b/src/lib/features/playground/playground-service.ts index 4b29f1d7888a..23ab10d74af0 100644 --- a/src/lib/features/playground/playground-service.ts +++ b/src/lib/features/playground/playground-service.ts @@ -101,7 +101,7 @@ export class PlaygroundService { ): Promise { const segments = await this.segmentService.getActive(); - let filteredProjects: typeof projects; + let filteredProjects: typeof projects = projects; if (this.flagResolver.isEnabled('privateProjects')) { const projectAccess = await this.privateProjectChecker.getUserAccessibleProjects( diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index fbafe4ae357b..3472c526b6db 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -14,7 +14,6 @@ import FakeGroupStore from '../../../test/fixtures/fake-group-store'; import FakeEventStore from '../../../test/fixtures/fake-event-store'; import ProjectStore from '../../db/project-store'; import FeatureToggleStore from '../feature-toggle/feature-toggle-store'; -import FeatureTypeStore from '../../db/feature-type-store'; import { FeatureEnvironmentStore } from '../../db/feature-environment-store'; import ProjectStatsStore from '../../db/project-stats-store'; import { @@ -29,7 +28,6 @@ import { FavoriteFeaturesStore } from '../../db/favorite-features-store'; import { FavoriteProjectsStore } from '../../db/favorite-projects-store'; import FakeProjectStore from '../../../test/fixtures/fake-project-store'; import FakeFeatureToggleStore from '../feature-toggle/fakes/fake-feature-toggle-store'; -import FakeFeatureTypeStore from '../../../test/fixtures/fake-feature-type-store'; import FakeEnvironmentStore from '../../../test/fixtures/fake-environment-store'; import FakeFeatureEnvironmentStore from '../../../test/fixtures/fake-feature-environment-store'; import FakeProjectStatsStore from '../../../test/fixtures/fake-project-stats-store'; @@ -41,8 +39,6 @@ import { createPrivateProjectChecker, } from '../private-project/createPrivateProjectChecker'; import FakeFeatureTagStore from '../../../test/fixtures/fake-feature-tag-store'; -import { LastSeenAtReadModel } from '../../services/client-metrics/last-seen/last-seen-read-model'; -import { FakeLastSeenReadModel } from '../../services/client-metrics/last-seen/fake-last-seen-read-model'; export const createProjectService = ( db: Db, @@ -63,7 +59,6 @@ export const createProjectService = ( getLogger, flagResolver, ); - const featureTypeStore = new FeatureTypeStore(db, getLogger); const accountStore = new AccountStore(db, getLogger); const environmentStore = new EnvironmentStore(db, eventBus, getLogger); const featureEnvironmentStore = new FeatureEnvironmentStore( @@ -106,14 +101,12 @@ export const createProjectService = ( ); const privateProjectChecker = createPrivateProjectChecker(db, config); - const lastSeenReadModel = new LastSeenAtReadModel(db); return new ProjectService( { projectStore, eventStore, featureToggleStore, - featureTypeStore, environmentStore, featureEnvironmentStore, accountStore, @@ -126,7 +119,6 @@ export const createProjectService = ( favoriteService, eventService, privateProjectChecker, - lastSeenReadModel, ); }; @@ -138,7 +130,6 @@ export const createFakeProjectService = ( const projectStore = new FakeProjectStore(); const groupStore = new FakeGroupStore(); const featureToggleStore = new FakeFeatureToggleStore(); - const featureTypeStore = new FakeFeatureTypeStore(); const accountStore = new FakeAccountStore(); const environmentStore = new FakeEnvironmentStore(); const featureEnvironmentStore = new FakeFeatureEnvironmentStore(); @@ -169,14 +160,12 @@ export const createFakeProjectService = ( ); const privateProjectChecker = createFakePrivateProjectChecker(); - const fakeLastSeenReadModel = new FakeLastSeenReadModel(); return new ProjectService( { projectStore, eventStore, featureToggleStore, - featureTypeStore, environmentStore, featureEnvironmentStore, accountStore, @@ -189,6 +178,5 @@ export const createFakeProjectService = ( favoriteService, eventService, privateProjectChecker, - fakeLastSeenReadModel, ); }; diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 812aef1b08fc..f7e55a4a5a78 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -568,7 +568,7 @@ export class AccessService { } async removeDefaultProjectRoles( - owner: User, + owner: IUser, projectId: string, ): Promise { this.logger.info(`Removing project roles for ${projectId}`); diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 81a544173222..961b20c71f1d 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -1,6 +1,6 @@ import { subDays } from 'date-fns'; import { ValidationError } from 'joi'; -import User, { IUser } from '../types/user'; +import { IUser } from '../types/user'; import { AccessService, AccessWithRoles } from './access-service'; import NameExistsError from '../error/name-exists-error'; import InvalidOperationError from '../error/invalid-operation-error'; @@ -15,7 +15,6 @@ import { IEventStore, IFeatureEnvironmentStore, IFeatureToggleStore, - IFeatureTypeStore, IProject, IProjectOverview, IProjectWithCount, @@ -65,8 +64,6 @@ import { ProjectDoraMetricsSchema } from 'lib/openapi'; import { checkFeatureNamingData } from '../features/feature-naming-pattern/feature-naming-validation'; import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType'; import EventService from './event-service'; -import { ILastSeenReadModel } from './client-metrics/last-seen/types/last-seen-read-model-type'; -import { LastSeenMapper } from './client-metrics/last-seen/last-seen-mapper'; const getCreatedBy = (user: IUser) => user.email || user.username || 'unknown'; @@ -89,6 +86,10 @@ interface ICalculateStatus { updates: IProjectStats; } +function includes(list: number[], { id }: { id: number }): boolean { + return list.some((l) => l === id); +} + export default class ProjectService { private projectStore: IProjectStore; @@ -98,8 +99,6 @@ export default class ProjectService { private featureToggleStore: IFeatureToggleStore; - private featureTypeStore: IFeatureTypeStore; - private featureEnvironmentStore: IFeatureEnvironmentStore; private environmentStore: IEnvironmentStore; @@ -120,8 +119,6 @@ export default class ProjectService { private projectStatsStore: IProjectStatsStore; - private lastSeenReadModel: ILastSeenReadModel; - private flagResolver: IFlagResolver; private isEnterprise: boolean; @@ -131,7 +128,6 @@ export default class ProjectService { projectStore, eventStore, featureToggleStore, - featureTypeStore, environmentStore, featureEnvironmentStore, accountStore, @@ -141,7 +137,6 @@ export default class ProjectService { | 'projectStore' | 'eventStore' | 'featureToggleStore' - | 'featureTypeStore' | 'environmentStore' | 'featureEnvironmentStore' | 'accountStore' @@ -154,7 +149,6 @@ export default class ProjectService { favoriteService: FavoritesService, eventService: EventService, privateProjectChecker: IPrivateProjectChecker, - lastSeenReadModel: ILastSeenReadModel, ) { this.projectStore = projectStore; this.environmentStore = environmentStore; @@ -162,7 +156,6 @@ export default class ProjectService { this.accessService = accessService; this.eventStore = eventStore; this.featureToggleStore = featureToggleStore; - this.featureTypeStore = featureTypeStore; this.featureToggleService = featureToggleService; this.favoritesService = favoriteService; this.privateProjectChecker = privateProjectChecker; @@ -170,7 +163,6 @@ export default class ProjectService { this.groupService = groupService; this.eventService = eventService; this.projectStatsStore = projectStatsStore; - this.lastSeenReadModel = lastSeenReadModel; this.logger = config.getLogger('services/project-service.js'); this.flagResolver = config.flagResolver; this.isEnterprise = config.isEnterprise; @@ -267,7 +259,7 @@ export default class ProjectService { return data; } - async updateProject(updatedProject: IProject, user: User): Promise { + async updateProject(updatedProject: IProject, user: IUser): Promise { const preData = await this.projectStore.get(updatedProject.id); await this.projectStore.update(updatedProject); @@ -283,7 +275,7 @@ export default class ProjectService { async updateProjectEnterpriseSettings( updatedProject: IProjectEnterpriseSettingsUpdate, - user: User, + user: IUser, ): Promise { const preData = await this.projectStore.get(updatedProject.id); @@ -330,7 +322,7 @@ export default class ProjectService { async changeProject( newProjectId: string, featureName: string, - user: User, + user: IUser, currentProjectId: string, ): Promise { const feature = await this.featureToggleStore.get(featureName); @@ -372,7 +364,7 @@ export default class ProjectService { return updatedFeature; } - async deleteProject(id: string, user: User): Promise { + async deleteProject(id: string, user: IUser): Promise { if (id === DEFAULT_PROJECT) { throw new InvalidOperationError( 'You can not delete the default project!', @@ -508,6 +500,11 @@ export default class ProjectService { userId, ); + const ownerRole = await this.accessService.getRoleByName( + RoleName.OWNER, + ); + await this.validateAtLeastOneOwner(projectId, ownerRole); + await this.accessService.removeUserAccess(projectId, userId); await this.eventService.storeEvent( @@ -532,6 +529,11 @@ export default class ProjectService { groupId, ); + const ownerRole = await this.accessService.getRoleByName( + RoleName.OWNER, + ); + await this.validateAtLeastOneOwner(projectId, ownerRole); + await this.accessService.removeGroupAccess(projectId, groupId); await this.eventService.storeEvent( @@ -598,6 +600,8 @@ export default class ProjectService { undefined, ); + await this.validateAtLeastOneOwner(projectId, role); + await this.accessService.removeGroupFromRole( group.id, role.id, @@ -675,28 +679,39 @@ export default class ProjectService { async setRolesForUser( projectId: string, userId: number, - roles: number[], + newRoles: number[], createdByUserName: string, ): Promise { - const existingRoles = await this.accessService.getProjectRolesForUser( + const currentRoles = await this.accessService.getProjectRolesForUser( projectId, userId, ); + + const ownerRole = await this.accessService.getRoleByName( + RoleName.OWNER, + ); + + const hasOwnerRole = includes(currentRoles, ownerRole); + const isRemovingOwnerRole = !includes(newRoles, ownerRole); + if (hasOwnerRole && isRemovingOwnerRole) { + await this.validateAtLeastOneOwner(projectId, ownerRole); + } + await this.accessService.setProjectRolesForUser( projectId, userId, - roles, + newRoles, ); await this.eventService.storeEvent( new ProjectAccessUserRolesUpdated({ project: projectId, createdBy: createdByUserName, data: { - roles, + roles: newRoles, userId, }, preData: { - roles: existingRoles, + roles: currentRoles, userId, }, }), @@ -706,17 +721,28 @@ export default class ProjectService { async setRolesForGroup( projectId: string, groupId: number, - roles: number[], + newRoles: number[], createdBy: string, ): Promise { - const existingRoles = await this.accessService.getProjectRolesForGroup( + const currentRoles = await this.accessService.getProjectRolesForGroup( projectId, groupId, ); + + const ownerRole = await this.accessService.getRoleByName( + RoleName.OWNER, + ); + const hasOwnerRole = includes(currentRoles, ownerRole); + const isRemovingOwnerRole = !includes(newRoles, ownerRole); + if (hasOwnerRole && isRemovingOwnerRole) { + await this.validateAtLeastOneOwner(projectId, ownerRole); + } + await this.validateAtLeastOneOwner(projectId, ownerRole); + await this.accessService.setProjectRolesForGroup( projectId, groupId, - roles, + newRoles, createdBy, ); await this.eventService.storeEvent( @@ -724,11 +750,11 @@ export default class ProjectService { project: projectId, createdBy, data: { - roles, + roles: newRoles, groupId, }, preData: { - roles: existingRoles, + roles: currentRoles, groupId, }, }), @@ -1091,7 +1117,7 @@ export default class ProjectService { return { stats: projectStats, name: project.name, - description: project.description, + description: project.description!, mode: project.mode, featureLimit: project.featureLimit, featureNaming: project.featureNaming, diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index bc0e83f6ed26..16d6b80172f2 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -146,9 +146,9 @@ export const SERVICE_ACCOUNT_DELETED = 'service-account-deleted' as const; export const FEATURE_POTENTIALLY_STALE_ON = 'feature-potentially-stale-on' as const; -export const MESSAGE_BANNER_CREATED = 'message-banner-created' as const; -export const MESSAGE_BANNER_UPDATED = 'message-banner-updated' as const; -export const MESSAGE_BANNER_DELETED = 'message-banner-deleted' as const; +export const BANNER_CREATED = 'banner-created' as const; +export const BANNER_UPDATED = 'banner-updated' as const; +export const BANNER_DELETED = 'banner-deleted' as const; export const IEventTypes = [ APPLICATION_CREATED, @@ -263,9 +263,9 @@ export const IEventTypes = [ FEATURE_DEPENDENCY_ADDED, FEATURE_DEPENDENCY_REMOVED, FEATURE_DEPENDENCIES_REMOVED, - MESSAGE_BANNER_CREATED, - MESSAGE_BANNER_UPDATED, - MESSAGE_BANNER_DELETED, + BANNER_CREATED, + BANNER_UPDATED, + BANNER_DELETED, ] as const; export type IEventType = typeof IEventTypes[number]; diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index e663acb27071..afbeb9b91fa7 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -34,8 +34,7 @@ export type IFlagKey = | 'datadogJsonTemplate' | 'disableMetrics' | 'useLastSeenRefactor' - | 'internalMessageBanners' - | 'internalMessageBanner' + | 'banners' | 'separateAdminClientApi' | 'disableEnvsOnRevive' | 'playgroundImprovements'; @@ -162,8 +161,8 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_USE_LAST_SEEN_REFACTOR, false, ), - internalMessageBanners: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_INTERNAL_MESSAGE_BANNERS, + banners: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_BANNERS, false, ), separateAdminClientApi: parseEnvVarBoolean( diff --git a/src/migrations/20231019110154-rename-message-banners-table-to-banners.js b/src/migrations/20231019110154-rename-message-banners-table-to-banners.js new file mode 100644 index 000000000000..460877c56b65 --- /dev/null +++ b/src/migrations/20231019110154-rename-message-banners-table-to-banners.js @@ -0,0 +1,9 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql(`ALTER TABLE message_banners RENAME TO banners`, cb); +}; + +exports.down = function (db, cb) { + db.runSql(`ALTER TABLE banners RENAME TO message_banners`, cb); +}; diff --git a/src/server-dev.ts b/src/server-dev.ts index 63d3c6eed832..c24a3059b5bd 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -47,6 +47,7 @@ process.nextTick(async () => { datadogJsonTemplate: true, dependentFeatures: true, useLastSeenRefactor: true, + disableEnvsOnRevive: true, separateAdminClientApi: true, playgroundImprovements: true, }, diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 281a35024497..efbd717a996a 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -17,8 +17,10 @@ import { createFeatureToggleService, createProjectService, } from '../../../lib/features'; +import { IGroup, IUnleashStores } from 'lib/types'; +import { User } from 'lib/server-impl'; -let stores; +let stores: IUnleashStores; let db: ITestDb; let projectService: ProjectService; @@ -26,7 +28,8 @@ let accessService: AccessService; let eventService: EventService; let environmentService: EnvironmentService; let featureToggleService: FeatureToggleService; -let user; +let user: User; // many methods in this test use User instead of IUser +let group: IGroup; const isProjectUser = async ( userId: number, @@ -41,13 +44,17 @@ const isProjectUser = async ( beforeAll(async () => { db = await dbInit('project_service_serial', getLogger); stores = db.stores; + // @ts-ignore return type IUser type missing generateImageUrl user = await stores.userStore.insert({ name: 'Some Name', email: 'test@getunleash.io', }); + group = await stores.groupStore.create({ + name: 'aTestGroup', + description: '', + }); const config = createTestConfig({ getLogger, - // @ts-ignore experimental: { flags: { privateProjects: true }, }, @@ -164,9 +171,6 @@ test('should not be able to delete project with toggles', async () => { await projectService.createProject(project, user); await stores.featureToggleStore.create(project.id, { name: 'test-project-delete', - project: project.id, - enabled: false, - defaultStickiness: 'default', }); try { @@ -491,31 +495,6 @@ test('should remove user from the project', async () => { expect(memberUsers).toHaveLength(0); }); -test('should not remove user from the project', async () => { - const project = { - id: 'remove-users-not-allowed', - name: 'New project', - description: 'Blah', - mode: 'open' as const, - defaultStickiness: 'clientId', - }; - await projectService.createProject(project, user); - - const roles = await stores.roleStore.getRolesForProject(project.id); - const ownerRole = roles.find((r) => r.name === RoleName.OWNER); - - await expect(async () => { - await projectService.removeUser( - project.id, - ownerRole.id, - user.id, - 'test', - ); - }).rejects.toThrowError( - new Error('A project must have at least one owner'), - ); -}); - test('should not change project if feature toggle project does not match current project id', async () => { const project = { id: 'test-change-project', @@ -528,7 +507,11 @@ test('should not change project if feature toggle project does not match current const toggle = { name: 'test-toggle' }; await projectService.createProject(project, user); - await featureToggleService.createFeatureToggle(project.id, toggle, user); + await featureToggleService.createFeatureToggle( + project.id, + toggle, + user.email, + ); try { await projectService.changeProject( @@ -555,7 +538,11 @@ test('should return 404 if no project is found with the project id', async () => const toggle = { name: 'test-toggle-2' }; await projectService.createProject(project, user); - await featureToggleService.createFeatureToggle(project.id, toggle, user); + await featureToggleService.createFeatureToggle( + project.id, + toggle, + user.email, + ); try { await projectService.changeProject( @@ -594,7 +581,11 @@ test('should fail if user is not authorized', async () => { await projectService.createProject(project, user); await projectService.createProject(projectDestination, projectAdmin1); - await featureToggleService.createFeatureToggle(project.id, toggle, user); + await featureToggleService.createFeatureToggle( + project.id, + toggle, + user.email, + ); try { await projectService.changeProject( @@ -626,7 +617,11 @@ test('should change project when checks pass', async () => { await projectService.createProject(projectA, user); await projectService.createProject(projectB, user); - await featureToggleService.createFeatureToggle(projectA.id, toggle, user); + await featureToggleService.createFeatureToggle( + projectA.id, + toggle, + user.email, + ); await projectService.changeProject( projectB.id, toggle.name, @@ -656,7 +651,11 @@ test('changing project should emit event even if user does not have a username s const toggle = { name: randomId() }; await projectService.createProject(projectA, user); await projectService.createProject(projectB, user); - await featureToggleService.createFeatureToggle(projectA.id, toggle, user); + await featureToggleService.createFeatureToggle( + projectA.id, + toggle, + user.email, + ); const eventsBeforeChange = await stores.eventStore.getEvents(); await projectService.changeProject( projectB.id, @@ -686,7 +685,11 @@ test('should require equal project environments to move features', async () => { await projectService.createProject(projectA, user); await projectService.createProject(projectB, user); - await featureToggleService.createFeatureToggle(projectA.id, toggle, user); + await featureToggleService.createFeatureToggle( + projectA.id, + toggle, + user.email, + ); await stores.environmentStore.create(environment); await environmentService.addEnvironmentToProject( environment.name, @@ -1013,40 +1016,180 @@ test('should able to assign role without existing members', async () => { expect(testUsers).toHaveLength(1); }); -test('should not update role for user on project when she is the owner', async () => { - const project = { - id: 'update-users-not-allowed', - name: 'New project', - description: 'Blah', - mode: 'open' as const, - defaultStickiness: 'clientId', - }; - await projectService.createProject(project, user); +describe('ensure project has at least one owner', () => { + test('should not remove user from the project', async () => { + const project = { + id: 'remove-users-not-allowed', + name: 'New project', + description: 'Blah', + mode: 'open' as const, + defaultStickiness: 'clientId', + }; + await projectService.createProject(project, user); - const projectMember1 = await stores.userStore.insert({ - name: 'Some Member', - email: 'update991@getunleash.io', + const roles = await stores.roleStore.getRolesForProject(project.id); + const ownerRole = roles.find((r) => r.name === RoleName.OWNER)!; + + await expect(async () => { + await projectService.removeUser( + project.id, + ownerRole.id, + user.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + + await expect(async () => { + await projectService.removeUserAccess(project.id, user.id, 'test'); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); }); - const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); + test('should not update role for user on project when she is the owner', async () => { + const project = { + id: 'update-users-not-allowed', + name: 'New project', + description: 'Blah', + mode: 'open' as const, + defaultStickiness: 'clientId', + }; + await projectService.createProject(project, user); - await projectService.addUser( - project.id, - memberRole.id, - projectMember1.id, - 'test', - ); + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'update991@getunleash.io', + }); + + const memberRole = await stores.roleStore.getRoleByName( + RoleName.MEMBER, + ); - await expect(async () => { - await projectService.changeRole( + await projectService.addUser( project.id, memberRole.id, + projectMember1.id, + 'test', + ); + + await expect(async () => { + await projectService.changeRole( + project.id, + memberRole.id, + user.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + + await expect(async () => { + await projectService.setRolesForUser( + project.id, + user.id, + [memberRole.id], + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + }); + + async function projectWithGroupOwner(projectId: string) { + const project = { + id: projectId, + name: 'New project', + description: 'Blah', + mode: 'open' as const, + defaultStickiness: 'clientId', + }; + await projectService.createProject(project, user); + + const roles = await stores.roleStore.getRolesForProject(project.id); + const ownerRole = roles.find((r) => r.name === RoleName.OWNER)!; + + await projectService.addGroup( + project.id, + ownerRole.id, + group.id, + 'test', + ); + + // this should be fine, leaving the group as the only owner + // note group has zero members, but it still acts as an owner + await projectService.removeUser( + project.id, + ownerRole.id, user.id, 'test', ); - }).rejects.toThrowError( - new Error('A project must have at least one owner'), - ); + + return { + project, + group, + ownerRole, + }; + } + + test('should not remove group from the project', async () => { + const { project, group, ownerRole } = await projectWithGroupOwner( + 'remove-group-not-allowed', + ); + + await expect(async () => { + await projectService.removeGroup( + project.id, + ownerRole.id, + group.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + + await expect(async () => { + await projectService.removeGroupAccess( + project.id, + group.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + }); + + test('should not update role for group on project when she is the owner', async () => { + const { project, group } = await projectWithGroupOwner( + 'update-group-not-allowed', + ); + const memberRole = await stores.roleStore.getRoleByName( + RoleName.MEMBER, + ); + + await expect(async () => { + await projectService.changeGroupRole( + project.id, + memberRole.id, + group.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + + await expect(async () => { + await projectService.setRolesForGroup( + project.id, + group.id, + [memberRole.id], + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); + }); }); test('Should allow bulk update of group permissions', async () => { @@ -1056,7 +1199,7 @@ test('Should allow bulk update of group permissions', async () => { mode: 'open' as const, defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const groupStore = stores.groupStore; const user1 = await stores.userStore.insert({ @@ -1124,7 +1267,7 @@ test('Should allow bulk update of only groups', async () => { }; const groupStore = stores.groupStore; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const group1 = await groupStore.create({ name: 'ViewersOnly', @@ -1158,7 +1301,7 @@ test('Should allow permutations of roles, groups and users when adding a new acc defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const group1 = await stores.groupStore.create({ name: 'permutation-group-1', @@ -1211,11 +1354,13 @@ test('Should allow permutations of roles, groups and users when adding a new acc const { users, groups } = await projectService.getAccessToProject( project.id, ); + const ownerRole = await stores.roleStore.getRoleByName(RoleName.OWNER); - expect(users).toHaveLength(2); + expect(users).toHaveLength(3); // the 2 added plus the one that created the project expect(groups).toHaveLength(2); - expect(users[0].roles).toStrictEqual([role1.id, role2.id]); + expect(users[0].roles).toStrictEqual([ownerRole.id]); + expect(users[1].roles).toStrictEqual([role1.id, role2.id]); expect(groups[0].roles).toStrictEqual([role1.id, role2.id]); }); @@ -1232,13 +1377,9 @@ test('should only count active feature toggles for project', async () => { await stores.featureToggleStore.create(project.id, { name: 'only-active-t1', - project: project.id, - enabled: false, }); await stores.featureToggleStore.create(project.id, { name: 'only-active-t2', - project: project.id, - enabled: false, }); await featureToggleService.archiveToggle('only-active-t2', user); @@ -1261,8 +1402,6 @@ test('should list projects with all features archived', async () => { await stores.featureToggleStore.create(project.id, { name: 'archived-toggle', - project: project.id, - enabled: false, }); await featureToggleService.archiveToggle('archived-toggle', user); @@ -1294,7 +1433,7 @@ test('should calculate average time to production', async () => { defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const toggles = [ { name: 'average-prod-time' }, @@ -1309,7 +1448,7 @@ test('should calculate average time to production', async () => { return featureToggleService.createFeatureToggle( project.id, toggle, - user, + user.email, ); }), ); @@ -1360,7 +1499,7 @@ test('should calculate average time to production ignoring some items', async () tags: [], }); - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); await stores.environmentStore.create({ name: 'customEnv', type: 'development', @@ -1369,7 +1508,11 @@ test('should calculate average time to production ignoring some items', async () // actual toggle we take for calculations const toggle = { name: 'main-toggle' }; - await featureToggleService.createFeatureToggle(project.id, toggle, user); + await featureToggleService.createFeatureToggle( + project.id, + toggle, + user.email, + ); await updateFeature(toggle.name, { created_at: subDays(new Date(), 20), }); @@ -1384,7 +1527,11 @@ test('should calculate average time to production ignoring some items', async () // ignore toggles enabled in non-prod envs const devToggle = { name: 'dev-toggle' }; - await featureToggleService.createFeatureToggle(project.id, devToggle, user); + await featureToggleService.createFeatureToggle( + project.id, + devToggle, + user.email, + ); await eventService.storeEvent( new FeatureEnvironmentEvent({ ...makeEvent(devToggle.name), @@ -1397,7 +1544,7 @@ test('should calculate average time to production ignoring some items', async () await featureToggleService.createFeatureToggle( 'default', otherProjectToggle, - user, + user.email, ); await eventService.storeEvent( new FeatureEnvironmentEvent(makeEvent(otherProjectToggle.name)), @@ -1408,7 +1555,7 @@ test('should calculate average time to production ignoring some items', async () await featureToggleService.createFeatureToggle( project.id, nonReleaseToggle, - user, + user.email, ); await eventService.storeEvent( new FeatureEnvironmentEvent(makeEvent(nonReleaseToggle.name)), @@ -1419,7 +1566,7 @@ test('should calculate average time to production ignoring some items', async () await featureToggleService.createFeatureToggle( project.id, previouslyDeleteToggle, - user, + user.email, ); await eventService.storeEvent( new FeatureEnvironmentEvent(makeEvent(previouslyDeleteToggle.name)), @@ -1441,7 +1588,7 @@ test('should get correct amount of features created in current and past window', defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const toggles = [ { name: 'features-created' }, @@ -1455,7 +1602,7 @@ test('should get correct amount of features created in current and past window', return featureToggleService.createFeatureToggle( project.id, toggle, - user, + user.email, ); }), ); @@ -1478,7 +1625,7 @@ test('should get correct amount of features archived in current and past window' defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const toggles = [ { name: 'features-archived' }, @@ -1492,7 +1639,7 @@ test('should get correct amount of features archived in current and past window' return featureToggleService.createFeatureToggle( project.id, toggle, - user, + user.email, ); }), ); @@ -1529,7 +1676,7 @@ test('should get correct amount of project members for current and past window', defaultStickiness: 'default', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const users = [ { name: 'memberOne', email: 'memberOne@getunleash.io' }, @@ -1556,7 +1703,7 @@ test('should get correct amount of project members for current and past window', ); const result = await projectService.getStatusUpdates(project.id); - expect(result.updates.projectMembersAddedCurrentWindow).toBe(5); + expect(result.updates.projectMembersAddedCurrentWindow).toBe(6); // 5 members + 1 owner expect(result.updates.projectActivityCurrentWindow).toBe(6); expect(result.updates.projectActivityPastWindow).toBe(0); }); @@ -1569,7 +1716,7 @@ test('should return average time to production per toggle', async () => { defaultStickiness: 'clientId', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const toggles = [ { name: 'average-prod-time-pt', subdays: 7 }, @@ -1584,7 +1731,7 @@ test('should return average time to production per toggle', async () => { return featureToggleService.createFeatureToggle( project.id, toggle, - user, + user.email, ); }), ); @@ -1633,8 +1780,8 @@ test('should return average time to production per toggle for a specific project defaultStickiness: 'clientId', }; - await projectService.createProject(project1, user.id); - await projectService.createProject(project2, user.id); + await projectService.createProject(project1, user); + await projectService.createProject(project2, user); const togglesProject1 = [ { name: 'average-prod-time-pt-10', subdays: 7 }, @@ -1652,7 +1799,7 @@ test('should return average time to production per toggle for a specific project return featureToggleService.createFeatureToggle( project1.id, toggle, - user, + user.email, ); }), ); @@ -1662,7 +1809,7 @@ test('should return average time to production per toggle for a specific project return featureToggleService.createFeatureToggle( project2.id, toggle, - user, + user.email, ); }), ); @@ -1726,7 +1873,7 @@ test('should return average time to production per toggle and include archived t defaultStickiness: 'clientId', }; - await projectService.createProject(project1, user.id); + await projectService.createProject(project1, user); const togglesProject1 = [ { name: 'average-prod-time-pta-10', subdays: 7 }, @@ -1739,7 +1886,7 @@ test('should return average time to production per toggle and include archived t return featureToggleService.createFeatureToggle( project1.id, toggle, - user, + user.email, ); }), ); @@ -1790,7 +1937,7 @@ describe('feature flag naming patterns', () => { featureNaming, }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); await projectService.updateProjectEnterpriseSettings(project, user); @@ -1804,7 +1951,7 @@ describe('feature flag naming patterns', () => { ...project, featureNaming: { pattern: newPattern }, }, - user.id, + user, ); const updatedProject = await projectService.getProject(project.id); @@ -1822,13 +1969,10 @@ test('deleting a project with archived toggles should result in any remaining ar }; const toggleName = 'archived-and-deleted'; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); await stores.featureToggleStore.create(project.id, { name: toggleName, - project: project.id, - enabled: false, - defaultStickiness: 'default', }); await stores.featureToggleStore.archive(toggleName); @@ -1836,7 +1980,7 @@ test('deleting a project with archived toggles should result in any remaining ar // bring the project back again, previously this would allow those archived toggles to be resurrected // we now expect them to be deleted correctly - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); const toggles = await stores.featureToggleStore.getAll({ project: project.id, @@ -1852,6 +1996,6 @@ test('deleting a project with no archived toggles should not result in an error' name: 'project-with-nothing', }; - await projectService.createProject(project, user.id); + await projectService.createProject(project, user); await projectService.deleteProject(project.id, user); });