diff --git a/frontend/src/component/impact-metrics/ImpactMetrics.tsx b/frontend/src/component/impact-metrics/ImpactMetrics.tsx index 3cbbbc5027..6c6ba9cff6 100644 --- a/frontend/src/component/impact-metrics/ImpactMetrics.tsx +++ b/frontend/src/component/impact-metrics/ImpactMetrics.tsx @@ -9,6 +9,8 @@ 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 useToast from 'hooks/useToast'; +import { formatUnknownError } from 'utils/formatUnknownError'; const StyledEmptyState = styled(Paper)(({ theme }) => ({ textAlign: 'center', @@ -21,9 +23,18 @@ const StyledEmptyState = styled(Paper)(({ theme }) => ({ export const ImpactMetrics: FC = () => { const [modalOpen, setModalOpen] = useState(false); const [editingChart, setEditingChart] = useState(); + const { setToastApiError } = useToast(); - const { charts, layout, addChart, updateChart, deleteChart, updateLayout } = - useImpactMetricsState(); + const { + charts, + layout, + loading: settingsLoading, + error: settingsError, + addChart, + updateChart, + deleteChart, + updateLayout, + } = useImpactMetricsState(); const { metadata, @@ -51,20 +62,39 @@ export const ImpactMetrics: FC = () => { setModalOpen(true); }; - const handleSaveChart = (config: Omit) => { - if (editingChart) { - updateChart(editingChart.id, config); - } else { - addChart(config); + const handleSaveChart = async (config: Omit) => { + try { + if (editingChart) { + await updateChart(editingChart.id, config); + } else { + await addChart(config); + } + setModalOpen(false); + } catch (error) { + setToastApiError(formatUnknownError(error)); } - setModalOpen(false); }; const handleLayoutChange = useCallback( - (layout: any[]) => { - updateLayout(layout as LayoutItem[]); + async (layout: any[]) => { + try { + await updateLayout(layout as LayoutItem[]); + } catch (error) { + setToastApiError(formatUnknownError(error)); + } }, - [updateLayout], + [updateLayout, setToastApiError], + ); + + const handleDeleteChart = useCallback( + async (id: string) => { + try { + await deleteChart(id); + } catch (error) { + setToastApiError(formatUnknownError(error)); + } + }, + [deleteChart], ); const gridItems: GridItem[] = useMemo( @@ -79,7 +109,7 @@ export const ImpactMetrics: FC = () => { ), w: existingLayout?.w ?? 6, @@ -92,10 +122,11 @@ export const ImpactMetrics: FC = () => { maxH: 8, }; }), - [charts, layout, handleEditChart, deleteChart], + [charts, layout, handleEditChart, handleDeleteChart], ); - const hasError = metadataError; + const hasError = metadataError || settingsError; + const isLoading = metadataLoading || settingsLoading; return ( <> @@ -111,14 +142,14 @@ export const ImpactMetrics: FC = () => { variant='contained' startIcon={} onClick={handleAddChart} - disabled={metadataLoading || !!hasError} + disabled={isLoading || !!hasError} > Add Chart } /> - {charts.length === 0 && !metadataLoading && !hasError ? ( + {charts.length === 0 && !isLoading && !hasError ? ( No charts configured @@ -135,7 +166,7 @@ export const ImpactMetrics: FC = () => { variant='contained' startIcon={} onClick={handleAddChart} - disabled={metadataLoading || !!hasError} + disabled={isLoading || !!hasError} > Add Chart @@ -153,7 +184,7 @@ export const ImpactMetrics: FC = () => { onSave={handleSaveChart} initialConfig={editingChart} metricSeries={metricSeries} - loading={metadataLoading} + loading={metadataLoading || settingsLoading} /> ); diff --git a/frontend/src/component/impact-metrics/ImpactMetricsChart.tsx b/frontend/src/component/impact-metrics/ImpactMetricsChart.tsx index eacb969c2d..27476e8829 100644 --- a/frontend/src/component/impact-metrics/ImpactMetricsChart.tsx +++ b/frontend/src/component/impact-metrics/ImpactMetricsChart.tsx @@ -184,7 +184,11 @@ export const ImpactMetricsChart: FC = ({ background: theme.palette.background.elevation1, })} > - + {debug.query} diff --git a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.test.tsx b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.test.tsx index c546ad4f32..f113df5347 100644 --- a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.test.tsx +++ b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.test.tsx @@ -1,123 +1,332 @@ +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { render } from 'utils/testRenderer'; +import { testServerRoute, testServerSetup } from 'utils/testServer'; import { useImpactMetricsState } from './useImpactMetricsState.ts'; -import { Route, Routes } from 'react-router-dom'; -import { createLocalStorage } from '../../../utils/createLocalStorage.ts'; import type { FC } from 'react'; import type { ImpactMetricsState } from '../types.ts'; -const TestComponent: FC = () => { - const { charts, layout } = useImpactMetricsState(); +const server = testServerSetup(); + +const TestComponent: FC<{ + enableActions?: boolean; +}> = ({ enableActions = false }) => { + const { + charts, + layout, + loading, + error, + addChart, + updateChart, + deleteChart, + } = useImpactMetricsState(); return (
{charts.length} {layout.length} + {loading.toString()} + {error ? 'has-error' : 'no-error'} + + {enableActions && ( + + )} + + {enableActions && charts.length > 0 && ( + + )} + + {enableActions && charts.length > 0 && ( + + )}
); }; -const TestWrapper = () => ( - - } /> - -); +const mockSettings: ImpactMetricsState = { + charts: [ + { + id: 'test-chart', + selectedSeries: 'test-series', + selectedRange: 'day' as const, + beginAtZero: true, + showRate: false, + selectedLabels: {}, + title: 'Test Chart', + }, + ], + layout: [ + { + i: 'test-chart', + x: 0, + y: 0, + w: 6, + h: 4, + minW: 4, + minH: 2, + maxW: 12, + maxH: 8, + }, + ], +}; + +const emptySettings: ImpactMetricsState = { + charts: [], + layout: [], +}; describe('useImpactMetricsState', () => { beforeEach(() => { - window.localStorage.clear(); + testServerRoute(server, '/api/admin/ui-config', {}); }); - it('loads state from localStorage to the URL after opening page without URL state', async () => { - const { setValue } = createLocalStorage( - 'impact-metrics-state', - { - charts: [], - layout: [], - }, + it('loads settings from API', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + mockSettings, ); - setValue({ - charts: [ - { - id: 'test-chart', - selectedSeries: 'test-series', - selectedRange: 'day' as const, - beginAtZero: true, - showRate: false, - selectedLabels: {}, - title: 'Test Chart', - }, - ], - layout: [ - { - i: 'test-chart', - x: 0, - y: 0, - w: 6, - h: 4, - minW: 4, - minH: 2, - maxW: 12, - maxH: 8, - }, - ], + render(); + + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('1'); + expect(screen.getByTestId('layout-count')).toHaveTextContent('1'); + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + expect(screen.getByTestId('error')).toHaveTextContent('no-error'); }); - - render(, { route: '/impact-metrics' }); - - expect(window.location.href).toContain('charts='); - expect(window.location.href).toContain('layout='); }); - it('does not modify URL when URL already contains data', async () => { - const { setValue } = createLocalStorage( - 'impact-metrics-state', + it('handles empty settings', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + emptySettings, + ); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('0'); + expect(screen.getByTestId('layout-count')).toHaveTextContent('0'); + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + expect(screen.getByTestId('error')).toHaveTextContent('no-error'); + }); + }); + + it('handles API errors', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + { message: 'Server error' }, + 'get', + 500, + ); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('error')).toHaveTextContent('has-error'); + }); + }); + + it('adds a chart successfully', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + emptySettings, + ); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('0'); + }); + + testServerRoute( + server, + '/api/admin/impact-metrics/settings', { - charts: [], - layout: [], + charts: [ + { + id: 'new-chart-id', + selectedSeries: 'test-series', + selectedRange: 'day', + beginAtZero: true, + showRate: false, + selectedLabels: {}, + 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, ); - setValue({ - charts: [ - { - id: 'old-chart', - selectedSeries: 'old-series', - selectedRange: 'day' as const, - beginAtZero: true, - showRate: false, - selectedLabels: {}, - title: 'Old Chart', - }, - ], - layout: [], - }); - - const urlCharts = btoa( - JSON.stringify([ - { - id: 'url-chart', - selectedSeries: 'url-series', - selectedRange: 'day', - beginAtZero: true, - showRate: false, - selectedLabels: {}, - title: 'URL Chart', - }, - ]), + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + { + charts: [ + { + id: 'new-chart-id', + selectedSeries: 'test-series', + selectedRange: 'day', + beginAtZero: true, + showRate: false, + selectedLabels: {}, + 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, ); - render(, { - route: `/impact-metrics?charts=${encodeURIComponent(urlCharts)}`, + const addButton = screen.getByTestId('add-chart'); + await userEvent.click(addButton); + + await waitFor( + () => { + expect(screen.getByTestId('charts-count')).toHaveTextContent( + '1', + ); + }, + { timeout: 5000 }, + ); + }); + + it('updates a chart successfully', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + mockSettings, + ); + + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + { + charts: [ + { + ...mockSettings.charts[0], + title: 'Updated Chart', + }, + ], + layout: mockSettings.layout, + }, + 'put', + 200, + ); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('1'); }); - const urlParams = new URLSearchParams(window.location.search); - const chartsParam = urlParams.get('charts'); + const updateButton = screen.getByTestId('update-chart'); + await userEvent.click(updateButton); - expect(chartsParam).toBeTruthy(); + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('1'); + }); + }); - const decodedCharts = JSON.parse(atob(chartsParam!)); - expect(decodedCharts[0].id).toBe('url-chart'); - expect(decodedCharts[0].id).not.toBe('old-chart'); + it('deletes a chart successfully', async () => { + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + mockSettings, + ); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('charts-count')).toHaveTextContent('1'); + }); + + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + emptySettings, + 'put', + 200, + ); + + testServerRoute( + server, + '/api/admin/impact-metrics/settings', + emptySettings, + 'get', + 200, + ); + + const deleteButton = screen.getByTestId('delete-chart'); + await userEvent.click(deleteButton); + + await waitFor( + () => { + expect(screen.getByTestId('charts-count')).toHaveTextContent( + '0', + ); + }, + { timeout: 5000 }, + ); }); }); diff --git a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts index 58069dba09..b2e8caec52 100644 --- a/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts +++ b/frontend/src/component/impact-metrics/hooks/useImpactMetricsState.ts @@ -1,72 +1,40 @@ -import { useCallback, useMemo } from 'react'; -import { withDefault } from 'use-query-params'; -import { usePersistentTableState } from 'hooks/usePersistentTableState'; +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'; -const createArrayParam = () => ({ - encode: (items: T[]): string => - items.length > 0 ? btoa(JSON.stringify(items)) : '', - - decode: (value: string | (string | null)[] | null | undefined): T[] => { - if (typeof value !== 'string' || !value) return []; - try { - return JSON.parse(atob(value)); - } catch { - return []; - } - }, -}); - -const ChartsParam = createArrayParam(); -const LayoutParam = createArrayParam(); - export const useImpactMetricsState = () => { - const stateConfig = { - charts: withDefault(ChartsParam, []), - layout: withDefault(LayoutParam, []), - }; + const { + settings, + loading: settingsLoading, + error: settingsError, + refetch, + } = useImpactMetricsSettings(); - const [tableState, setTableState] = usePersistentTableState( - 'impact-metrics-state', - stateConfig, - ); - - const currentState: ImpactMetricsState = useMemo( - () => ({ - charts: tableState.charts || [], - layout: tableState.layout || [], - }), - [tableState.charts, tableState.layout], - ); - - const updateState = useCallback( - (newState: ImpactMetricsState) => { - setTableState({ - charts: newState.charts, - layout: newState.layout, - }); - }, - [setTableState], - ); + const { + updateSettings, + loading: actionLoading, + errors: actionErrors, + } = useImpactMetricsSettingsApi(); const addChart = useCallback( - (config: Omit) => { + async (config: Omit) => { const newChart: ChartConfig = { ...config, id: `chart-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, }; const maxY = - currentState.layout.length > 0 + settings.layout.length > 0 ? Math.max( - ...currentState.layout.map((item) => item.y + item.h), + ...settings.layout.map((item) => item.y + item.h), ) : 0; - updateState({ - charts: [...currentState.charts, newChart], + const newState: ImpactMetricsState = { + charts: [...settings.charts, newChart], layout: [ - ...currentState.layout, + ...settings.layout, { i: newChart.id, x: 0, @@ -79,46 +47,61 @@ export const useImpactMetricsState = () => { maxH: 8, }, ], - }); + }; + + await updateSettings(newState); + refetch(); }, - [currentState.charts, currentState.layout, updateState], + [settings, updateSettings, refetch], ); const updateChart = useCallback( - (id: string, updates: Partial) => { - updateState({ - charts: currentState.charts.map((chart) => - chart.id === id ? { ...chart, ...updates } : chart, - ), - layout: currentState.layout, - }); + 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); + refetch(); }, - [currentState.charts, currentState.layout, updateState], + [settings, updateSettings, refetch], ); const deleteChart = useCallback( - (id: string) => { - updateState({ - charts: currentState.charts.filter((chart) => chart.id !== id), - layout: currentState.layout.filter((item) => item.i !== id), - }); + 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); + refetch(); }, - [currentState.charts, currentState.layout, updateState], + [settings, updateSettings, refetch], ); const updateLayout = useCallback( - (newLayout: LayoutItem[]) => { - updateState({ - charts: currentState.charts, + async (newLayout: LayoutItem[]) => { + const newState: ImpactMetricsState = { + charts: settings.charts, layout: newLayout, - }); + }; + await updateSettings(newState); + refetch(); }, - [currentState.charts, updateState], + [settings, updateSettings, refetch], ); return { - charts: currentState.charts || [], - layout: currentState.layout || [], + charts: settings.charts || [], + layout: settings.layout || [], + loading: settingsLoading || actionLoading, + error: + settingsError || Object.keys(actionErrors).length > 0 + ? actionErrors + : undefined, addChart, updateChart, deleteChart, diff --git a/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts new file mode 100644 index 0000000000..a0e562313d --- /dev/null +++ b/frontend/src/hooks/api/actions/useImpactMetricsSettingsApi/useImpactMetricsSettingsApi.ts @@ -0,0 +1,32 @@ +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, + }; +}; diff --git a/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts b/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts new file mode 100644 index 0000000000..6847abc294 --- /dev/null +++ b/frontend/src/hooks/api/getters/useImpactMetricsSettings/useImpactMetricsSettings.ts @@ -0,0 +1,18 @@ +import { fetcher, useApiGetter } from '../useApiGetter/useApiGetter.js'; +import { formatApiPath } from 'utils/formatPath'; +import type { ImpactMetricsState } from 'component/impact-metrics/types.ts'; + +export const useImpactMetricsSettings = () => { + const PATH = `api/admin/impact-metrics/settings`; + const { data, refetch, loading, error } = useApiGetter( + formatApiPath(PATH), + () => fetcher(formatApiPath(PATH), 'Impact metrics settings'), + ); + + return { + settings: data || { charts: [], layout: [] }, + refetch, + loading, + error, + }; +};