Modify undo manager to return a value and use it on undo (#1616)

* Modify undo manager to return a value and use it on undo

* Storing the last redo value internally to correctly run undo

* Fix types

* Improve test ensuring redo didn't add original block

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

webapp/src/undoManager.test.ts

@@ -47,6 +47,59 @@ test('Basic undo/redo', async () => {
     expect(undoManager.redoDescription).toBe(undefined)
 })
 
+test('Basic undo/redo response dependant', async () => {
+    expect(undoManager.canUndo).toBe(false)
+    expect(undoManager.canRedo).toBe(false)
+    expect(undoManager.currentCheckpoint).toBe(0)
+
+    const blockIds = [2, 1]
+    const blocks: Record<string, any> = {}
+
+    const newBlock = await undoManager.perform(
+        async () => {
+            const responseId = blockIds.pop() // every time we run the action a new ID is obtained
+            const block: Record<string, any> = {id: responseId, title: 'Sample'}
+            blocks[block.id] = block
+            return block
+        },
+        async (block: Record<string, any>) => {
+            delete blocks[block.id]
+        },
+        'test',
+    )
+
+    // should insert the block and return the new block for its use
+    expect(undoManager.canUndo).toBe(true)
+    expect(undoManager.canRedo).toBe(false)
+    expect(blocks).toHaveProperty('1')
+    expect(blocks[1]).toEqual(newBlock)
+
+    // should correctly remove the block based on the info gathered in
+    // the redo function
+    await undoManager.undo()
+    expect(undoManager.canUndo).toBe(false)
+    expect(undoManager.canRedo).toBe(true)
+    expect(blocks).not.toHaveProperty('1')
+
+    // when redoing, as the function has side effects the new id will
+    // be different
+    await undoManager.redo()
+    expect(undoManager.canUndo).toBe(true)
+    expect(undoManager.canRedo).toBe(false)
+    expect(blocks).not.toHaveProperty('1')
+    expect(blocks).toHaveProperty('2')
+    expect(blocks[2].id).toEqual(2)
+
+    // when undoing, the undo manager has saved the new id internally
+    // and it removes the right block
+    await undoManager.undo()
+    expect(undoManager.canUndo).toBe(false)
+    expect(undoManager.canRedo).toBe(true)
+    expect(blocks).not.toHaveProperty('2')
+
+    await undoManager.clear()
+})
+
 test('Grouped undo/redo', async () => {
     expect(undoManager.canUndo).toBe(false)
     expect(undoManager.canRedo).toBe(false)

webapp/src/undomanager.ts

@@ -2,10 +2,11 @@
 // See LICENSE.txt for license information.
 interface UndoCommand {
     checkpoint: number
-    undo: () => Promise<void>
+    undo: (value?: any) => Promise<void>
     redo: () => Promise<void>
     description?: string
     groupId?: string
+    value?: any
 }
 
 //
@@ -50,30 +51,36 @@ class UndoManager {
         }
         this.isExecuting = true
 
-        await command[action]()
+        if (action === 'redo') {
+            command.value = await command[action]()
+        } else {
+            await command[action](command.value)
+        }
 
         this.isExecuting = false
         return this
     }
 
     async perform(
-        redo: () => Promise<void>,
+        redo: () => Promise<any>,
-        undo: () => Promise<void>,
+        undo: (value?: any) => Promise<void>,
         description?: string,
         groupId?: string,
         isDiscardable = false,
-    ): Promise<UndoManager> {
+    ): Promise<any> {
-        await redo()
+        const value = await redo()
-        return this.registerUndo({undo, redo}, description, groupId, isDiscardable)
+        this.registerUndo({undo, redo}, description, groupId, value, isDiscardable)
+        return value
     }
 
     registerUndo(
         command: {
-            undo: () => Promise<void>,
+            undo: (value?: any) => Promise<void>,
             redo: () => Promise<void>
         },
         description?: string,
         groupId?: string,
+        value?: any,
         isDiscardable = false,
     ): UndoManager {
         if (this.isExecuting) {
@@ -97,6 +104,7 @@ class UndoManager {
             redo: command.redo,
             description,
             groupId,
+            value,
         }
         this.commands.push(internalCommand)