From 5a167a408cc66c5bc78772ee3661cf42eb1a0531 Mon Sep 17 00:00:00 2001 From: karilint Date: Wed, 3 Jun 2026 14:26:19 +0300 Subject: [PATCH] Address field update history review --- .../DetailView/common/FieldUpdateHistory.tsx | 24 ++-- .../DetailView/FieldUpdateHistory.test.tsx | 104 ++++++++++++++---- 2 files changed, 94 insertions(+), 34 deletions(-) diff --git a/frontend/src/components/DetailView/common/FieldUpdateHistory.tsx b/frontend/src/components/DetailView/common/FieldUpdateHistory.tsx index 3060e129..838e2768 100644 --- a/frontend/src/components/DetailView/common/FieldUpdateHistory.tsx +++ b/frontend/src/components/DetailView/common/FieldUpdateHistory.tsx @@ -1,6 +1,6 @@ import HistoryIcon from '@mui/icons-material/History' import { Box, Card, Divider, IconButton, Popover, Tooltip, Typography } from '@mui/material' -import { createContext, useContext, useMemo, useState, type Context, type MouseEvent } from 'react' +import { useContext, useMemo, useState, type Context, type MouseEvent } from 'react' import { DetailContext, type DetailContextType } from '@/components/DetailView/Context/DetailContext' import type { AnyReference, UpdateLog } from '@/shared/types' import { ReferenceList } from './ReferenceList' @@ -14,15 +14,13 @@ type FieldUpdate = { container: UpdateContainer } -const SafeDetailContext = - (DetailContext as unknown as Context> | null> | undefined) ?? - createContext> | null>(null) +const SafeDetailContext = DetailContext as unknown as Context> | null> const isObject = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value) const isUpdateLog = (value: unknown): value is UpdateLog => - isObject(value) && ('column_name' in value || 'old_data' in value || 'new_data' in value || 'log_action' in value) + isObject(value) && 'column_name' in value && 'log_action' in value const isUpdateContainer = (value: unknown): value is UpdateContainer => isObject(value) && Array.isArray(value.updates) && value.updates.every(isUpdateLog) @@ -53,13 +51,6 @@ const getFieldUpdates = (data: unknown, field: string): FieldUpdate[] => })) ) -const formatDate = (value: unknown): string => { - if (!value) return 'No date' - const date = new Date(value as string | number | Date) - if (Number.isNaN(date.getTime())) return formatValue(value) - return date.toISOString().split('T')[0] -} - const formatValue = (value: unknown): string => { if (value === null || value === undefined) return '' if (typeof value === 'string') return value @@ -68,6 +59,13 @@ const formatValue = (value: unknown): string => { return JSON.stringify(value) } +const formatDate = (value: unknown): string => { + if (!value) return 'No date' + const date = new Date(value as string | number | Date) + if (Number.isNaN(date.getTime())) return formatValue(value) + return date.toISOString().split('T')[0] +} + const findFirstBySuffix = (container: UpdateContainer, suffix: string): unknown => { const entry = Object.entries(container).find(([key]) => key.endsWith(suffix)) return entry?.[1] @@ -149,7 +147,7 @@ export const FieldUpdateHistory = ({ field, label }: { field: string; label: str {updates.map(({ log, container }, index) => { const references = collectReferences(container) return ( - + Date: {formatDate(getDate(container))} diff --git a/frontend/src/tests/components/DetailView/FieldUpdateHistory.test.tsx b/frontend/src/tests/components/DetailView/FieldUpdateHistory.test.tsx index eedb3813..1a0a5c22 100644 --- a/frontend/src/tests/components/DetailView/FieldUpdateHistory.test.tsx +++ b/frontend/src/tests/components/DetailView/FieldUpdateHistory.test.tsx @@ -18,32 +18,38 @@ const reference = { }, } +const detailData = { + body_mass: 10, + now_sau: [ + { + sau_date: '2026-05-29', + sau_authorizer: 'ED', + sau_coordinator: 'CO', + sau_comment: 'Updated body mass', + now_sr: [reference], + updates: [ + { + log_id: 1, + column_name: 'body_mass', + log_action: 3, + old_data: '10', + new_data: '12', + }, + ], + }, + ], +} + const contextValue = { - data: { - body_mass: 10, - now_sau: [ - { - sau_date: '2026-05-29', - sau_authorizer: 'ED', - sau_coordinator: 'CO', - sau_comment: 'Updated body mass', - now_sr: [reference], - updates: [ - { - log_id: 1, - column_name: 'body_mass', - log_action: 3, - old_data: '10', - new_data: '12', - }, - ], - }, - ], - }, + data: detailData, mode: { read: true, staging: false, new: false, option: 'read' }, } as unknown as ContextType describe('FieldUpdateHistory', () => { + beforeEach(() => { + jest.restoreAllMocks() + }) + it('shows field-specific update history from detail data', async () => { const user = userEvent.setup() @@ -77,4 +83,60 @@ describe('FieldUpdateHistory', () => { expect(screen.queryByLabelText('Show update history for Diet')).not.toBeInTheDocument() }) + + it('does not show the history icon in edit mode', () => { + render( + + + ]]} /> + + + ) + + expect(screen.queryByLabelText('Show update history for Body mass')).not.toBeInTheDocument() + }) + + it('renders multiple same-field updates without duplicate key warnings', async () => { + const user = userEvent.setup() + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined) + const duplicateLogContext = { + ...contextValue, + data: { + ...detailData, + now_sau: [ + detailData.now_sau[0], + { + ...detailData.now_sau[0], + sau_date: '2026-05-30', + sau_comment: 'Updated body mass again', + updates: [ + { + log_id: 1, + column_name: 'body_mass', + log_action: 3, + old_data: '12', + new_data: '14', + }, + ], + }, + ], + }, + } as unknown as ContextType + + render( + + + ]]} /> + + + ) + + await user.click(screen.getByLabelText('Show update history for Body mass')) + + expect(screen.getByText('Updated body mass')).toBeInTheDocument() + expect(screen.getByText('Updated body mass again')).toBeInTheDocument() + expect(consoleError).not.toHaveBeenCalledWith(expect.stringContaining('Encountered two children with the same key')) + }) })