From 9d61681921b1e6d7a502c43d5dbde30fa53bc5c5 Mon Sep 17 00:00:00 2001 From: Alex Peelman Date: Mon, 9 Dec 2024 11:57:37 +0100 Subject: [PATCH 1/4] ISSUE 282 keep history messages in sync with storage --- __tests__/services/ChatHistoryService.test.ts | 19 ++++++++++++++++++- src/services/ChatHistoryService.tsx | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/__tests__/services/ChatHistoryService.test.ts b/__tests__/services/ChatHistoryService.test.ts index 9b6f494..c4ced6e 100644 --- a/__tests__/services/ChatHistoryService.test.ts +++ b/__tests__/services/ChatHistoryService.test.ts @@ -61,13 +61,30 @@ describe("ChatHistoryService", () => { setHistoryStorageValues(settings); await saveChatHistory([mockMessage]); expect(storage.setItem).not.toHaveBeenCalled(); + + const historyMessages = getHistoryMessages(); + expect(historyMessages).toEqual([]); }); it("should save history if not disabled", async () => { setHistoryStorageValues(mockSettings(storageType)); await saveChatHistory([mockMessage]); - + expect(storage.setItem).toHaveBeenCalled(); + + const historyMessages = getHistoryMessages(); + expect(historyMessages).toEqual([mockMessage]); + }); + + it("should retain previous history if not disabled and empty message array is passed", async () => { + setHistoryStorageValues(mockSettings(storageType)); + await saveChatHistory([mockMessage]); + await saveChatHistory([]); + + expect(storage.setItem).toHaveBeenCalledTimes(2); + + const historyMessages = getHistoryMessages(); + expect(historyMessages).toEqual([mockMessage]); }); it("should handle empty message array without errors", async () => { diff --git a/src/services/ChatHistoryService.tsx b/src/services/ChatHistoryService.tsx index 4edb816..1aa65fc 100644 --- a/src/services/ChatHistoryService.tsx +++ b/src/services/ChatHistoryService.tsx @@ -51,6 +51,7 @@ const saveChatHistory = async (messages: Message[]) => { parsedMessages = [...historyMessages.slice(-difference), ...parsedMessages] } + historyMessages = parsedMessages; setHistoryMessages(parsedMessages); } From 4d8f7dd286edbd6ba20a517799a48643936db940 Mon Sep 17 00:00:00 2001 From: Alex Peelman Date: Mon, 9 Dec 2024 16:52:28 +0100 Subject: [PATCH 2/4] ISSUE 282 when restartFlow is called, load history from storage --- .../hooks/internal/useFlowInternal.test.ts | 196 ++++++++++-------- __tests__/services/ChatHistoryService.test.ts | 17 -- src/hooks/internal/useFlowInternal.ts | 26 ++- src/hooks/internal/useMessagesInternal.ts | 2 +- src/services/ChatHistoryService.tsx | 1 - 5 files changed, 123 insertions(+), 119 deletions(-) diff --git a/__tests__/hooks/internal/useFlowInternal.test.ts b/__tests__/hooks/internal/useFlowInternal.test.ts index e040f39..ad7e3be 100644 --- a/__tests__/hooks/internal/useFlowInternal.test.ts +++ b/__tests__/hooks/internal/useFlowInternal.test.ts @@ -7,106 +7,118 @@ import { usePathsContext } from "../../../src/context/PathsContext"; import { useToastsContext } from "../../../src/context/ToastsContext"; import { useBotStatesContext } from "../../../src/context/BotStatesContext"; import { useBotRefsContext } from "../../../src/context/BotRefsContext"; +import { useSettingsContext } from "../../../src/context/SettingsContext"; +import { setHistoryStorageValues } from "../../../src/services/ChatHistoryService"; jest.mock("../../../src/context/MessagesContext"); jest.mock("../../../src/context/PathsContext"); jest.mock("../../../src/context/ToastsContext"); jest.mock("../../../src/context/BotStatesContext"); jest.mock("../../../src/context/BotRefsContext"); +jest.mock("../../../src/context/SettingsContext"); +jest.mock("../../../src/services/ChatHistoryService"); describe("useFlowInternal Hook", () => { const setMessagesMock = jest.fn(); - const setPathsMock = jest.fn(); + const setPathsMock = jest.fn(); const setToastsMock = jest.fn(); - const flowRefMock = { current: { id: "test-flow" } }; - const hasFlowStartedMock = true; - - beforeEach(() => { - jest.clearAllMocks(); - - - (useMessagesContext as jest.Mock).mockReturnValue({ setMessages: setMessagesMock }); - (usePathsContext as jest.Mock).mockReturnValue({ setPaths: setPathsMock }); - (useToastsContext as jest.Mock).mockReturnValue({ setToasts: setToastsMock }); - (useBotRefsContext as jest.Mock).mockReturnValue({ flowRef: flowRefMock }); - (useBotStatesContext as jest.Mock).mockReturnValue({ hasFlowStarted: hasFlowStartedMock }); - }); - - -// Test to ensure initial values (hasFlowStarted and flowRef) are returned correctly from the hook - it("should return initial values from context", () => { - const { result } = renderHook(() => useFlowInternal()); - - expect(result.current.hasFlowStarted).toBe(true); - expect(result.current.getFlow()).toEqual({ id: "test-flow" }); - }); - - // Test to ensure that restartFlow clears messages, toasts, and resets paths - it("should restart the flow by clearing messages, toasts, and resetting paths", () => { - const { result } = renderHook(() => useFlowInternal()); - - act(() => { - result.current.restartFlow(); - }); - - expect(setMessagesMock).toHaveBeenCalledWith([]); - expect(setToastsMock).toHaveBeenCalledWith([]); - expect(setPathsMock).toHaveBeenCalledWith(["start"]); - }); - -// Test to ensure that getFlow returns the current flow from flowRef - it("should get the current flow from flowRef", () => { - const { result } = renderHook(() => useFlowInternal()); - - const flow = result.current.getFlow(); - expect(flow).toEqual({ id: "test-flow" }); - }); - -// Test to ensure that calling restartFlow multiple times works correctly - it("should handle multiple restarts correctly", () => { - const { result } = renderHook(() => useFlowInternal()); - - act(() => { - result.current.restartFlow(); - }); - - act(() => { - result.current.restartFlow(); - }); - - expect(setMessagesMock).toHaveBeenCalledTimes(2); - expect(setToastsMock).toHaveBeenCalledTimes(2); - expect(setPathsMock).toHaveBeenCalledTimes(2); - }); - -// Test to check flow state when hasFlowStarted is false - it("should correctly reflect flow state when it hasn't started", () => { - (useBotStatesContext as jest.Mock).mockReturnValue({ hasFlowStarted: false }); - const { result } = renderHook(() => useFlowInternal()); - - expect(result.current.hasFlowStarted).toBe(false); - }); - -// Test to ensure messages, toasts, and paths are initialized correctly when restarting the flow - it("should initialize messages, toasts, and paths correctly", () => { - const { result } = renderHook(() => useFlowInternal()); - - act(() => { - result.current.restartFlow(); - }); - - expect(setMessagesMock).toHaveBeenCalledWith([]); - expect(setToastsMock).toHaveBeenCalledWith([]); - expect(setPathsMock).toHaveBeenCalledWith(["start"]); - }); - -// Test to check that getFlow returns different flowRef values correctly - it("should handle different flowRef values", () => { - const differentFlowRefMock = { current: { id: "different-flow" } }; - (useBotRefsContext as jest.Mock).mockReturnValue({ flowRef: differentFlowRefMock }); - const { result } = renderHook(() => useFlowInternal()); - - const flow = result.current.getFlow(); - expect(flow).toEqual({ id: "different-flow" }); - }); + const flowRefMock = { current: { id: "test-flow" } }; + const hasFlowStartedMock = true; + const mockSettings = { + chatHistory: { + storageType: "localStorage", + storageKey: "rcb-history", + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + + (useMessagesContext as jest.Mock).mockReturnValue({ setMessages: setMessagesMock }); + (usePathsContext as jest.Mock).mockReturnValue({ setPaths: setPathsMock }); + (useToastsContext as jest.Mock).mockReturnValue({ setToasts: setToastsMock }); + (useBotRefsContext as jest.Mock).mockReturnValue({ flowRef: flowRefMock }); + (useBotStatesContext as jest.Mock).mockReturnValue({ hasFlowStarted: hasFlowStartedMock }); + (useSettingsContext as jest.Mock).mockReturnValue({ settings: mockSettings }); + }); + + + // Test to ensure initial values (hasFlowStarted and flowRef) are returned correctly from the hook + it("should return initial values from context", () => { + const { result } = renderHook(() => useFlowInternal()); + + expect(result.current.hasFlowStarted).toBe(true); + expect(result.current.getFlow()).toEqual({ id: "test-flow" }); + }); + + // Test to ensure that restartFlow clears messages, toasts, and resets paths + it("should restart the flow by clearing messages, toasts, resetting paths and loading history", () => { + const { result } = renderHook(() => useFlowInternal()); + + act(() => { + result.current.restartFlow(); + }); + + expect(setMessagesMock).toHaveBeenCalledWith([]); + expect(setToastsMock).toHaveBeenCalledWith([]); + expect(setPathsMock).toHaveBeenCalledWith(["start"]); + expect(setHistoryStorageValues) + .toHaveBeenCalledWith({ chatHistory: { storageType: "localStorage", storageKey: "rcb-history" } }); + }); + + // Test to ensure that getFlow returns the current flow from flowRef + it("should get the current flow from flowRef", () => { + const { result } = renderHook(() => useFlowInternal()); + + const flow = result.current.getFlow(); + expect(flow).toEqual({ id: "test-flow" }); + }); + + // Test to ensure that calling restartFlow multiple times works correctly + it("should handle multiple restarts correctly", () => { + const { result } = renderHook(() => useFlowInternal()); + + act(() => { + result.current.restartFlow(); + }); + + act(() => { + result.current.restartFlow(); + }); + + expect(setMessagesMock).toHaveBeenCalledTimes(2); + expect(setToastsMock).toHaveBeenCalledTimes(2); + expect(setPathsMock).toHaveBeenCalledTimes(2); + }); + + // Test to check flow state when hasFlowStarted is false + it("should correctly reflect flow state when it hasn't started", () => { + (useBotStatesContext as jest.Mock).mockReturnValue({ hasFlowStarted: false }); + const { result } = renderHook(() => useFlowInternal()); + + expect(result.current.hasFlowStarted).toBe(false); + }); + + // Test to ensure messages, toasts, and paths are initialized correctly when restarting the flow + it("should initialize messages, toasts, and paths correctly", () => { + const { result } = renderHook(() => useFlowInternal()); + + act(() => { + result.current.restartFlow(); + }); + + expect(setMessagesMock).toHaveBeenCalledWith([]); + expect(setToastsMock).toHaveBeenCalledWith([]); + expect(setPathsMock).toHaveBeenCalledWith(["start"]); + }); + + // Test to check that getFlow returns different flowRef values correctly + it("should handle different flowRef values", () => { + const differentFlowRefMock = { current: { id: "different-flow" } }; + (useBotRefsContext as jest.Mock).mockReturnValue({ flowRef: differentFlowRefMock }); + const { result } = renderHook(() => useFlowInternal()); + + const flow = result.current.getFlow(); + expect(flow).toEqual({ id: "different-flow" }); + }); }); \ No newline at end of file diff --git a/__tests__/services/ChatHistoryService.test.ts b/__tests__/services/ChatHistoryService.test.ts index c4ced6e..3eaae99 100644 --- a/__tests__/services/ChatHistoryService.test.ts +++ b/__tests__/services/ChatHistoryService.test.ts @@ -61,9 +61,6 @@ describe("ChatHistoryService", () => { setHistoryStorageValues(settings); await saveChatHistory([mockMessage]); expect(storage.setItem).not.toHaveBeenCalled(); - - const historyMessages = getHistoryMessages(); - expect(historyMessages).toEqual([]); }); it("should save history if not disabled", async () => { @@ -71,20 +68,6 @@ describe("ChatHistoryService", () => { await saveChatHistory([mockMessage]); expect(storage.setItem).toHaveBeenCalled(); - - const historyMessages = getHistoryMessages(); - expect(historyMessages).toEqual([mockMessage]); - }); - - it("should retain previous history if not disabled and empty message array is passed", async () => { - setHistoryStorageValues(mockSettings(storageType)); - await saveChatHistory([mockMessage]); - await saveChatHistory([]); - - expect(storage.setItem).toHaveBeenCalledTimes(2); - - const historyMessages = getHistoryMessages(); - expect(historyMessages).toEqual([mockMessage]); }); it("should handle empty message array without errors", async () => { diff --git a/src/hooks/internal/useFlowInternal.ts b/src/hooks/internal/useFlowInternal.ts index bec3236..4c36e6c 100644 --- a/src/hooks/internal/useFlowInternal.ts +++ b/src/hooks/internal/useFlowInternal.ts @@ -1,17 +1,19 @@ -import { useCallback } from "react"; +import {useCallback} from "react"; -import { useMessagesInternal } from "./useMessagesInternal"; -import { usePathsInternal } from "./usePathsInternal"; -import { useToastsInternal } from "./useToastsInternal"; -import { useBotRefsContext } from "../../context/BotRefsContext"; -import { useBotStatesContext } from "../../context/BotStatesContext"; +import {useMessagesInternal} from "./useMessagesInternal"; +import {usePathsInternal} from "./usePathsInternal"; +import {useToastsInternal} from "./useToastsInternal"; +import {useBotRefsContext} from "../../context/BotRefsContext"; +import {useBotStatesContext} from "../../context/BotStatesContext"; +import {setHistoryStorageValues} from "../../services/ChatHistoryService"; +import {useSettingsContext} from "../../context/SettingsContext"; /** * Internal custom hook for managing flow. */ export const useFlowInternal = () => { // handles messages - const { replaceMessages } = useMessagesInternal(); + const { replaceMessages, messages } = useMessagesInternal(); // handles paths const { replacePaths } = usePathsInternal(); @@ -24,6 +26,9 @@ export const useFlowInternal = () => { // handles bot refs const { flowRef } = useBotRefsContext(); + + // handles bot settings + const { settings } = useSettingsContext(); /** * Restarts the conversation flow for the chatbot. @@ -32,7 +37,12 @@ export const useFlowInternal = () => { replaceMessages([]); replaceToasts([]); replacePaths(["start"]); - }, [replaceMessages, replaceToasts, replacePaths]); + + //reload the chat history from storage + setHistoryStorageValues(settings) + console.log("Flow restarted"); + console.log("Messages", messages); + }, [replaceMessages, replaceToasts, replacePaths, setHistoryStorageValues]); /** * Retrieves the conversation flow for the chatbot. diff --git a/src/hooks/internal/useMessagesInternal.ts b/src/hooks/internal/useMessagesInternal.ts index 0a8fd27..67d25ed 100644 --- a/src/hooks/internal/useMessagesInternal.ts +++ b/src/hooks/internal/useMessagesInternal.ts @@ -323,7 +323,7 @@ export const useMessagesInternal = () => { // defer update to next event loop, handles edge case where messages are sent too fast // and the scrolling does not properly reach the bottom setTimeout(() => { - if (!chatBodyRef.current) { + if (!chatBodyRef?.current) { return; } diff --git a/src/services/ChatHistoryService.tsx b/src/services/ChatHistoryService.tsx index 1aa65fc..4edb816 100644 --- a/src/services/ChatHistoryService.tsx +++ b/src/services/ChatHistoryService.tsx @@ -51,7 +51,6 @@ const saveChatHistory = async (messages: Message[]) => { parsedMessages = [...historyMessages.slice(-difference), ...parsedMessages] } - historyMessages = parsedMessages; setHistoryMessages(parsedMessages); } From ba945e38c9971b6af07ab569e6eb330a4b3bfbe2 Mon Sep 17 00:00:00 2001 From: Alex Peelman Date: Mon, 9 Dec 2024 16:54:23 +0100 Subject: [PATCH 3/4] ISSUE 282 remove console.log --- src/hooks/internal/useFlowInternal.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hooks/internal/useFlowInternal.ts b/src/hooks/internal/useFlowInternal.ts index 4c36e6c..d5b82b6 100644 --- a/src/hooks/internal/useFlowInternal.ts +++ b/src/hooks/internal/useFlowInternal.ts @@ -13,7 +13,7 @@ import {useSettingsContext} from "../../context/SettingsContext"; */ export const useFlowInternal = () => { // handles messages - const { replaceMessages, messages } = useMessagesInternal(); + const { replaceMessages } = useMessagesInternal(); // handles paths const { replacePaths } = usePathsInternal(); @@ -40,8 +40,6 @@ export const useFlowInternal = () => { //reload the chat history from storage setHistoryStorageValues(settings) - console.log("Flow restarted"); - console.log("Messages", messages); }, [replaceMessages, replaceToasts, replacePaths, setHistoryStorageValues]); /** From e0bea37b6ae8d94bd9ff9b54c142046f8a101307 Mon Sep 17 00:00:00 2001 From: Alex Peelman Date: Mon, 9 Dec 2024 16:57:10 +0100 Subject: [PATCH 4/4] ISSUE 282 reload history from storage before other actions --- src/hooks/internal/useFlowInternal.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hooks/internal/useFlowInternal.ts b/src/hooks/internal/useFlowInternal.ts index d5b82b6..8822dd6 100644 --- a/src/hooks/internal/useFlowInternal.ts +++ b/src/hooks/internal/useFlowInternal.ts @@ -34,12 +34,12 @@ export const useFlowInternal = () => { * Restarts the conversation flow for the chatbot. */ const restartFlow = useCallback(() => { + //reload the chat history from storage + setHistoryStorageValues(settings) + replaceMessages([]); replaceToasts([]); replacePaths(["start"]); - - //reload the chat history from storage - setHistoryStorageValues(settings) }, [replaceMessages, replaceToasts, replacePaths, setHistoryStorageValues]); /**