diff --git a/src/components/archive/EditStatus.tsx b/src/components/archive/EditStatus.tsx index 3cac3796a0c37e36f978f44b23e834502daaa0c2..73eaaf5a3d3e798e5fcd3afdff2f7d852a1482eb 100644 --- a/src/components/archive/EditStatus.tsx +++ b/src/components/archive/EditStatus.tsx @@ -1,6 +1,7 @@ -import {Button} from '@mui/material' +import {Button, IconButton} from '@mui/material' import {useAsyncFn} from 'react-use' +import {Redo, Undo} from '../../components/icons' import useAuth from '../../hooks/useAuth' import {MSectionResponse} from '../../models/graphResponseModels' import entryRoute from '../../pages/entry/entryRoute' @@ -8,13 +9,14 @@ import {archiveEditApi} from '../../utils/api' import {assert} from '../../utils/utils' import useRoute from '../routing/useRoute' import useRouteData from '../routing/useRouteData' -import {useArchiveChanges} from './useArchive' +import useArchive, {useArchiveChanges} from './useArchive' export default function EditStatus() { const {user} = useAuth() const {reload} = useRoute() const {entry_id} = useRouteData(entryRoute) const {changeStack, clearChangeStack} = useArchiveChanges() + const archive = useArchive() const [saving, save] = useAsyncFn(async () => { const apiChanges = changeStack.map((change) => { @@ -29,6 +31,9 @@ export default function EditStatus() { } delete (apiChange.new_value as MSectionResponse)['m_def'] } + if (apiChange.oldValue) { + delete apiChange['oldValue'] + } return apiChange }) @@ -40,16 +45,30 @@ export default function EditStatus() { }, [changeStack, clearChangeStack, reload]) return ( - <Button - onClick={save} - variant='contained' - disabled={changeStack.length === 0 || saving.loading} - > - {saving.loading - ? 'saving ...' - : changeStack.length === 0 - ? 'no changes to save' - : `save changes`} - </Button> + <> + <IconButton + disabled={!archive.hasChanges()} + onClick={() => archive.undo()} + > + <Undo /> + </IconButton> + <IconButton + disabled={!archive.hasUnDoneChanges()} + onClick={() => archive.redo()} + > + <Redo /> + </IconButton> + <Button + onClick={save} + variant='contained' + disabled={changeStack.length === 0 || saving.loading} + > + {saving.loading + ? 'saving ...' + : changeStack.length === 0 + ? 'no changes to save' + : `save changes`} + </Button> + </> ) } diff --git a/src/components/archive/Section.tsx b/src/components/archive/Section.tsx index a433e0fa5bbeeef99fa7a6f35950fa35af1f5119..a102831363e196349af031efe544df481fc3b55d 100644 --- a/src/components/archive/Section.tsx +++ b/src/components/archive/Section.tsx @@ -62,10 +62,10 @@ export default function Section({path, editable = false}: SectionProps) { const {value} = useArchiveProperty<MSectionResponse>(path) const definition = value?.m_def as SectionDefinition - assert(value !== undefined, 'The section should have been loaded by now') + assert(value !== undefined, 'The section should have been loaded by now.') assert( definition !== undefined, - 'The section definition should have been loaded by now', + 'The section definition should have been loaded by now.', ) return ( diff --git a/src/components/archive/archive.helper.tsx b/src/components/archive/archive.helper.tsx index 98c4b3ca6047a501904d6e90cba93c0dab27079e..39014f3015a6b1cbc233d49824905cc5e70c997b 100644 --- a/src/components/archive/archive.helper.tsx +++ b/src/components/archive/archive.helper.tsx @@ -2,18 +2,17 @@ import {useEffect, useState} from 'react' import {vi} from 'vitest' import {UseDataResult} from '../../hooks/useData' -import {ArchiveChange} from '../../models/entryEditRequestModels' import {MDefResponse, MSectionResponse} from '../../models/graphResponseModels' import {Quantity, Section, SubSection} from '../../utils/metainfo' import {DefaultToObject} from '../../utils/types' import * as useDataForRoute from '../routing/useDataForRoute' import * as useRouteData from '../routing/useRouteData' -import {Archive, allArchives} from './useArchive' +import {Archive, ArchiveChangeWithOldValue, allArchives} from './useArchive' export type MockArchiveArgs = { entryId?: string data?: MSectionResponse - changes?: ArchiveChange[] + changes?: ArchiveChangeWithOldValue[] } /** diff --git a/src/components/archive/useArchive.test.tsx b/src/components/archive/useArchive.test.tsx index 34a1a99b1a95c5f9fc44cbcc8a6010d50fe14947..3683a5e95b2686e28750ad5a1c1b8d8749870e78 100644 --- a/src/components/archive/useArchive.test.tsx +++ b/src/components/archive/useArchive.test.tsx @@ -331,6 +331,109 @@ describe('useArchive', () => { q: 'test', }) }) + + it.each([ + ['quantity set', {q: '*'}, {q: 1}, 'q', 'upsert', 2], + ['nested quantity set', {s: {q: '*'}}, {s: {q: 1}}, 's/q', 'upsert', 2], + [ + 'repeated nested quantity set', + {s: [{q: '*'}]}, + {s: [{q: 1}]}, + 's/0/q', + 'upsert', + 2, + ], + [ + 'repeated nested quantity set unnormalized', + {s: [{q: '*'}]}, + {s: [{q: 1}]}, + 's[0]/q', + 'upsert', + 2, + ], + ['sub-section replace', {s: '*'}, {s: {q: 1}}, 's', 'upsert', {q: 2}], + [ + 'repeated sub-section replace implicit', + {s: '*'}, + {s: [{q: 1}]}, + 's', + 'upsert', + [{q: 2}], + ], + [ + 'repeated sub-section implicit add', + {s: '*'}, + {s: [{q: 1}]}, + 's', + 'upsert', + [{q: 1}, {q: 2}], + ], + [ + 'repeated sub-section implicit remove', + {s: '*'}, + {s: [{q: 1}, {q: 2}]}, + 's', + 'upsert', + [{q: 1}], + ], + [ + 'repeated sub-section explicit replace', + {s: '*'}, + {s: [{q: 1}]}, + 's/0', + 'upsert', + {q: 2}, + ], + [ + 'repeated sub-section explicit add', + {s: '*'}, + {s: [{q: 1}]}, + 's/1', + 'upsert', + {q: 2}, + ], + [ + 'repeated sub-section explicit remove', + {s: '*'}, + {s: [{q: 1}, {q: 2}]}, + 's/1', + 'remove', + undefined, + ], + ])( + 'does undo/redo %s', + (description, request, response, path, action, value) => { + const dispatch = vi.fn() + const changeListener = vi.fn() + const {result} = renderHook(() => useArchive()) + act(() => { + archive.startUpdate(request) + archive.commitUpdate(addDefinitions(response), {}) + }) + const oldValue = result.current.getValue(path) + act(() => { + archive.registerElement(path, dispatch) + archive.registerChangeListener(changeListener) + archive.submitElementChange(path, action as 'upsert' | 'remove', value) + }) + expect(result.current.getValue(path)).toBe(value) + expect(dispatch).toHaveBeenCalledTimes(1) + expect(changeListener).toHaveBeenCalledTimes(1) + expect(result.current.changeStack).toHaveLength(1) + + act(() => result.current.undo()) + expect(result.current.changeStack).toHaveLength(0) + expect(result.current.getValue(path)).toBe(oldValue) + expect(dispatch).toHaveBeenCalledTimes(2) + expect(changeListener).toHaveBeenCalledTimes(2) + + act(() => result.current.redo()) + expect(result.current.changeStack).toHaveLength(1) + expect(result.current.getValue(path)).toBe(value) + expect(dispatch).toHaveBeenCalledTimes(3) + expect(changeListener).toHaveBeenCalledTimes(3) + }, + ) }) describe('useArchiveProperty', () => { diff --git a/src/components/archive/useArchive.tsx b/src/components/archive/useArchive.tsx index 14700a6ed29bacf20913815c328aec61e0f47504..82038f5d5ca3c0172390ed1f1a9a5db08b7fc3e8 100644 --- a/src/components/archive/useArchive.tsx +++ b/src/components/archive/useArchive.tsx @@ -21,6 +21,7 @@ export type ArchiveChangeConflict = { type: 'parent' | 'child' | 'exact' change: ArchiveChange } +export type ArchiveChangeWithOldValue = ArchiveChange & {oldValue?: unknown} class MultiMap<K, V> { map = new Map<K, V[]>() @@ -67,7 +68,8 @@ export class Archive { elements: MultiMap<string, ArchivePropertyDispatch> changeListener: (() => void)[] archive: MSectionResponse - changeStack: ArchiveChange[] + changeStack: ArchiveChangeWithOldValue[] + redoStack: ArchiveChangeWithOldValue[] loading: boolean responsesWithReferencedArchives: MultiMap<string, GraphResponse> currentUpdateRequests: MSectionRequest | undefined @@ -77,6 +79,7 @@ export class Archive { this.changeListener = [] this.archive = {} this.changeStack = [] + this.redoStack = [] this.responsesWithReferencedArchives = new MultiMap() this.loading = false this.currentUpdateRequests = undefined @@ -103,6 +106,96 @@ export class Archive { this.changeListener.forEach((listener) => listener()) } + hasUnDoneChanges() { + return this.redoStack.length > 0 + } + + hasChanges() { + return this.changeStack.length > 0 + } + + /** + * Removes the last change from the change stack and + * - pushes it to the redo stack + * - applies the change to the archive + * - notifies all components that have registered to the particular path + * via `useArchiveProperty` (and `registerElement`). + * - notifies all components that have registered to via `registerChangeListener`. + */ + undo() { + const change = this.changeStack.pop() + assert(change, 'No changes to undo.') + this.applyAndDispatchValue(change.path, change.oldValue) + this.redoStack.push(change) + if (change) { + this.changeListener.forEach((listener) => listener()) + } + } + + /** + * Removes the last change from the redo stack and + * - pushes it back to the change stack + * - applies the change (old value) to the archive + * - notifies all components that have registered to the particular path + * via `useArchiveProperty` (and `registerElement`). + * - notifies all components that have registered to via `registerChangeListener`. + */ + redo() { + const change = this.redoStack.pop() + assert(change, 'No changes to redo.') + this.applyAndDispatchValue(change.path, change.new_value) + this.changeStack.push(change) + if (change) { + this.changeListener.forEach((listener) => listener()) + } + } + + /** + * Perform a change on the archive data and notify all components that have + * registered to the particular path via `useArchiveProperty` (and + * `registerElement`). + * + * @param path The path to the property. + * @param value The new value to set. + * @returns The old value of the property. + */ + private applyAndDispatchValue(path: string, value: unknown) { + // apply + let current = this.archive + const parts = normalizeArchivePath(path) + const subSections = parts.slice(0, -1) + for (const key of subSections) { + current = current[key] as MSectionResponse + } + const quantityOrIndex = parts[parts.length - 1] + const oldValue = current[quantityOrIndex] + current[quantityOrIndex] = value + + // additionally remove deleted sub sections at the end of repeated + // sub sections + if ( + value === undefined && + Array.isArray(current) && + current.length === parseInt(quantityOrIndex) + 1 + ) { + current.pop() + } + + // additionally dispatch the repeated sub section if a section was + // added or removed + if (Array.isArray(current)) { + for (const dispatch of this.elements.get(subSections.join('/'))) { + dispatch({value: current, loading: false}) + } + } + // dispatch change + for (const dispatch of this.elements.get(path)) { + dispatch({value, loading: false}) + } + + return oldValue + } + /** * Submits a change to the archive. This will update the archive, * dispatch the change to all registered elements, and keep track @@ -120,37 +213,28 @@ export class Archive { value: unknown, index?: number, ) { - // apply the value to the archive - let current = this.archive - const parts = normalizeArchivePath(path) - const subSections = parts.slice(0, -1) - for (const key of subSections) { - current = current[key] as MSectionResponse - } - const valueIndex = parts[parts.length - 1] - current[valueIndex] = value - - // dispatch change - for (const dispatch of this.elements.get(path)) { - dispatch({value, loading: false}) - } + assert(!this.loading) + const oldValue = this.applyAndDispatchValue(path, value) // translate the change into an ArchiveChange and keep the change in // this.changes and this.changeStack let change if (index !== undefined) { assert(Array.isArray(value), 'Value must be an array for indexed changes') + assert(Array.isArray(oldValue) || oldValue === undefined) change = { path: `${path}/${index}`, new_value: action === 'remove' ? undefined : {...value[index]}, + oldValue: action === 'remove' ? oldValue?.[index] : undefined, action, - } satisfies ArchiveChange + } satisfies ArchiveChangeWithOldValue } else { change = { path, new_value: action === 'remove' ? undefined : value, + oldValue: oldValue, action, - } satisfies ArchiveChange + } satisfies ArchiveChangeWithOldValue if (this.changeStack.length > 0) { const lastChange = this.changeStack[this.changeStack.length - 1] @@ -221,7 +305,7 @@ export class Archive { /** * Detects conflicts between changing the given path and the changes in the - * current chanage stack. The function returns 'parent' if the path is a + * current change stack. The function returns 'parent' if the path is a * prefix to a path in the change stack, 'child' if a path in the change stack * is a prefix to the given path, 'none' if there is no conflict, and 'exact' * of the path is exactly the same as a path in the change stack. @@ -640,7 +724,7 @@ export default function useArchive() { } export type ArchiveChanges = { - changeStack: ArchiveChange[] + changeStack: ArchiveChangeWithOldValue[] clearChangeStack: () => void // undo: () => void, // redo: () => void, @@ -652,7 +736,7 @@ export type ArchiveChanges = { */ export function useArchiveChanges() { const archive = useArchive() - const [changeStack, setChangeStack] = useState<ArchiveChange[]>([ + const [changeStack, setChangeStack] = useState<ArchiveChangeWithOldValue[]>([ ...archive.changeStack, ]) @@ -711,6 +795,14 @@ export type ArchiveProperty<Type, DefinitionType extends Property> = { resolveRef: (ref: string) => MSectionResponse | undefined } +/** + * Normalizes the use of repeated sub section syntax in paths. The + * normalized syntax uses index segments (e.g. `section/0`) instead + * of having the index in the sub section segment (e.g. `section[0]`). + * + * @param path The path to normalize. + * @returns The normalized path. + */ function normalizeArchivePath(path: string) { const result: string[] = [] path.split('/').forEach((segment) => { @@ -733,7 +825,7 @@ function normalizeArchivePath(path: string) { * root to the property. The paths use `/` as separator and align with the syntax * of the graph API or archive references. * - * What it means that an propery changes depends on the + * What it means that an property changes depends on the * kind of property. A quantity value changes if the value changes, a (repeating) * sub-section changes if a/the section is added, removed, or replaced. * @@ -746,6 +838,7 @@ function normalizeArchivePath(path: string) { export function useArchiveProperty<T = unknown, D extends Property = Property>( path: string, ): ArchiveProperty<T, D> { + path = normalizeArchivePath(path).join('/') const archive = useArchive() const previousPath = usePrevious(path) diff --git a/src/components/icons.tsx b/src/components/icons.tsx index 97bc8b78dbc91d9d79fc932347e265bb49ac1ec3..c74127c22e565b55c9e2eed1a62e40d7b98f397c 100644 --- a/src/components/icons.tsx +++ b/src/components/icons.tsx @@ -28,7 +28,9 @@ import PieChartIcon from '@mui/icons-material/PieChart' import PublicIcon from '@mui/icons-material/Public' import PushPinIcon from '@mui/icons-material/PushPin' import QuestionMarkIcon from '@mui/icons-material/QuestionMark' +import RedoIcon from '@mui/icons-material/Redo' import SearchIcon from '@mui/icons-material/Search' +import UndoIcon from '@mui/icons-material/Undo' import UploadIcon from '@mui/icons-material/Upload' import VisibilityIcon from '@mui/icons-material/Visibility' import VisibilityOffIcon from '@mui/icons-material/VisibilityOff' @@ -68,6 +70,8 @@ export const Help = HelpIcon export const Dataset = DatasetIcon export const Pin = PushPinIcon export const Group = GroupIcon +export const Undo = UndoIcon +export const Redo = RedoIcon export function ToggleColorMode() { const theme = useTheme() diff --git a/src/pages/entry/EntryOverview.tsx b/src/pages/entry/EntryOverview.tsx index 7c5f61010daef079be9f846e3f266647b41e3ddd..e48971a14709b0edfa450061b7db43ca57f7abac 100644 --- a/src/pages/entry/EntryOverview.tsx +++ b/src/pages/entry/EntryOverview.tsx @@ -82,15 +82,19 @@ function EntryOverviewEditor() { // While this works for now, it will be a problem if the startUpdate function // will cause state updates in the future as those are not allowed during // render. - if (request !== usePrevious(request)) { + const requestHasChanged = request !== usePrevious(request) + if (requestHasChanged) { archive.startUpdate(request.archive as MSectionRequest, true) } const onBeforeFetch = useCallback(() => { + if (!archive.loading) { + archive.startUpdate(request.archive as MSectionRequest, true) + } return () => { archive.abortUpdate() } - }, [archive]) + }, [archive, request]) const onFetch = useCallback( (data: EntryResponse, fullResponse: GraphResponse) => {