Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Fetches chat history when loading Xpert #64

Merged
merged 9 commits into from
Nov 13, 2024
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely helpful to have when creating unit tests.

},
"files": [
"/dist"
Expand Down
14 changes: 12 additions & 2 deletions src/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,15 @@ async function fetchLearningAssistantEnabled(courseId) {
return data;
}

export default fetchChatResponse;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for mixing default and named exports on these type of modules.

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,
};
53 changes: 53 additions & 0 deletions src/data/api.test.js
Original file line number Diff line number Diff line change
@@ -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 () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we think of other test cases here? Maybe error conditions?

Copy link
Member Author

@rijuma rijuma Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well.. the fail conditions are handled on the thunk usage, so it is contemplated. On the api module side any error would just throw.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was having in mind that any error, whether an endpoint failure or a parsing error (payload structure) would result on not loading the chat history, like a fallback for the current default behavior.

Let me know if you want us to contemplate some custom error messaging for the user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const response = await fetchLearningAssistantMessageHistory(fakeCourseId);

expect(response).toEqual(apiPayload);
expect(fakeGet).toHaveBeenCalledTimes(1);
expect(fakeGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${fakeCourseId}/history`);
});
});
});
29 changes: 28 additions & 1 deletion src/data/thunks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
rijuma marked this conversation as resolved.
Show resolved Hide resolved
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 }));
Expand Down
127 changes: 127 additions & 0 deletions src/data/thunks.test.js
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
});
});
2 changes: 2 additions & 0 deletions src/hooks/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/* eslint-disable import/prefer-default-export */
export { useMessageHistory } from './message-history';
15 changes: 15 additions & 0 deletions src/hooks/message-history.js
Original file line number Diff line number Diff line change
@@ -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]);
};
57 changes: 57 additions & 0 deletions src/hooks/message-history.test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
2 changes: 2 additions & 0 deletions src/widgets/Xpert.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading