From afc37ca3af59124a33f65e80479fb242cf8d394f Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 21 Oct 2021 15:44:27 +0200 Subject: [PATCH 1/3] Add non-reliable websockets support (#1497) * Add non-reliable websockets support * Remove unused properties Co-authored-by: Mattermod Co-authored-by: Scott Bishel --- webapp/src/wsclient.ts | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/webapp/src/wsclient.ts b/webapp/src/wsclient.ts index 8fccb33c5..979d92739 100644 --- a/webapp/src/wsclient.ts +++ b/webapp/src/wsclient.ts @@ -34,10 +34,10 @@ export const ACTION_UPDATE_CLIENT_CONFIG = 'UPDATE_CLIENT_CONFIG' export interface MMWebSocketClient { conn: WebSocket | null; sendMessage(action: string, data: any, responseCallback?: () => void): void /* eslint-disable-line @typescript-eslint/no-explicit-any */ + setFirstConnectCallback(callback: () => void): void setReconnectCallback(callback: () => void): void setErrorCallback(callback: (event: Event) => void): void setCloseCallback(callback: (connectFailCount: number) => void): void - connectionId: string } type OnChangeHandler = (client: WSClient, blocks: Block[]) => void @@ -57,8 +57,6 @@ class WSClient { onChange: OnChangeHandler[] = [] onError: OnErrorHandler[] = [] onConfigChange: OnConfigChangeHandler[] = [] - private mmWSMaxRetries = 100 - private mmWSRetryDelay = 300 private notificationDelay = 100 private reopenDelay = 3000 private updatedBlocks: Block[] = [] @@ -163,10 +161,19 @@ class WSClient { open(): void { if (this.client !== null) { // configure the Mattermost websocket client callbacks + const onConnect = () => { + Utils.log('WSClient in plugin mode, reusing Mattermost WS connection') + + for (const handler of this.onStateChange) { + handler(this, 'open') + } + this.state = 'open' + } + const onReconnect = () => { Utils.logWarn('WSClient reconnected') - this.open() + onConnect() for (const handler of this.onReconnect) { handler(this) } @@ -204,34 +211,11 @@ class WSClient { } } + this.client.setFirstConnectCallback(onConnect) this.client.setErrorCallback(onError) this.client.setCloseCallback(onClose) this.client.setReconnectCallback(onReconnect) - // WSClient needs to ensure that the Mattermost client has - // correctly stablished the connection before opening - let retries = 0 - const setPluginOpen = () => { - if (this.client?.connectionId !== '') { - Utils.log('WSClient in plugin mode, reusing Mattermost WS connection') - - for (const handler of this.onStateChange) { - handler(this, 'open') - } - this.state = 'open' - return - } - - retries++ - if (retries <= this.mmWSMaxRetries) { - Utils.log('WSClient Mattermost Websocket not ready, retrying') - setTimeout(setPluginOpen, this.mmWSRetryDelay) - } else { - Utils.logError('WSClient error on open: Mattermost Websocket client is not ready') - } - } - - setPluginOpen() return } From 9f7428780257bea86f242d1baea7b3d0e02da20d Mon Sep 17 00:00:00 2001 From: Hossein Date: Thu, 21 Oct 2021 10:29:06 -0400 Subject: [PATCH 2/3] fix main (#1619) --- .../__snapshots__/galleryCard.test.tsx.snap | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/webapp/src/components/gallery/__snapshots__/galleryCard.test.tsx.snap b/webapp/src/components/gallery/__snapshots__/galleryCard.test.tsx.snap index f19bb59bc..ca3311994 100644 --- a/webapp/src/components/gallery/__snapshots__/galleryCard.test.tsx.snap +++ b/webapp/src/components/gallery/__snapshots__/galleryCard.test.tsx.snap @@ -38,14 +38,14 @@ exports[`src/components/gallery/GalleryCard with a comment content should match class="MenuWrapper optionsMenu" role="button" > -
-
+
@@ -177,14 +177,14 @@ exports[`src/components/gallery/GalleryCard with an image content should match s class="MenuWrapper optionsMenu" role="button" > -
-
+
@@ -352,14 +352,14 @@ exports[`src/components/gallery/GalleryCard with many contents should match snap class="MenuWrapper optionsMenu" role="button" > -
-
+
@@ -495,14 +495,14 @@ exports[`src/components/gallery/GalleryCard with many images content should matc class="MenuWrapper optionsMenu" role="button" > -
-
+
@@ -640,14 +640,14 @@ exports[`src/components/gallery/GalleryCard without block content return Gallery class="MenuWrapper optionsMenu" role="button" > -
-
+
-
-
+
-
-
+
-
-
+
-
-
+
From d22fe7fbc030177ae353c612410bcc31bbc3b3b9 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 21 Oct 2021 18:50:19 +0200 Subject: [PATCH 3/3] 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 --- webapp/src/undoManager.test.ts | 53 ++++++++++++++++++++++++++++++++++ webapp/src/undomanager.ts | 24 ++++++++++----- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/webapp/src/undoManager.test.ts b/webapp/src/undoManager.test.ts index 87536a842..375fed535 100644 --- a/webapp/src/undoManager.test.ts +++ b/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 = {} + + const newBlock = await undoManager.perform( + async () => { + const responseId = blockIds.pop() // every time we run the action a new ID is obtained + const block: Record = {id: responseId, title: 'Sample'} + blocks[block.id] = block + return block + }, + async (block: Record) => { + 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) diff --git a/webapp/src/undomanager.ts b/webapp/src/undomanager.ts index 303987106..3b6046ec5 100644 --- a/webapp/src/undomanager.ts +++ b/webapp/src/undomanager.ts @@ -2,10 +2,11 @@ // See LICENSE.txt for license information. interface UndoCommand { checkpoint: number - undo: () => Promise + undo: (value?: any) => Promise redo: () => Promise 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, - undo: () => Promise, + redo: () => Promise, + undo: (value?: any) => Promise, description?: string, groupId?: string, isDiscardable = false, - ): Promise { - await redo() - return this.registerUndo({undo, redo}, description, groupId, isDiscardable) + ): Promise { + const value = await redo() + this.registerUndo({undo, redo}, description, groupId, value, isDiscardable) + return value } registerUndo( command: { - undo: () => Promise, + undo: (value?: any) => Promise, redo: () => Promise }, 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)