From bc2e7a13635e13e23962ddab271336e132ab2654 Mon Sep 17 00:00:00 2001 From: Marcos Rigoli Date: Wed, 13 Nov 2024 11:01:34 -0300 Subject: [PATCH] feat: Fetches chat history when loading Xpert (#64) * feat: Fethes chat history when loading Xpert * chore: Added coverage for new API method * chore: Added test:watch script * fix: Fixed tests and made chat history error silent * chore: Added coverage to useMessageHistory() hook * chore: Added test for Xpert to use the new hook * chore: Updated message history hook test * chore: Updated thunk tests for getLearningAssistantMessageHistory() * chore: Updated caniuse-lite db --- package-lock.json | 6 +- package.json | 3 +- src/data/api.js | 14 +++- src/data/api.test.js | 53 +++++++++++++ src/data/thunks.js | 29 ++++++- src/data/thunks.test.js | 127 ++++++++++++++++++++++++++++++ src/hooks/index.js | 2 + src/hooks/message-history.js | 15 ++++ src/hooks/message-history.test.js | 57 ++++++++++++++ src/widgets/Xpert.jsx | 2 + src/widgets/Xpert.test.jsx | 14 +++- 11 files changed, 312 insertions(+), 10 deletions(-) create mode 100644 src/data/api.test.js create mode 100644 src/data/thunks.test.js create mode 100644 src/hooks/index.js create mode 100644 src/hooks/message-history.js create mode 100644 src/hooks/message-history.test.js diff --git a/package-lock.json b/package-lock.json index ac46507a..e881a552 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7440,9 +7440,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001618", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001618.tgz", - "integrity": "sha512-p407+D1tIkDvsEAPS22lJxLQQaG8OTBEqo0KhzfABGk0TU4juBNDSfH0hyAp/HRyx+M8L17z/ltyhxh27FTfQg==", + "version": "1.0.30001680", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001680.tgz", + "integrity": "sha512-rPQy70G6AGUMnbwS1z6Xg+RkHYPAi18ihs47GH0jcxIG7wArmPgY3XbS2sRdBbxJljp3thdT8BIqv9ccCypiPA==", "dev": true, "funding": [ { diff --git a/package.json b/package.json index 462b1998..f0a2a823 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,8 @@ "snapshot": "fedx-scripts jest --updateSnapshot", "start": "fedx-scripts webpack-dev-server --progress", "test": "fedx-scripts jest --coverage --passWithNoTests", - "test:ci": "fedx-scripts jest --silent --coverage --passWithNoTests" + "test:ci": "fedx-scripts jest --silent --coverage --passWithNoTests", + "test:watch": "fedx-scripts jest --passWithNoTests --watch" }, "files": [ "/dist" diff --git a/src/data/api.js b/src/data/api.js index 7155dac0..2ebfdb68 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -30,5 +30,15 @@ async function fetchLearningAssistantEnabled(courseId) { return data; } -export default fetchChatResponse; -export { fetchLearningAssistantEnabled }; +async function fetchLearningAssistantMessageHistory(courseId) { + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/history`); + + const { data } = await getAuthenticatedHttpClient().get(url.href); + return data; +} + +export { + fetchChatResponse, + fetchLearningAssistantEnabled, + fetchLearningAssistantMessageHistory, +}; diff --git a/src/data/api.test.js b/src/data/api.test.js new file mode 100644 index 00000000..7b747a5a --- /dev/null +++ b/src/data/api.test.js @@ -0,0 +1,53 @@ +/* eslint-disable no-import-assign */ +import * as auth from '@edx/frontend-platform/auth'; + +import { fetchLearningAssistantMessageHistory } from './api'; + +jest.mock('@edx/frontend-platform/auth'); + +const CHAT_RESPONSE_URL = 'https://some.url/endpoint'; +jest.mock('@edx/frontend-platform', () => ({ + getConfig: () => ({ CHAT_RESPONSE_URL }), + CHAT_RESPONSE_URL, +})); + +describe('API', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('fetchLearningAssistantMessageHistory()', () => { + const fakeCourseId = 'course-v1:edx+test+23'; + const apiPayload = [ + { + role: 'user', + content: 'Marco', + timestamp: '2024-11-04T19:05:07.403363Z', + }, + { + role: 'assistant', + content: 'Polo', + timestamp: '2024-11-04T19:05:21.357636Z', + }, + ]; + + const fakeGet = jest.fn(async () => ({ + data: apiPayload, + catch: () => {}, + })); + + beforeEach(() => { + auth.getAuthenticatedHttpClient = jest.fn(() => ({ + get: fakeGet, + })); + }); + + it('should call the endpoint and process the results', async () => { + const response = await fetchLearningAssistantMessageHistory(fakeCourseId); + + expect(response).toEqual(apiPayload); + expect(fakeGet).toHaveBeenCalledTimes(1); + expect(fakeGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${fakeCourseId}/history`); + }); + }); +}); diff --git a/src/data/thunks.js b/src/data/thunks.js index b63d466a..891c0f4d 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -2,7 +2,7 @@ import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import trackChatBotMessageOptimizely from '../utils/optimizelyExperiment'; -import fetchChatResponse, { fetchLearningAssistantEnabled } from './api'; +import { fetchChatResponse, fetchLearningAssistantMessageHistory, fetchLearningAssistantEnabled } from './api'; import { setCurrentMessage, clearCurrentMessage, @@ -81,6 +81,33 @@ export function clearMessages() { }; } +export function getLearningAssistantMessageHistory(courseId) { + return async (dispatch) => { + dispatch(setApiIsLoading(true)); + + try { + const rawMessageList = await fetchLearningAssistantMessageHistory(courseId); + + if (rawMessageList.length) { + const messageList = rawMessageList + .map(({ timestamp, ...msg }) => ({ + ...msg, + timestamp: new Date(timestamp), // Parse ISO time to Date() + })); + + dispatch(setMessageList({ messageList })); + + // If it has chat history, then we assume the user already aknowledged. + dispatch(setDisclosureAcknowledged(true)); + } + } catch (e) { + // If fetching the messages fail, we just won't show it. + } + + dispatch(setApiIsLoading(false)); + }; +} + export function updateCurrentMessage(content) { return (dispatch) => { dispatch(setCurrentMessage({ currentMessage: content })); diff --git a/src/data/thunks.test.js b/src/data/thunks.test.js new file mode 100644 index 00000000..2a5039aa --- /dev/null +++ b/src/data/thunks.test.js @@ -0,0 +1,127 @@ +import { fetchLearningAssistantMessageHistory } from './api'; + +import { getLearningAssistantMessageHistory } from './thunks'; + +jest.mock('./api'); + +describe('Thunks unit tests', () => { + const dispatch = jest.fn(); + + afterEach(() => jest.resetAllMocks()); + + describe('getLearningAssistantMessageHistory()', () => { + const fakeCourseId = 'course-v1:edx+test+23'; + + describe('when returning results', () => { + const apiResponse = [ + { + role: 'user', + content: 'Marco', + timestamp: '2024-11-04T19:05:07.403363Z', + }, + { + role: 'assistant', + content: 'Polo', + timestamp: '2024-11-04T19:05:21.357636Z', + }, + ]; + + beforeEach(() => { + fetchLearningAssistantMessageHistory.mockResolvedValue(apiResponse); + }); + + it('should set the loading state, fetch, parse and set the messages and remove the loading state', async () => { + await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, + }); + + expect(fetchLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setMessageList', + payload: { + messageList: apiResponse.map(({ timestamp, ...msg }) => ({ + ...msg, + timestamp: new Date(timestamp), // Parse ISO time to Date() + })), + }, + }); + + expect(dispatch).toHaveBeenNthCalledWith(3, { + type: 'learning-assistant/setDisclosureAcknowledged', + payload: true, + }); + + expect(dispatch).toHaveBeenNthCalledWith(4, { + type: 'learning-assistant/setApiIsLoading', + payload: false, + }); + }); + }); + + describe('when returning no messages', () => { + const apiResponse = []; + + beforeEach(() => { + fetchLearningAssistantMessageHistory.mockResolvedValue(apiResponse); + }); + + it('should only set and remove the loading state', async () => { + await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, + }); + + expect(fetchLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setMessageList' }), + ); + + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), + ); + + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setApiIsLoading', + payload: false, + }); + }); + }); + + describe('when throwing on fetching', () => { + beforeEach(() => { + fetchLearningAssistantMessageHistory.mockRejectedValue('Whoopsie!'); + }); + + it('should only set and remove the loading state', async () => { + await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, + }); + + expect(fetchLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setMessageList' }), + ); + + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), + ); + + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setApiIsLoading', + payload: false, + }); + }); + }); + }); +}); diff --git a/src/hooks/index.js b/src/hooks/index.js new file mode 100644 index 00000000..bfa4b76e --- /dev/null +++ b/src/hooks/index.js @@ -0,0 +1,2 @@ +/* eslint-disable import/prefer-default-export */ +export { useMessageHistory } from './message-history'; diff --git a/src/hooks/message-history.js b/src/hooks/message-history.js new file mode 100644 index 00000000..c27d5f46 --- /dev/null +++ b/src/hooks/message-history.js @@ -0,0 +1,15 @@ +/* eslint-disable import/prefer-default-export */ +import { useEffect } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { getLearningAssistantMessageHistory } from '../data/thunks'; + +export const useMessageHistory = (courseId) => { + const dispatch = useDispatch(); + const { isEnabled } = useSelector(state => state.learningAssistant); + + useEffect(() => { + if (!courseId || !isEnabled) { return; } + + dispatch(getLearningAssistantMessageHistory(courseId)); + }, [dispatch, isEnabled, courseId]); +}; diff --git a/src/hooks/message-history.test.js b/src/hooks/message-history.test.js new file mode 100644 index 00000000..b3c86b7f --- /dev/null +++ b/src/hooks/message-history.test.js @@ -0,0 +1,57 @@ +import { renderHook } from '@testing-library/react-hooks'; + +import { useSelector } from 'react-redux'; +import { useMessageHistory } from './message-history'; +import { getLearningAssistantMessageHistory } from '../data/thunks'; + +const mockDispatch = jest.fn(); +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useSelector: jest.fn(), + useDispatch: () => mockDispatch, +})); + +const getLearningAssistantMessageHistorySignature = { getLearningAssistantMessageHistory: 'getLearningAssistantMessageHistory' }; +jest.mock('../data/thunks', () => ({ + getLearningAssistantMessageHistory: jest.fn().mockReturnValue(getLearningAssistantMessageHistorySignature), +})); + +describe('Learning Assistant Message History Hooks', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('useMessageHistory()', () => { + let hook; + const fakeCourseId = 'course-v1:edx+test+23'; + + const renderTestHook = (courseId, isEnabled) => { + const mockedStoreState = { learningAssistant: { isEnabled } }; + useSelector.mockImplementation(selector => selector(mockedStoreState)); + hook = renderHook(() => useMessageHistory(courseId)); + return hook; + }; + + it('should dispatch getLearningAssistantMessageHistory() with the chat history', () => { + renderTestHook(fakeCourseId, true); + + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockDispatch).toHaveBeenCalledWith(getLearningAssistantMessageHistorySignature); + expect(getLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + }); + + it('should NOT dispatch getLearningAssistantMessageHistory() when disabled', () => { + renderTestHook(fakeCourseId, false); + + expect(mockDispatch).not.toHaveBeenCalled(); + expect(getLearningAssistantMessageHistory).not.toHaveBeenCalled(); + }); + + it('should NOT dispatch getLearningAssistantMessageHistory() with no courseId', () => { + renderTestHook(null, true); + + expect(mockDispatch).not.toHaveBeenCalled(); + expect(getLearningAssistantMessageHistory).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index ade3b1e9..88ee461d 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -6,9 +6,11 @@ import { updateSidebarIsOpen, getIsEnabled } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; import { ExperimentsProvider } from '../experiments'; +import { useMessageHistory } from '../hooks'; const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const dispatch = useDispatch(); + useMessageHistory(courseId); const { isEnabled, diff --git a/src/widgets/Xpert.test.jsx b/src/widgets/Xpert.test.jsx index f1ecede5..ec6bef04 100644 --- a/src/widgets/Xpert.test.jsx +++ b/src/widgets/Xpert.test.jsx @@ -9,6 +9,7 @@ import Xpert from './Xpert'; import * as surveyMonkey from '../utils/surveyMonkey'; import { render, createRandomResponseForTesting } from '../utils/utils.test'; import { usePromptExperimentDecision } from '../experiments'; +import { useMessageHistory } from '../hooks'; jest.mock('@edx/frontend-platform/analytics'); jest.mock('@edx/frontend-platform/auth', () => ({ @@ -20,6 +21,8 @@ jest.mock('../experiments', () => ({ usePromptExperimentDecision: jest.fn(), })); +jest.mock('../hooks'); + const initialState = { learningAssistant: { currentMessage: '', @@ -46,7 +49,7 @@ const assertSidebarElementsNotInDOM = () => { beforeEach(() => { const responseMessage = createRandomResponseForTesting(); - jest.spyOn(api, 'default').mockResolvedValue(responseMessage); + jest.spyOn(api, 'fetchChatResponse').mockResolvedValue(responseMessage); jest.spyOn(api, 'fetchLearningAssistantEnabled').mockResolvedValue({ enabled: true }); usePromptExperimentDecision.mockReturnValue([]); @@ -76,6 +79,11 @@ test('initial load displays correct elements', async () => { // assert that UI elements in the sidebar are not in the DOM assertSidebarElementsNotInDOM(); }); +test('calls useMessageHistory() hook', () => { + render(, { preloadedState: initialState }); + + expect(useMessageHistory).toHaveBeenCalledWith(courseId); +}); test('clicking the call to action dismiss button removes the message', async () => { const user = userEvent.setup(); render(, { preloadedState: initialState }); @@ -165,7 +173,7 @@ test('response text appears as message in the sidebar', async () => { // re-mock the fetchChatResponse API function so that we can assert that the // responseMessage appears in the DOM const responseMessage = createRandomResponseForTesting(); - jest.spyOn(api, 'default').mockResolvedValue(responseMessage); + jest.spyOn(api, 'fetchChatResponse').mockResolvedValue(responseMessage); render(, { preloadedState: initialState }); @@ -189,7 +197,7 @@ test('clicking the clear button clears messages in the sidebar', async () => { // re-mock the fetchChatResponse API function so that we can assert that the // responseMessage appears in the DOM and then is successfully cleared const responseMessage = createRandomResponseForTesting(); - jest.spyOn(api, 'default').mockImplementation(() => responseMessage); + jest.spyOn(api, 'fetchChatResponse').mockImplementation(() => responseMessage); render(, { preloadedState: initialState });