From e5c24117f0afed563e096612c5dc723fa89ab086 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Mon, 8 Sep 2025 15:21:47 +0200 Subject: [PATCH] refactor: global impact metrics saving --- .../impact-metrics/GridLayoutWrapper.tsx | 6 +- .../impact-metrics/ImpactMetrics.tsx | 34 +++--- .../hooks/useImpactMetricsState.ts | 115 ++++++------------ .../useImpactMetricsApi.ts | 4 +- .../useImpactMetricsSettingsApi.ts | 3 + .../useImpactMetricsConfig.ts | 21 ++++ .../useImpactMetricsSettings.ts | 3 + 7 files changed, 90 insertions(+), 96 deletions(-) create mode 100644 frontend/src/hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts diff --git a/frontend/src/component/impact-metrics/GridLayoutWrapper.tsx b/frontend/src/component/impact-metrics/GridLayoutWrapper.tsx index 8b1f3ef4ca..c1f70bb83d 100644 --- a/frontend/src/component/impact-metrics/GridLayoutWrapper.tsx +++ b/frontend/src/component/impact-metrics/GridLayoutWrapper.tsx @@ -134,8 +134,10 @@ export const GridLayoutWrapper: FC = ({ Number.parseInt(theme.spacing(2)), ]} containerPadding={[0, 0]} - isDraggable={!isMobileBreakpoint} - isResizable={!isMobileBreakpoint} + // isDraggable={!isMobileBreakpoint} + // isResizable={!isMobileBreakpoint} + isDraggable={false} + isResizable={false} onLayoutChange={handleLayoutChange} resizeHandles={['se']} draggableHandle='.grid-item-drag-handle' diff --git a/frontend/src/component/impact-metrics/ImpactMetrics.tsx b/frontend/src/component/impact-metrics/ImpactMetrics.tsx index 4e7c2eba6a..3b91a31c69 100644 --- a/frontend/src/component/impact-metrics/ImpactMetrics.tsx +++ b/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,7 @@ export const ImpactMetrics: FC = () => { addChart, updateChart, deleteChart, - updateLayout, + // updateLayout, } = useImpactMetricsState(); const { @@ -91,16 +90,16 @@ export const ImpactMetrics: FC = () => { } }; - const handleLayoutChange = useCallback( - async (layout: any[]) => { - try { - await updateLayout(layout as LayoutItem[]); - } catch (error) { - setToastApiError(formatUnknownError(error)); - } - }, - [updateLayout, setToastApiError], - ); + // 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) => { @@ -127,9 +126,10 @@ export const ImpactMetrics: FC = () => { onEdit={handleEditChart} onDelete={handleDeleteChart} dragHandle={ - - - + null + // + // + // } /> ), @@ -196,7 +196,7 @@ export const ImpactMetrics: FC = () => { ) : charts.length > 0 ? ( ) : null} diff --git a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts index 55e427ea44..a2ddabc3ac 100644 --- a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts +++ b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts @@ -1,7 +1,7 @@ -import { useCallback } from 'react'; -import { useImpactMetricsSettings } from 'hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.js'; -import { useImpactMetricsSettingsApi } from 'hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.js'; -import type { ChartConfig, ImpactMetricsState, LayoutItem } from '../types.ts'; +import { useCallback, useMemo } from 'react'; +import type { ChartConfig } from '../types.ts'; +import { useImpactMetricsConfig } from 'hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts'; +import { useImpactMetricsApi } from 'hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsApi.ts'; /** * "Select all" represents all current and future labels. @@ -11,106 +11,71 @@ export const METRIC_LABELS_SELECT_ALL = '*'; export const useImpactMetricsState = () => { const { - settings, - loading: settingsLoading, - error: settingsError, + configs, + loading: configLoading, + error: configError, refetch, - } = useImpactMetricsSettings(); + } = useImpactMetricsConfig(); + + const { layout, charts } = useMemo( + () => ({ + layout: configs.map((config, index) => ({ + i: config.id, + x: 0, + y: index * 4, + w: 6, + h: 4, + 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) => { - const newChart: ChartConfig = { - ...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); + await createImpactMetric(config); refetch(); }, - [settings, updateSettings, refetch], + [createImpactMetric, refetch], ); const updateChart = useCallback( - async (id: string, updates: Partial) => { - const updatedCharts = settings.charts.map((chart) => - chart.id === id ? { ...chart, ...updates } : chart, - ); - const newState: ImpactMetricsState = { - charts: updatedCharts, - layout: settings.layout, - }; - await updateSettings(newState); + async (id: string, updates: Omit) => { + await createImpactMetric({ ...updates, id }); refetch(); }, - [settings, updateSettings, refetch], + [configs, createImpactMetric, refetch], ); const deleteChart = useCallback( async (id: string) => { - const newState: ImpactMetricsState = { - charts: settings.charts.filter((chart) => chart.id !== id), - layout: settings.layout.filter((item) => item.i !== id), - }; - await updateSettings(newState); + await deleteImpactMetric(id); refetch(); }, - [settings, updateSettings, refetch], - ); - - const updateLayout = useCallback( - async (newLayout: LayoutItem[]) => { - const newState: ImpactMetricsState = { - charts: settings.charts, - layout: newLayout, - }; - await updateSettings(newState); - refetch(); - }, - [settings, updateSettings, refetch], + [configs, deleteImpactMetric, refetch], ); return { - charts: settings.charts || [], - layout: settings.layout || [], - loading: settingsLoading || actionLoading, + charts, + layout, + loading: configLoading || actionLoading, error: - settingsError || Object.keys(actionErrors).length > 0 + configError || Object.keys(actionErrors).length > 0 ? actionErrors : undefined, addChart, updateChart, deleteChart, - updateLayout, }; }; diff --git a/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsApi.ts b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsApi.ts index 5bbe66b408..f0a5938942 100644 --- a/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsApi.ts +++ b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/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 { diff --git a/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts index a0e562313d..fc5a42578d 100644 --- a/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts +++ b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts @@ -2,6 +2,9 @@ import { useCallback } from 'react'; import useAPI from '../useApi/useApi.js'; import type { ImpactMetricsState } from 'component/impact-metrics/types.ts'; +/** + * @deprecated use `useImpactMetricsApi()` instead + */ export const useImpactMetricsSettingsApi = () => { const { makeRequest, createRequest, errors, loading } = useAPI({ propagateErrors: true, diff --git a/frontend/src/hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts b/frontend/src/hooks/api/getters/useImpactMetricsConfig/useImpactMetricsConfig.ts new file mode 100644 index 0000000000..b8186f0344 --- /dev/null +++ b/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(formatApiPath(PATH), () => + fetcher(formatApiPath(PATH), 'impactMetricsConfig'), + ); + + return { + configs: data?.configs || [], + refetch, + loading, + error, + }; +}; diff --git a/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts b/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts index 3faa89a942..dcae6e1b03 100644 --- a/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts +++ b/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts @@ -2,6 +2,9 @@ import { fetcher, useApiGetter } from '../useApiGetter/useApiGetter.js'; import { formatApiPath } from 'utils/formatPath'; import type { DisplayImpactMetricsState } from 'component/impact-metrics/types.ts'; +/** + * @deprecated use `useImpactMetricsConfig()` instead + */ export const useImpactMetricsSettings = () => { const PATH = `api/admin/impact-metrics/settings`; const { data, refetch, loading, error } =