refactor: new endpoint for global impact metrics saving (#10631)

Use `impact-metrics/config` for saving global charts.

frontend/src/component/feature/FeatureView/FeatureMetrics/FeatureImpactMetrics.tsx

@@ -5,7 +5,7 @@ import Add from '@mui/icons-material/Add';
 import { useImpactMetricsMetadata } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata.ts';
 import { type FC, useMemo, useState } from 'react';
 import { ChartConfigModal } from '../../../impact-metrics/ChartConfigModal/ChartConfigModal.tsx';
-import { useImpactMetricsApi } from 'hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsApi.ts';
+import { useImpactMetricsApi } from 'hooks/api/actions/useImpactMetricsApi/useImpactMetricsApi.ts';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam.ts';
 import { useFeatureImpactMetrics } from 'hooks/api/getters/useFeatureImpactMetrics/useFeatureImpactMetrics.ts';
 import { ChartItem } from '../../../impact-metrics/ChartItem.tsx';

frontend/src/component/impact-metrics/GridLayoutWrapper.tsx

@@ -134,8 +134,8 @@ export const GridLayoutWrapper: FC<GridLayoutWrapperProps> = ({
                     Number.parseInt(theme.spacing(2)),
                 ]}
                 containerPadding={[0, 0]}
-                isDraggable={!isMobileBreakpoint}
+                isDraggable={false}
-                isResizable={!isMobileBreakpoint}
+                isResizable={false}
                 onLayoutChange={handleLayoutChange}
                 resizeHandles={['se']}
                 draggableHandle='.grid-item-drag-handle'

frontend/src/component/impact-metrics/ImpactMetrics.tsx

@@ -2,14 +2,13 @@ import type { FC } from 'react';
 import { useMemo, useState, useCallback } from 'react';
 import { Typography, Button, Paper, styled, Box } from '@mui/material';
 import Add from '@mui/icons-material/Add';
-import DragHandle from '@mui/icons-material/DragHandle';
 import { PageHeader } from 'component/common/PageHeader/PageHeader.tsx';
 import { useImpactMetricsMetadata } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata';
 import { ChartConfigModal } from './ChartConfigModal/ChartConfigModal.tsx';
 import { ChartItem } from './ChartItem.tsx';
 import { GridLayoutWrapper, type GridItem } from './GridLayoutWrapper.tsx';
 import { useImpactMetricsState } from './hooks/useImpactMetricsState.ts';
-import type { ChartConfig, LayoutItem } from './types.ts';
+import type { ChartConfig } from './types.ts';
 import useToast from 'hooks/useToast';
 import { formatUnknownError } from 'utils/formatUnknownError';
 import PermissionButton from 'component/common/PermissionButton/PermissionButton.tsx';
@@ -49,7 +48,6 @@ export const ImpactMetrics: FC = () => {
         addChart,
         updateChart,
         deleteChart,
-        updateLayout,
     } = useImpactMetricsState();
 
     const {
@@ -91,17 +89,6 @@ export const ImpactMetrics: FC = () => {
         }
     };
 
-    const handleLayoutChange = useCallback(
-        async (layout: any[]) => {
-            try {
-                await updateLayout(layout as LayoutItem[]);
-            } catch (error) {
-                setToastApiError(formatUnknownError(error));
-            }
-        },
-        [updateLayout, setToastApiError],
-    );
-
     const handleDeleteChart = useCallback(
         async (id: string) => {
             try {
@@ -126,11 +113,6 @@ export const ImpactMetrics: FC = () => {
                             config={config}
                             onEdit={handleEditChart}
                             onDelete={handleDeleteChart}
-                            dragHandle={
-                                <StyledDragHandle className='grid-item-drag-handle'>
-                                    <DragHandle fontSize='small' />
-                                </StyledDragHandle>
-                            }
                         />
                     ),
                     w: existingLayout?.w ?? 6,
@@ -194,10 +176,7 @@ export const ImpactMetrics: FC = () => {
                     </Button>
                 </StyledEmptyState>
             ) : charts.length > 0 ? (
-                <GridLayoutWrapper
+                <GridLayoutWrapper items={gridItems} />
-                    items={gridItems}
-                    onLayoutChange={handleLayoutChange}
-                />
             ) : null}
 
             <ChartConfigModal

frontend/src/component/impact-metrics/hooks/useImpactMetricsState.test.tsx

@@ -52,7 +52,14 @@ const TestComponent: FC<{
                     type='button'
                     data-testid='update-chart'
                     onClick={() =>
-                        updateChart(charts[0].id, { title: 'Updated Chart' })
+                        updateChart(charts[0].id, {
+                            metricName: charts[0].metricName,
+                            timeRange: charts[0].timeRange,
+                            yAxisMin: charts[0].yAxisMin,
+                            aggregationMode: charts[0].aggregationMode,
+                            labelSelectors: charts[0].labelSelectors,
+                            title: 'Updated Chart',
+                        })
                     }
                 >
                     Update Chart
@@ -110,11 +117,9 @@ describe('useImpactMetricsState', () => {
     });
 
     it('loads settings from API', async () => {
-        testServerRoute(
+        testServerRoute(server, '/api/admin/impact-metrics/config', {
-            server,
+            configs: mockSettings.charts,
-            '/api/admin/impact-metrics/settings',
+        });
-            mockSettings,
-        );
 
         render(<TestComponent />);
 
@@ -127,11 +132,9 @@ describe('useImpactMetricsState', () => {
     });
 
     it('handles empty settings', async () => {
-        testServerRoute(
+        testServerRoute(server, '/api/admin/impact-metrics/config', {
-            server,
+            configs: emptySettings.charts,
-            '/api/admin/impact-metrics/settings',
+        });
-            emptySettings,
-        );
 
         render(<TestComponent />);
 
@@ -146,7 +149,7 @@ describe('useImpactMetricsState', () => {
     it('handles API errors', async () => {
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config',
             { message: 'Server error' },
             'get',
             500,
@@ -160,11 +163,9 @@ describe('useImpactMetricsState', () => {
     });
 
     it('adds a chart successfully', async () => {
-        testServerRoute(
+        testServerRoute(server, '/api/admin/impact-metrics/config', {
-            server,
+            configs: emptySettings.charts,
-            '/api/admin/impact-metrics/settings',
+        });
-            emptySettings,
-        );
 
         render(<TestComponent enableActions />);
 
@@ -174,65 +175,27 @@ describe('useImpactMetricsState', () => {
 
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config',
-            {
+            'Created',
-                charts: [
+            'post',
-                    {
+            201,
-                        id: 'new-chart-id',
-                        metricName: 'test-series',
-                        timeRange: 'day',
-                        yAxisMin: 'zero',
-                        mode: 'count',
-                        labelSelectors: {},
-                        title: 'Test Chart',
-                    },
-                ],
-                layout: [
-                    {
-                        i: 'new-chart-id',
-                        x: 0,
-                        y: 0,
-                        w: 6,
-                        h: 4,
-                        minW: 4,
-                        minH: 2,
-                        maxW: 12,
-                        maxH: 8,
-                    },
-                ],
-            },
-            'put',
-            200,
         );
 
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config',
             {
-                charts: [
+                configs: [
                     {
                         id: 'new-chart-id',
                         metricName: 'test-series',
                         timeRange: 'day',
                         yAxisMin: 'zero',
-                        mode: 'count',
+                        aggregationMode: 'count',
                         labelSelectors: {},
                         title: 'Test Chart',
                     },
                 ],
-                layout: [
-                    {
-                        i: 'new-chart-id',
-                        x: 0,
-                        y: 0,
-                        w: 6,
-                        h: 4,
-                        minW: 4,
-                        minH: 2,
-                        maxW: 12,
-                        maxH: 8,
-                    },
-                ],
             },
             'get',
             200,
@@ -252,25 +215,15 @@ describe('useImpactMetricsState', () => {
     });
 
     it('updates a chart successfully', async () => {
-        testServerRoute(
+        testServerRoute(server, '/api/admin/impact-metrics/config', {
-            server,
+            configs: mockSettings.charts,
-            '/api/admin/impact-metrics/settings',
+        });
-            mockSettings,
-        );
 
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config',
-            {
+            'Updated',
-                charts: [
+            'post',
-                    {
-                        ...mockSettings.charts[0],
-                        title: 'Updated Chart',
-                    },
-                ],
-                layout: mockSettings.layout,
-            },
-            'put',
             200,
         );
 
@@ -289,11 +242,9 @@ describe('useImpactMetricsState', () => {
     });
 
     it('deletes a chart successfully', async () => {
-        testServerRoute(
+        testServerRoute(server, '/api/admin/impact-metrics/config', {
-            server,
+            configs: mockSettings.charts,
-            '/api/admin/impact-metrics/settings',
+        });
-            mockSettings,
-        );
 
         render(<TestComponent enableActions />);
 
@@ -303,16 +254,16 @@ describe('useImpactMetricsState', () => {
 
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config/test-chart',
-            emptySettings,
+            'Deleted',
-            'put',
+            'delete',
             200,
         );
 
         testServerRoute(
             server,
-            '/api/admin/impact-metrics/settings',
+            '/api/admin/impact-metrics/config',
-            emptySettings,
+            { configs: emptySettings.charts },
             'get',
             200,
         );

frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts

@@ -1,7 +1,7 @@
-import { useCallback } from 'react';
+import { useCallback, useMemo } from 'react';
-import { useImpactMetricsSettings } from 'hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.js';
+import type { ChartConfig } from '../types.ts';
-import { useImpactMetricsSettingsApi } from 'hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.js';
+import { useImpactMetricsConfig } from 'hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts';
-import type { ChartConfig, ImpactMetricsState, LayoutItem } from '../types.ts';
+import { useImpactMetricsApi } from 'hooks/api/actions/useImpactMetricsApi/useImpactMetricsApi.ts';
 
 /**
  * "Select all" represents all current and future labels.
@@ -11,106 +11,76 @@ export const METRIC_LABELS_SELECT_ALL = '*';
 
 export const useImpactMetricsState = () => {
     const {
-        settings,
+        configs,
-        loading: settingsLoading,
+        loading: configLoading,
-        error: settingsError,
+        error: configError,
         refetch,
-    } = useImpactMetricsSettings();
+    } = useImpactMetricsConfig();
+
+    const { layout, charts } = useMemo(
+        () => ({
+            layout: configs.map((config, index) => {
+                const column = index % 2;
+                const row = Math.floor(index / 2);
+
+                return {
+                    i: config.id,
+                    x: column * 6,
+                    y: row * 2,
+                    w: 6,
+                    h: 2,
+                    minW: 4,
+                    minH: 2,
+                    maxW: 12,
+                    maxH: 8,
+                };
+            }),
+            charts: configs,
+        }),
+        [configs],
+    );
 
     const {
-        updateSettings,
+        createImpactMetric,
+        deleteImpactMetric,
         loading: actionLoading,
         errors: actionErrors,
-    } = useImpactMetricsSettingsApi();
+    } = useImpactMetricsApi();
 
     const addChart = useCallback(
         async (config: Omit<ChartConfig, 'id'>) => {
-            const newChart: ChartConfig = {
+            await createImpactMetric(config);
-                ...config,
-                id: `chart-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
-            };
-
-            const maxY =
-                settings.layout.length > 0
-                    ? Math.max(
-                          ...settings.layout.map((item) => item.y + item.h),
-                      )
-                    : 0;
-
-            const newState: ImpactMetricsState = {
-                charts: [...settings.charts, newChart],
-                layout: [
-                    ...settings.layout,
-                    {
-                        i: newChart.id,
-                        x: 0,
-                        y: maxY,
-                        w: 6,
-                        h: 4,
-                        minW: 4,
-                        minH: 2,
-                        maxW: 12,
-                        maxH: 8,
-                    },
-                ],
-            };
-
-            await updateSettings(newState);
             refetch();
         },
-        [settings, updateSettings, refetch],
+        [createImpactMetric, refetch],
     );
 
     const updateChart = useCallback(
-        async (id: string, updates: Partial<ChartConfig>) => {
+        async (id: string, updates: Omit<ChartConfig, 'id'>) => {
-            const updatedCharts = settings.charts.map((chart) =>
+            await createImpactMetric({ ...updates, id });
-                chart.id === id ? { ...chart, ...updates } : chart,
-            );
-            const newState: ImpactMetricsState = {
-                charts: updatedCharts,
-                layout: settings.layout,
-            };
-            await updateSettings(newState);
             refetch();
         },
-        [settings, updateSettings, refetch],
+        [configs, createImpactMetric, refetch],
     );
 
     const deleteChart = useCallback(
         async (id: string) => {
-            const newState: ImpactMetricsState = {
+            await deleteImpactMetric(id);
-                charts: settings.charts.filter((chart) => chart.id !== id),
-                layout: settings.layout.filter((item) => item.i !== id),
-            };
-            await updateSettings(newState);
             refetch();
         },
-        [settings, updateSettings, refetch],
+        [configs, deleteImpactMetric, refetch],
-    );
-
-    const updateLayout = useCallback(
-        async (newLayout: LayoutItem[]) => {
-            const newState: ImpactMetricsState = {
-                charts: settings.charts,
-                layout: newLayout,
-            };
-            await updateSettings(newState);
-            refetch();
-        },
-        [settings, updateSettings, refetch],
     );
 
     return {
-        charts: settings.charts || [],
+        charts,
-        layout: settings.layout || [],
+        layout,
-        loading: settingsLoading || actionLoading,
+        loading: configLoading || actionLoading,
         error:
-            settingsError || Object.keys(actionErrors).length > 0
+            configError || Object.keys(actionErrors).length > 0
                 ? actionErrors
                 : undefined,
         addChart,
         updateChart,
         deleteChart,
-        updateLayout,
     };
 };

frontend/src/hooks/api/actions/useImpactMetricsApi/useImpactMetricsApi.ts

@@ -9,7 +9,7 @@ type UseImpactMetricsApiParams =
       }
     | undefined;
 
-export const useImpactMetricsApi = (params: UseImpactMetricsApiParams) => {
+export const useImpactMetricsApi = (params?: UseImpactMetricsApiParams) => {
     const basePath = params
         ? `api/admin/projects/${params.projectId}/features/${params.featureName}/impact-metrics/config`
         : `api/admin/impact-metrics/config`;
@@ -46,7 +46,7 @@ export const useImpactMetricsApi = (params: UseImpactMetricsApiParams) => {
 
             return makeRequest(req.caller, req.id);
         },
-        [makeRequest, createRequest],
+        [makeRequest, createRequest, basePath],
     );
 
     return {

frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts

@@ -1,32 +0,0 @@
-import { useCallback } from 'react';
-import useAPI from '../useApi/useApi.js';
-import type { ImpactMetricsState } from 'component/impact-metrics/types.ts';
-
-export const useImpactMetricsSettingsApi = () => {
-    const { makeRequest, createRequest, errors, loading } = useAPI({
-        propagateErrors: true,
-    });
-
-    const updateSettings = useCallback(
-        async (settings: ImpactMetricsState) => {
-            const path = `api/admin/impact-metrics/settings`;
-            const req = createRequest(
-                path,
-                {
-                    method: 'PUT',
-                    body: JSON.stringify(settings),
-                },
-                'updateImpactMetricsSettings',
-            );
-
-            return makeRequest(req.caller, req.id);
-        },
-        [makeRequest, createRequest],
-    );
-
-    return {
-        updateSettings,
-        errors,
-        loading,
-    };
-};

frontend/src/hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts

@@ -0,0 +1,21 @@
+import type { ImpactMetricsConfigListSchema } from 'openapi/index.js';
+import { fetcher, useApiGetter } from '../useApiGetter/useApiGetter.js';
+import { formatApiPath } from 'utils/formatPath';
+
+/**
+ * Get all impact metrics configurations now associated with any feature flag.
+ */
+export const useImpactMetricsConfig = () => {
+    const PATH = `api/admin/impact-metrics/config`;
+    const { data, refetch, loading, error } =
+        useApiGetter<ImpactMetricsConfigListSchema>(formatApiPath(PATH), () =>
+            fetcher(formatApiPath(PATH), 'impactMetricsConfig'),
+        );
+
+    return {
+        configs: data?.configs || [],
+        refetch,
+        loading,
+        error,
+    };
+};

frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts

@@ -1,18 +0,0 @@
-import { fetcher, useApiGetter } from '../useApiGetter/useApiGetter.js';
-import { formatApiPath } from 'utils/formatPath';
-import type { DisplayImpactMetricsState } from 'component/impact-metrics/types.ts';
-
-export const useImpactMetricsSettings = () => {
-    const PATH = `api/admin/impact-metrics/settings`;
-    const { data, refetch, loading, error } =
-        useApiGetter<DisplayImpactMetricsState>(formatApiPath(PATH), () =>
-            fetcher(formatApiPath(PATH), 'Impact metrics settings'),
-        );
-
-    return {
-        settings: data || { charts: [], layout: [] },
-        refetch,
-        loading,
-        error,
-    };
-};