From c702ca10c5b1d6fee999c9597f53c700b6103fd6 Mon Sep 17 00:00:00 2001 From: ilee2u Date: Mon, 25 Nov 2024 13:54:32 -0500 Subject: [PATCH 1/9] feat: Call LearningAssistantAuditTrial endpoint --- src/data/api.js | 23 +++++++++++++++++++++++ src/data/slice.js | 8 ++++++++ src/data/thunks.js | 14 +++++++++++++- src/widgets/Xpert.jsx | 12 +++++++++++- 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/data/api.js b/src/data/api.js index 2ebfdb68..4b1411cb 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -30,6 +30,28 @@ async function fetchLearningAssistantEnabled(courseId) { return data; } + +async function fetchLearningAssistantAuditTrial(userId) { + // TODO: Insert correct URL once get endpoint is implemented. + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${userId}/`); + + const { data } = await getAuthenticatedHttpClient().get(url.href); + return data; +} + + +async function fetchLearningAssistantAuditTrial(userId) { + // TODO: Insert correct URL once get endpoint is implemented. + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${userId}/`); + + const { data } = await getAuthenticatedHttpClient().get(url.href); + // TODO: Extract below values from data: + // Whether or not a trial exists + // Days remaining in trial + // When trial was created + return data; +} + async function fetchLearningAssistantMessageHistory(courseId) { const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/history`); @@ -39,6 +61,7 @@ async function fetchLearningAssistantMessageHistory(courseId) { export { fetchChatResponse, + fetchLearningAssistantAuditTrial, fetchLearningAssistantEnabled, fetchLearningAssistantMessageHistory, }; diff --git a/src/data/slice.js b/src/data/slice.js index ffc981af..9b3bb165 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -11,6 +11,11 @@ export const initialState = { disclosureAcknowledged: false, sidebarIsOpen: false, isEnabled: false, + auditTrial: { + trialExists: false, + daysRemaining: 0, + trialCreated: null, // TODO: what do we use for a datetime value here? + }, }; export const learningAssistantSlice = createSlice({ @@ -44,6 +49,9 @@ export const learningAssistantSlice = createSlice({ setIsEnabled: (state, { payload }) => { state.isEnabled = payload; }, + setAuditTrial: (state, { payload }) => { + state.auditTrial = payload; + }, }, }); diff --git a/src/data/thunks.js b/src/data/thunks.js index 13c64e51..ac353e1e 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, fetchLearningAssistantMessageHistory, fetchLearningAssistantEnabled } from './api'; +import { fetchChatResponse, fetchLearningAssistantMessageHistory, fetchLearningAssistantEnabled, fetchLearningAssistantAuditTrial } from './api'; import { setCurrentMessage, clearCurrentMessage, @@ -13,6 +13,7 @@ import { setDisclosureAcknowledged, setSidebarIsOpen, setIsEnabled, + setAuditTrial, } from './slice'; import { OPTIMIZELY_PROMPT_EXPERIMENT_KEY } from './optimizely'; @@ -134,3 +135,14 @@ export function getIsEnabled(courseId) { } }; } + +export function getIsAuditTrial(userId) { + return async (dispatch) => { + try { + const data = await fetchLearningAssistantAuditTrial(userId); + dispatch(setAuditTrial(data.enabled)); + } catch (error) { + dispatch(setApiError()); + } + }; +} diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index 88ee461d..155f13de 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -2,7 +2,7 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { updateSidebarIsOpen, getIsEnabled } from '../data/thunks'; +import { updateSidebarIsOpen, getIsEnabled, getIsAuditTrial } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; import { ExperimentsProvider } from '../experiments'; @@ -15,6 +15,12 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const { isEnabled, sidebarIsOpen, + // TODO: How do we plan to use this value to gate things? + // i.e. how to use values such as: + // auditTrial.trialExists + // auditTrial.daysRemaining + // auditTrial.trialCreated + auditTrial, } = useSelector(state => state.learningAssistant); const setSidebarIsOpen = (isOpen) => { @@ -25,6 +31,10 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { dispatch(getIsEnabled(courseId)); }, [dispatch, courseId]); + useEffect(() => { + dispatch(getIsAuditTrial(userId)); + }, [dispatch, userId]); + return isEnabled ? ( <> From 0e65fc33ae2eb226340d1e41c2cac305dd29e0a9 Mon Sep 17 00:00:00 2001 From: ilee2u Date: Tue, 26 Nov 2024 16:08:36 -0500 Subject: [PATCH 2/9] fix: correct state + add gating for message form --- src/components/Sidebar/index.jsx | 8 ++++++-- src/data/api.js | 20 +++----------------- src/data/slice.js | 8 ++++---- src/data/thunks.js | 6 ++++-- src/widgets/Xpert.jsx | 18 +++++++++++------- 5 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/components/Sidebar/index.jsx b/src/components/Sidebar/index.jsx index 097397a3..718b60d8 100644 --- a/src/components/Sidebar/index.jsx +++ b/src/components/Sidebar/index.jsx @@ -20,7 +20,8 @@ const Sidebar = ({ isOpen, setIsOpen, unitId, -}) => { + auditTrialNotExpired + &, > { const { apiError, disclosureAcknowledged, @@ -98,7 +99,10 @@ const Sidebar = ({ ) } - {getMessageForm()} + { + auditTrialNotExpired + && getMessageForm() + } ); diff --git a/src/data/api.js b/src/data/api.js index 4b1411cb..8a9033b8 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -30,30 +30,16 @@ async function fetchLearningAssistantEnabled(courseId) { return data; } - -async function fetchLearningAssistantAuditTrial(userId) { - // TODO: Insert correct URL once get endpoint is implemented. - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${userId}/`); +async function fetchLearningAssistantMessageHistory(courseId) { + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/history`); const { data } = await getAuthenticatedHttpClient().get(url.href); return data; } - async function fetchLearningAssistantAuditTrial(userId) { // TODO: Insert correct URL once get endpoint is implemented. - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${userId}/`); - - const { data } = await getAuthenticatedHttpClient().get(url.href); - // TODO: Extract below values from data: - // Whether or not a trial exists - // Days remaining in trial - // When trial was created - return data; -} - -async function fetchLearningAssistantMessageHistory(courseId) { - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/history`); + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/data/${userId}/`); const { data } = await getAuthenticatedHttpClient().get(url.href); return data; diff --git a/src/data/slice.js b/src/data/slice.js index 9b3bb165..5d7df191 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -12,9 +12,8 @@ export const initialState = { sidebarIsOpen: false, isEnabled: false, auditTrial: { - trialExists: false, - daysRemaining: 0, - trialCreated: null, // TODO: what do we use for a datetime value here? + startDate: 0, + expirationDate: null, // TODO: what do we use for a datetime value here? }, }; @@ -50,7 +49,8 @@ export const learningAssistantSlice = createSlice({ state.isEnabled = payload; }, setAuditTrial: (state, { payload }) => { - state.auditTrial = payload; + state.auditTrial.startDate = payload.start_date; + state.auditTrial.expirationDate = payload.expiration_date; }, }, }); diff --git a/src/data/thunks.js b/src/data/thunks.js index ac353e1e..6f389607 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -136,11 +136,13 @@ export function getIsEnabled(courseId) { }; } -export function getIsAuditTrial(userId) { +export function getAuditTrial(userId) { return async (dispatch) => { try { const data = await fetchLearningAssistantAuditTrial(userId); - dispatch(setAuditTrial(data.enabled)); + if (!_.isEmpty(data)) { // If returned data is not empty + dispatch(setAuditTrial(data)); + } } catch (error) { dispatch(setApiError()); } diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index 155f13de..f3fb84b7 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -2,7 +2,7 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { updateSidebarIsOpen, getIsEnabled, getIsAuditTrial } from '../data/thunks'; +import { updateSidebarIsOpen, getIsEnabled, getAuditTrial } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; import { ExperimentsProvider } from '../experiments'; @@ -15,11 +15,6 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const { isEnabled, sidebarIsOpen, - // TODO: How do we plan to use this value to gate things? - // i.e. how to use values such as: - // auditTrial.trialExists - // auditTrial.daysRemaining - // auditTrial.trialCreated auditTrial, } = useSelector(state => state.learningAssistant); @@ -32,9 +27,17 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { }, [dispatch, courseId]); useEffect(() => { - dispatch(getIsAuditTrial(userId)); + dispatch(getAuditTrial(userId)); }, [dispatch, userId]); + const isAuditTrialNotExpired = () => { + const auditTrialExpired = (Date.now() - auditTrial.expirationDate) > 0; + if (auditTrialExpired) { + return true + } + return false + } + return isEnabled ? ( <> @@ -49,6 +52,7 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { isOpen={sidebarIsOpen} setIsOpen={setSidebarIsOpen} unitId={unitId} + auditTrialNotExpired={isAuditTrialNotExpired} /> From f238e09203969db607056bfce02ddcf6500467b3 Mon Sep 17 00:00:00 2001 From: ilee2u Date: Mon, 2 Dec 2024 13:30:25 -0500 Subject: [PATCH 3/9] temp: fixed slice, attempting to implment tests --- src/components/Sidebar/index.jsx | 8 ++------ src/data/api.js | 1 + src/data/slice.js | 5 +++-- src/data/thunks.js | 10 ++++++++-- src/hooks/message-history.test.js | 2 +- src/widgets/Xpert.jsx | 12 +++++++----- src/widgets/Xpert.test.jsx | 14 ++++++++++++++ 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/components/Sidebar/index.jsx b/src/components/Sidebar/index.jsx index 718b60d8..097397a3 100644 --- a/src/components/Sidebar/index.jsx +++ b/src/components/Sidebar/index.jsx @@ -20,8 +20,7 @@ const Sidebar = ({ isOpen, setIsOpen, unitId, - auditTrialNotExpired - &, > { +}) => { const { apiError, disclosureAcknowledged, @@ -99,10 +98,7 @@ const Sidebar = ({ ) } - { - auditTrialNotExpired - && getMessageForm() - } + {getMessageForm()} ); diff --git a/src/data/api.js b/src/data/api.js index 8a9033b8..4c0adab6 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -39,6 +39,7 @@ async function fetchLearningAssistantMessageHistory(courseId) { async function fetchLearningAssistantAuditTrial(userId) { // TODO: Insert correct URL once get endpoint is implemented. + console.log("\n\n\n\nUSER ID:", userId); const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/data/${userId}/`); const { data } = await getAuthenticatedHttpClient().get(url.href); diff --git a/src/data/slice.js b/src/data/slice.js index 5d7df191..e4b54e61 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -12,8 +12,8 @@ export const initialState = { sidebarIsOpen: false, isEnabled: false, auditTrial: { - startDate: 0, - expirationDate: null, // TODO: what do we use for a datetime value here? + startDate: null, + expirationDate: null, }, }; @@ -65,6 +65,7 @@ export const { setDisclosureAcknowledged, setSidebarIsOpen, setIsEnabled, + setAuditTrial, } = learningAssistantSlice.actions; export const { diff --git a/src/data/thunks.js b/src/data/thunks.js index 6f389607..21e09211 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -2,7 +2,12 @@ import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import trackChatBotMessageOptimizely from '../utils/optimizelyExperiment'; -import { fetchChatResponse, fetchLearningAssistantMessageHistory, fetchLearningAssistantEnabled, fetchLearningAssistantAuditTrial } from './api'; +import { + fetchChatResponse, + fetchLearningAssistantMessageHistory, + fetchLearningAssistantEnabled, + fetchLearningAssistantAuditTrial, +} from './api'; import { setCurrentMessage, clearCurrentMessage, @@ -140,7 +145,8 @@ export function getAuditTrial(userId) { return async (dispatch) => { try { const data = await fetchLearningAssistantAuditTrial(userId); - if (!_.isEmpty(data)) { // If returned data is not empty + // If returned data is not empty + if (!_.isEmpty(data)) { // eslint-disable-line no-undef dispatch(setAuditTrial(data)); } } catch (error) { diff --git a/src/hooks/message-history.test.js b/src/hooks/message-history.test.js index b3c86b7f..fe1352b1 100644 --- a/src/hooks/message-history.test.js +++ b/src/hooks/message-history.test.js @@ -1,4 +1,4 @@ -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react-hooks'; // eslint-disable-line import/no-unresolved import { useSelector } from 'react-redux'; import { useMessageHistory } from './message-history'; diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index f3fb84b7..a67de058 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -1,6 +1,7 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import { updateSidebarIsOpen, getIsEnabled, getAuditTrial } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; @@ -9,6 +10,7 @@ import { ExperimentsProvider } from '../experiments'; import { useMessageHistory } from '../hooks'; const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { + const { userId } = getAuthenticatedUser(); const dispatch = useDispatch(); useMessageHistory(courseId); @@ -30,13 +32,14 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { dispatch(getAuditTrial(userId)); }, [dispatch, userId]); - const isAuditTrialNotExpired = () => { + // NOTE: This value can be used later on if/when we pass the enrollment mode to this componentn + const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars const auditTrialExpired = (Date.now() - auditTrial.expirationDate) > 0; if (auditTrialExpired) { - return true + return true; } - return false - } + return false; + }; return isEnabled ? ( @@ -52,7 +55,6 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { isOpen={sidebarIsOpen} setIsOpen={setSidebarIsOpen} unitId={unitId} - auditTrialNotExpired={isAuditTrialNotExpired} /> diff --git a/src/widgets/Xpert.test.jsx b/src/widgets/Xpert.test.jsx index 532669f3..8b4fe7a3 100644 --- a/src/widgets/Xpert.test.jsx +++ b/src/widgets/Xpert.test.jsx @@ -10,6 +10,7 @@ import * as surveyMonkey from '../utils/surveyMonkey'; import { render, createRandomResponseForTesting } from '../utils/utils.test'; import { usePromptExperimentDecision } from '../experiments'; import { useMessageHistory } from '../hooks'; +// import { updateSidebarIsOpen, getIsEnabled, getAuditTrial } from '../data/thunks'; jest.mock('@edx/frontend-platform/analytics'); jest.mock('@edx/frontend-platform/auth', () => ({ @@ -23,6 +24,12 @@ jest.mock('../experiments', () => ({ jest.mock('../hooks'); +// TODO: Fix whatever the heck is going on here. +// jest.mock('../data/thunks', () => ({ +// getIsEnabled: jest.fn(), +// getAuditTrial: jest.fn(), +// })); + const initialState = { learningAssistant: { currentMessage: '', @@ -33,6 +40,11 @@ const initialState = { // I will remove this and write tests in a future pull request. disclosureAcknowledged: true, sidebarIsOpen: false, + + auditTrial: { + startDate: Date.now(), + expirationDate: Date(8.64e15), + }, }, }; const courseId = 'course-v1:edX+DemoX+Demo_Course'; @@ -115,6 +127,8 @@ test('clicking the call to action opens the sidebar', async () => { expect(screen.getByRole('button', { name: 'submit' })).toBeVisible(); expect(screen.getByTestId('close-button')).toBeVisible(); + // API error caused by... getAuditTrial in thunks. + // I need to mock what's returned by fetchLearningAssistantAuditTrial... // assert that text input has focus expect(screen.getByRole('textbox')).toHaveFocus(); }); From 75dfdb7123202905035e297b52307319fcba3b66 Mon Sep 17 00:00:00 2001 From: ilee2u Date: Mon, 2 Dec 2024 17:09:18 -0500 Subject: [PATCH 4/9] temp: debug stdout for testing --- src/data/api.js | 1 - src/data/slice.js | 5 +++-- src/data/thunks.js | 8 +++++++- src/widgets/Xpert.jsx | 2 ++ src/widgets/Xpert.test.jsx | 12 +++++------- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/data/api.js b/src/data/api.js index 4c0adab6..8a9033b8 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -39,7 +39,6 @@ async function fetchLearningAssistantMessageHistory(courseId) { async function fetchLearningAssistantAuditTrial(userId) { // TODO: Insert correct URL once get endpoint is implemented. - console.log("\n\n\n\nUSER ID:", userId); const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/data/${userId}/`); const { data } = await getAuthenticatedHttpClient().get(url.href); diff --git a/src/data/slice.js b/src/data/slice.js index e4b54e61..c479822f 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -49,8 +49,9 @@ export const learningAssistantSlice = createSlice({ state.isEnabled = payload; }, setAuditTrial: (state, { payload }) => { - state.auditTrial.startDate = payload.start_date; - state.auditTrial.expirationDate = payload.expiration_date; + state.auditTrial = payload; + // state.auditTrial.startDate = payload.start_date; + // state.auditTrial.expirationDate = payload.expiration_date; }, }, }); diff --git a/src/data/thunks.js b/src/data/thunks.js index 21e09211..eebae47e 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -145,8 +145,14 @@ export function getAuditTrial(userId) { return async (dispatch) => { try { const data = await fetchLearningAssistantAuditTrial(userId); + console.log("DATA:", data) + console.log("data.start_date:", data.start_date) + console.log("data.expiration_date:", data.expiration_date) // If returned data is not empty - if (!_.isEmpty(data)) { // eslint-disable-line no-undef + if (Object.keys(data).length !== 0) { // eslint-disable-line no-undef + console.log("SETTING!") + // TODO: Why isn't the dispatch below setting the state? + debugger; dispatch(setAuditTrial(data)); } } catch (error) { diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index a67de058..6dbbaa8d 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -31,6 +31,8 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { useEffect(() => { dispatch(getAuditTrial(userId)); }, [dispatch, userId]); + console.log("auditTrial:", auditTrial); + console.log("auditTrial.expirationDate:", auditTrial.expirationDate); // NOTE: This value can be used later on if/when we pass the enrollment mode to this componentn const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars diff --git a/src/widgets/Xpert.test.jsx b/src/widgets/Xpert.test.jsx index 8b4fe7a3..ca30270c 100644 --- a/src/widgets/Xpert.test.jsx +++ b/src/widgets/Xpert.test.jsx @@ -41,10 +41,7 @@ const initialState = { disclosureAcknowledged: true, sidebarIsOpen: false, - auditTrial: { - startDate: Date.now(), - expirationDate: Date(8.64e15), - }, + }, }; const courseId = 'course-v1:edX+DemoX+Demo_Course'; @@ -62,6 +59,10 @@ beforeEach(() => { const responseMessage = createRandomResponseForTesting(); jest.spyOn(api, 'fetchChatResponse').mockResolvedValue(responseMessage); jest.spyOn(api, 'fetchLearningAssistantEnabled').mockResolvedValue({ enabled: true }); + jest.spyOn(api, 'fetchLearningAssistantAuditTrial').mockResolvedValue({ + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }); usePromptExperimentDecision.mockReturnValue([]); window.localStorage.clear(); @@ -127,9 +128,6 @@ test('clicking the call to action opens the sidebar', async () => { expect(screen.getByRole('button', { name: 'submit' })).toBeVisible(); expect(screen.getByTestId('close-button')).toBeVisible(); - // API error caused by... getAuditTrial in thunks. - // I need to mock what's returned by fetchLearningAssistantAuditTrial... - // assert that text input has focus expect(screen.getByRole('textbox')).toHaveFocus(); }); test('clicking the toggle button opens the sidebar', async () => { From 4cdc1869360b67378523dc792f005a9c76529ebc Mon Sep 17 00:00:00 2001 From: ilee2u Date: Tue, 3 Dec 2024 13:41:58 -0500 Subject: [PATCH 5/9] temp: rollback point before summary endpt refactor --- src/data/api.js | 4 ++-- src/data/thunks.js | 7 +++---- src/widgets/Xpert.jsx | 6 +----- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/data/api.js b/src/data/api.js index 8a9033b8..2ff6a906 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -37,9 +37,9 @@ async function fetchLearningAssistantMessageHistory(courseId) { return data; } -async function fetchLearningAssistantAuditTrial(userId) { +async function fetchLearningAssistantAuditTrial(courseId) { // TODO: Insert correct URL once get endpoint is implemented. - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/data/${userId}/`); + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/summary`); const { data } = await getAuthenticatedHttpClient().get(url.href); return data; diff --git a/src/data/thunks.js b/src/data/thunks.js index eebae47e..996c9f44 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -141,18 +141,17 @@ export function getIsEnabled(courseId) { }; } -export function getAuditTrial(userId) { +export function getAuditTrial(courseId) { return async (dispatch) => { try { - const data = await fetchLearningAssistantAuditTrial(userId); + const data = await fetchLearningAssistantAuditTrial(courseId); console.log("DATA:", data) console.log("data.start_date:", data.start_date) console.log("data.expiration_date:", data.expiration_date) // If returned data is not empty if (Object.keys(data).length !== 0) { // eslint-disable-line no-undef console.log("SETTING!") - // TODO: Why isn't the dispatch below setting the state? - debugger; + // TODO: FIGURE OUT how to sync the data from Michael's PR into this MFEs state... dispatch(setAuditTrial(data)); } } catch (error) { diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index 6dbbaa8d..cdb99fa2 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -10,7 +10,6 @@ import { ExperimentsProvider } from '../experiments'; import { useMessageHistory } from '../hooks'; const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { - const { userId } = getAuthenticatedUser(); const dispatch = useDispatch(); useMessageHistory(courseId); @@ -26,11 +25,8 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { useEffect(() => { dispatch(getIsEnabled(courseId)); + dispatch(getAuditTrial(courseId)); }, [dispatch, courseId]); - - useEffect(() => { - dispatch(getAuditTrial(userId)); - }, [dispatch, userId]); console.log("auditTrial:", auditTrial); console.log("auditTrial.expirationDate:", auditTrial.expirationDate); From 5c3b5cf1e61ca0e778abdf72df47e5d8703beb1a Mon Sep 17 00:00:00 2001 From: ilee2u Date: Tue, 3 Dec 2024 15:22:28 -0500 Subject: [PATCH 6/9] feat: refactored to fit with summary endpt - also refactored tests accordingly --- src/data/api.js | 21 +-- src/data/api.test.js | 47 +++--- src/data/slice.js | 2 - src/data/thunks.js | 83 ++++------ src/data/thunks.test.js | 251 ++++++++++++++++++++---------- src/hooks/index.js | 2 - src/hooks/message-history.js | 15 -- src/hooks/message-history.test.js | 57 ------- src/utils/utils.test.jsx | 4 +- src/widgets/Xpert.jsx | 12 +- src/widgets/Xpert.test.jsx | 63 +++----- 11 files changed, 261 insertions(+), 296 deletions(-) delete mode 100644 src/hooks/index.js delete mode 100644 src/hooks/message-history.js delete mode 100644 src/hooks/message-history.test.js diff --git a/src/data/api.js b/src/data/api.js index 2ff6a906..4450e269 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -23,22 +23,7 @@ async function fetchChatResponse(courseId, messageList, unitId, customQueryParam return data; } -async function fetchLearningAssistantEnabled(courseId) { - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/enabled`); - - const { data } = await getAuthenticatedHttpClient().get(url.href); - return data; -} - -async function fetchLearningAssistantMessageHistory(courseId) { - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/history`); - - const { data } = await getAuthenticatedHttpClient().get(url.href); - return data; -} - -async function fetchLearningAssistantAuditTrial(courseId) { - // TODO: Insert correct URL once get endpoint is implemented. +async function fetchLearningAssistantSummary(courseId) { const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/summary`); const { data } = await getAuthenticatedHttpClient().get(url.href); @@ -47,7 +32,5 @@ async function fetchLearningAssistantAuditTrial(courseId) { export { fetchChatResponse, - fetchLearningAssistantAuditTrial, - fetchLearningAssistantEnabled, - fetchLearningAssistantMessageHistory, + fetchLearningAssistantSummary, }; diff --git a/src/data/api.test.js b/src/data/api.test.js index 7b747a5a..d558122e 100644 --- a/src/data/api.test.js +++ b/src/data/api.test.js @@ -1,7 +1,7 @@ /* eslint-disable no-import-assign */ import * as auth from '@edx/frontend-platform/auth'; -import { fetchLearningAssistantMessageHistory } from './api'; +import { fetchLearningAssistantSummary } from './api'; jest.mock('@edx/frontend-platform/auth'); @@ -16,38 +16,45 @@ describe('API', () => { jest.restoreAllMocks(); }); - describe('fetchLearningAssistantMessageHistory()', () => { - const fakeCourseId = 'course-v1:edx+test+23'; - const apiPayload = [ - { - role: 'user', - content: 'Marco', - timestamp: '2024-11-04T19:05:07.403363Z', + describe('fetchLearningAssistantSummary()', () => { + const courseId = 'course-v1:edx+test+23'; + const apiPayload = { + enabled: true, + message_history: [ + { + role: 'user', + content: 'Marco', + timestamp: '2024-11-04T19:05:07.403363Z', + }, + { + role: 'assistant', + content: 'Polo', + timestamp: '2024-11-04T19:05:21.357636Z', + }, + ], + audit_trial: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', }, - { - role: 'assistant', - content: 'Polo', - timestamp: '2024-11-04T19:05:21.357636Z', - }, - ]; + }; - const fakeGet = jest.fn(async () => ({ + const mockGet = jest.fn(async () => ({ data: apiPayload, - catch: () => {}, + catch: () => { }, })); beforeEach(() => { auth.getAuthenticatedHttpClient = jest.fn(() => ({ - get: fakeGet, + get: mockGet, })); }); it('should call the endpoint and process the results', async () => { - const response = await fetchLearningAssistantMessageHistory(fakeCourseId); + const response = await fetchLearningAssistantSummary(courseId); expect(response).toEqual(apiPayload); - expect(fakeGet).toHaveBeenCalledTimes(1); - expect(fakeGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${fakeCourseId}/history`); + expect(mockGet).toHaveBeenCalledTimes(1); + expect(mockGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${courseId}/summary`); }); }); }); diff --git a/src/data/slice.js b/src/data/slice.js index c479822f..39e06ec7 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -50,8 +50,6 @@ export const learningAssistantSlice = createSlice({ }, setAuditTrial: (state, { payload }) => { state.auditTrial = payload; - // state.auditTrial.startDate = payload.start_date; - // state.auditTrial.expirationDate = payload.expiration_date; }, }, }); diff --git a/src/data/thunks.js b/src/data/thunks.js index 996c9f44..501e588b 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -4,9 +4,7 @@ import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import trackChatBotMessageOptimizely from '../utils/optimizelyExperiment'; import { fetchChatResponse, - fetchLearningAssistantMessageHistory, - fetchLearningAssistantEnabled, - fetchLearningAssistantAuditTrial, + fetchLearningAssistantSummary, } from './api'; import { setCurrentMessage, @@ -79,33 +77,6 @@ export function getChatResponse(courseId, unitId, promptExperimentVariationKey = }; } -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 })); @@ -130,32 +101,46 @@ export function updateSidebarIsOpen(isOpen) { }; } -export function getIsEnabled(courseId) { +export function getLearningAssistantSummary(courseId) { return async (dispatch) => { + dispatch(setApiIsLoading(true)); + try { - const data = await fetchLearningAssistantEnabled(courseId); + const data = await fetchLearningAssistantSummary(courseId); + + // Enabled dispatch(setIsEnabled(data.enabled)); - } catch (error) { - dispatch(setApiError()); - } - }; -} -export function getAuditTrial(courseId) { - return async (dispatch) => { - try { - const data = await fetchLearningAssistantAuditTrial(courseId); - console.log("DATA:", data) - console.log("data.start_date:", data.start_date) - console.log("data.expiration_date:", data.expiration_date) - // If returned data is not empty - if (Object.keys(data).length !== 0) { // eslint-disable-line no-undef - console.log("SETTING!") - // TODO: FIGURE OUT how to sync the data from Michael's PR into this MFEs state... - dispatch(setAuditTrial(data)); + // Message History + const rawMessageList = data.message_history; + + // If returned message history data is not empty + if (rawMessageList.length) { + const messageList = rawMessageList + .map(({ timestamp, ...msg }) => ({ + ...msg, + // NOTE to self: can't store Date() objects in store: https://github.com/reduxjs/redux-toolkit/issues/456 + timestamp: new Date(timestamp).toString(), // Parse ISO time to Date() + })); + + dispatch(setMessageList({ messageList })); + + // If it has chat history, then we assume the user already aknowledged. + dispatch(setDisclosureAcknowledged(true)); + } + + // Audit Trial + const auditTrial = data.audit_trial; + + // If returned audit trial data is not empty + if (Object.keys(auditTrial).length !== 0) { // eslint-disable-line no-undef + dispatch(setAuditTrial(auditTrial)); } } catch (error) { + // NOTE: When used to not show if fetching the messages failed + // But we do know since this endpoint is combined. dispatch(setApiError()); } + dispatch(setApiIsLoading(false)); }; } diff --git a/src/data/thunks.test.js b/src/data/thunks.test.js index 2a5039aa..75d1d576 100644 --- a/src/data/thunks.test.js +++ b/src/data/thunks.test.js @@ -1,6 +1,6 @@ -import { fetchLearningAssistantMessageHistory } from './api'; +import { fetchLearningAssistantSummary } from './api'; -import { getLearningAssistantMessageHistory } from './thunks'; +import { getLearningAssistantSummary } from './thunks'; jest.mock('./api'); @@ -9,118 +9,201 @@ describe('Thunks unit tests', () => { 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', + describe('getLearningAssistantSummary()', () => { + const courseId = 'course-v1:edx+test+23'; + + it('with message_history and audit_trial data returned, call all expected dispatches', async () => { + const apiResponse = { + enabled: true, + message_history: [ + { + role: 'user', + content: 'Marco', + timestamp: '2024-11-04T19:05:07.403363Z', + }, + { + role: 'assistant', + content: 'Polo', + timestamp: '2024-11-04T19:05:21.357636Z', + }, + ], + audit_trial: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', }, - ]; + }; + + fetchLearningAssistantSummary.mockResolvedValue(apiResponse); - beforeEach(() => { - fetchLearningAssistantMessageHistory.mockResolvedValue(apiResponse); + await getLearningAssistantSummary(courseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, }); - it('should set the loading state, fetch, parse and set the messages and remove the loading state', async () => { - await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); - expect(dispatch).toHaveBeenNthCalledWith(1, { - type: 'learning-assistant/setApiIsLoading', - payload: true, - }); + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setIsEnabled', + payload: true, + }); - expect(fetchLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + expect(dispatch).toHaveBeenNthCalledWith(3, { + type: 'learning-assistant/setMessageList', + payload: { + messageList: apiResponse.message_history.map(({ timestamp, ...msg }) => ({ + ...msg, + timestamp: new Date(timestamp).toString(), // Parse ISO time to Date() + })), + }, + }); - 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(4, { + type: 'learning-assistant/setDisclosureAcknowledged', + payload: true, + }); - expect(dispatch).toHaveBeenNthCalledWith(3, { - type: 'learning-assistant/setDisclosureAcknowledged', - payload: true, - }); + expect(dispatch).toHaveBeenNthCalledWith(5, { + type: 'learning-assistant/setAuditTrial', + payload: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }, + }); - expect(dispatch).toHaveBeenNthCalledWith(4, { - type: 'learning-assistant/setApiIsLoading', - payload: false, - }); + expect(dispatch).toHaveBeenNthCalledWith(6, { + type: 'learning-assistant/setApiIsLoading', + payload: false, }); }); - describe('when returning no messages', () => { - const apiResponse = []; + it('with no message_history data returned, do not call message history related dispatches', async () => { + const apiResponse = { + enabled: true, + message_history: [], + audit_trial: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }, + }; + + fetchLearningAssistantSummary.mockResolvedValue(apiResponse); - beforeEach(() => { - fetchLearningAssistantMessageHistory.mockResolvedValue(apiResponse); + await getLearningAssistantSummary(courseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, }); - it('should only set and remove the loading state', async () => { - await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); - expect(dispatch).toHaveBeenNthCalledWith(1, { - type: 'learning-assistant/setApiIsLoading', - payload: true, - }); + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setIsEnabled', + 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/setMessageList' }), - ); + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), + ); - expect(dispatch).not.toHaveBeenCalledWith( - expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), - ); + expect(dispatch).toHaveBeenNthCalledWith(3, { + type: 'learning-assistant/setAuditTrial', + payload: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }, + }); - expect(dispatch).toHaveBeenNthCalledWith(2, { - type: 'learning-assistant/setApiIsLoading', - payload: false, - }); + expect(dispatch).toHaveBeenNthCalledWith(4, { + type: 'learning-assistant/setApiIsLoading', + payload: false, }); }); - describe('when throwing on fetching', () => { - beforeEach(() => { - fetchLearningAssistantMessageHistory.mockRejectedValue('Whoopsie!'); + it('with no audit_trial data returned, do not call audit trial dispatch', async () => { + const apiResponse = { + enabled: true, + message_history: [ + { + role: 'user', + content: 'Marco', + timestamp: '2024-11-04T19:05:07.403363Z', + }, + { + role: 'assistant', + content: 'Polo', + timestamp: '2024-11-04T19:05:21.357636Z', + }, + ], + audit_trial: {}, + }; + + fetchLearningAssistantSummary.mockResolvedValue(apiResponse); + + await getLearningAssistantSummary(courseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, + }); + + expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); + + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setIsEnabled', + payload: true, + }); + + expect(dispatch).toHaveBeenNthCalledWith(3, { + type: 'learning-assistant/setMessageList', + payload: { + messageList: apiResponse.message_history.map(({ timestamp, ...msg }) => ({ + ...msg, + timestamp: new Date(timestamp).toString(), // Parse ISO time to Date() + })), + }, }); - it('should only set and remove the loading state', async () => { - await getLearningAssistantMessageHistory(fakeCourseId)(dispatch); + expect(dispatch).toHaveBeenNthCalledWith(4, { + type: 'learning-assistant/setDisclosureAcknowledged', + payload: true, + }); - expect(dispatch).toHaveBeenNthCalledWith(1, { - type: 'learning-assistant/setApiIsLoading', - payload: true, - }); + expect(dispatch).toHaveBeenNthCalledWith(5, { + type: 'learning-assistant/setApiIsLoading', + payload: false, + }); + }); + + it('when throwing on fetching, should set the loading state and throw error', async () => { + fetchLearningAssistantSummary.mockRejectedValue('Whoopsie!'); + + await getLearningAssistantSummary(courseId)(dispatch); + + expect(dispatch).toHaveBeenNthCalledWith(1, { + type: 'learning-assistant/setApiIsLoading', + payload: true, + }); - expect(fetchLearningAssistantMessageHistory).toHaveBeenCalledWith(fakeCourseId); + expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); - expect(dispatch).not.toHaveBeenCalledWith( - expect.objectContaining({ type: 'learning-assistant/setMessageList' }), - ); + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setMessageList' }), + ); - expect(dispatch).not.toHaveBeenCalledWith( - expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), - ); + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'learning-assistant/setDisclosureAcknowledged' }), + ); - expect(dispatch).toHaveBeenNthCalledWith(2, { - type: 'learning-assistant/setApiIsLoading', - payload: false, - }); + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: 'learning-assistant/setApiError', + // payload: false, }); }); }); diff --git a/src/hooks/index.js b/src/hooks/index.js deleted file mode 100644 index bfa4b76e..00000000 --- a/src/hooks/index.js +++ /dev/null @@ -1,2 +0,0 @@ -/* 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 deleted file mode 100644 index c27d5f46..00000000 --- a/src/hooks/message-history.js +++ /dev/null @@ -1,15 +0,0 @@ -/* 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 deleted file mode 100644 index fe1352b1..00000000 --- a/src/hooks/message-history.test.js +++ /dev/null @@ -1,57 +0,0 @@ -import { renderHook } from '@testing-library/react-hooks'; // eslint-disable-line import/no-unresolved - -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/utils/utils.test.jsx b/src/utils/utils.test.jsx index 3edeef44..870c0a04 100644 --- a/src/utils/utils.test.jsx +++ b/src/utils/utils.test.jsx @@ -48,7 +48,9 @@ const createRandomResponseForTesting = () => { message.push(words[Math.floor(Math.random() * words.length)]); } - return { role: 'assistant', content: message.join(' ') }; + const timestamp = new Date(); + + return [{ role: 'assistant', content: message.join(' '), timestamp }]; }; export { renderWithProviders as render, createRandomResponseForTesting }; diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index cdb99fa2..ce36c6dc 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -1,17 +1,16 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; -import { updateSidebarIsOpen, getIsEnabled, getAuditTrial } from '../data/thunks'; +import { updateSidebarIsOpen, getLearningAssistantSummary } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; import { ExperimentsProvider } from '../experiments'; -import { useMessageHistory } from '../hooks'; +// import { getLearningAssistantData } from '../hooks'; const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const dispatch = useDispatch(); - useMessageHistory(courseId); + // getLearningAssistantData(courseId); const { isEnabled, @@ -24,11 +23,8 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { }; useEffect(() => { - dispatch(getIsEnabled(courseId)); - dispatch(getAuditTrial(courseId)); + dispatch(getLearningAssistantSummary(courseId)); }, [dispatch, courseId]); - console.log("auditTrial:", auditTrial); - console.log("auditTrial.expirationDate:", auditTrial.expirationDate); // NOTE: This value can be used later on if/when we pass the enrollment mode to this componentn const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars diff --git a/src/widgets/Xpert.test.jsx b/src/widgets/Xpert.test.jsx index ca30270c..cb3a99af 100644 --- a/src/widgets/Xpert.test.jsx +++ b/src/widgets/Xpert.test.jsx @@ -9,8 +9,6 @@ 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'; -// import { updateSidebarIsOpen, getIsEnabled, getAuditTrial } from '../data/thunks'; jest.mock('@edx/frontend-platform/analytics'); jest.mock('@edx/frontend-platform/auth', () => ({ @@ -22,14 +20,6 @@ jest.mock('../experiments', () => ({ usePromptExperimentDecision: jest.fn(), })); -jest.mock('../hooks'); - -// TODO: Fix whatever the heck is going on here. -// jest.mock('../data/thunks', () => ({ -// getIsEnabled: jest.fn(), -// getAuditTrial: jest.fn(), -// })); - const initialState = { learningAssistant: { currentMessage: '', @@ -40,8 +30,6 @@ const initialState = { // I will remove this and write tests in a future pull request. disclosureAcknowledged: true, sidebarIsOpen: false, - - }, }; const courseId = 'course-v1:edX+DemoX+Demo_Course'; @@ -57,12 +45,15 @@ const assertSidebarElementsNotInDOM = () => { beforeEach(() => { const responseMessage = createRandomResponseForTesting(); - jest.spyOn(api, 'fetchChatResponse').mockResolvedValue(responseMessage); - jest.spyOn(api, 'fetchLearningAssistantEnabled').mockResolvedValue({ enabled: true }); - jest.spyOn(api, 'fetchLearningAssistantAuditTrial').mockResolvedValue({ - start_date: '2024-12-02T14:59:16.148236Z', - expiration_date: '9999-12-16T14:59:16.148236Z', + jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ + enabled: true, + message_history: responseMessage, + audit_trial: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }, }); + usePromptExperimentDecision.mockReturnValue([]); window.localStorage.clear(); @@ -72,7 +63,7 @@ beforeEach(() => { }); test('doesn\'t load if not enabled', async () => { - jest.spyOn(api, 'fetchLearningAssistantEnabled').mockResolvedValue({ enabled: false }); + jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ enabled: false }); render(, { preloadedState: initialState }); @@ -91,11 +82,6 @@ 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 }); @@ -181,7 +167,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(); + const responseMessage = createRandomResponseForTesting()[0]; jest.spyOn(api, 'fetchChatResponse').mockResolvedValue(responseMessage); render(, { preloadedState: initialState }); @@ -330,23 +316,22 @@ test('popup modal should close and display CTA', async () => { expect(screen.queryByTestId('action-message')).toBeVisible(); }); test('survey monkey survey should appear after closing sidebar', async () => { + jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ + enabled: true, + message_history: [ + // A length >= 2 is required to load the survey + { role: 'user', content: 'hi', timestamp: new Date().toString() }, + { role: 'user', content: 'hi', timestamp: new Date().toString() }, + ], + audit_trial: { + start_date: '2024-12-02T14:59:16.148236Z', + expiration_date: '9999-12-16T14:59:16.148236Z', + }, + }); const survey = jest.spyOn(surveyMonkey, 'default').mockReturnValueOnce(1); const user = userEvent.setup(); - const surveyState = { - learningAssistant: { - currentMessage: '', - messageList: [ - { role: 'user', content: 'hi', timestamp: new Date() }, - { role: 'user', content: 'hi', timestamp: new Date() + 1 }, - ], - apiIsLoading: false, - apiError: false, - disclosureAcknowledged: true, - sidebarIsOpen: false, - }, - }; - render(, { preloadedState: surveyState }); + render(); // wait for button to appear await screen.findByTestId('toggle-button'); @@ -357,6 +342,6 @@ test('survey monkey survey should appear after closing sidebar', async () => { await user.click(screen.queryByTestId('close-button')); // assert mock called - expect(survey).toBeCalledTimes(1); + expect(survey).toHaveBeenCalledTimes(1); survey.mockRestore(); }); From 6a4a2aa60eaf28bf073a86f5b365f28d1b8588e5 Mon Sep 17 00:00:00 2001 From: ilee2u Date: Wed, 4 Dec 2024 10:18:19 -0500 Subject: [PATCH 7/9] chore: some nits --- src/data/api.js | 2 +- src/data/api.test.js | 2 +- src/data/thunks.js | 1 - src/widgets/Xpert.jsx | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/data/api.js b/src/data/api.js index 4450e269..09f04b9b 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -24,7 +24,7 @@ async function fetchChatResponse(courseId, messageList, unitId, customQueryParam } async function fetchLearningAssistantSummary(courseId) { - const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/summary`); + const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/chat-summary`); const { data } = await getAuthenticatedHttpClient().get(url.href); return data; diff --git a/src/data/api.test.js b/src/data/api.test.js index d558122e..da61e169 100644 --- a/src/data/api.test.js +++ b/src/data/api.test.js @@ -54,7 +54,7 @@ describe('API', () => { expect(response).toEqual(apiPayload); expect(mockGet).toHaveBeenCalledTimes(1); - expect(mockGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${courseId}/summary`); + expect(mockGet).toHaveBeenCalledWith(`${CHAT_RESPONSE_URL}/${courseId}/chat-summary`); }); }); }); diff --git a/src/data/thunks.js b/src/data/thunks.js index 501e588b..037be51e 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -119,7 +119,6 @@ export function getLearningAssistantSummary(courseId) { const messageList = rawMessageList .map(({ timestamp, ...msg }) => ({ ...msg, - // NOTE to self: can't store Date() objects in store: https://github.com/reduxjs/redux-toolkit/issues/456 timestamp: new Date(timestamp).toString(), // Parse ISO time to Date() })); diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index ce36c6dc..b15853b6 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -26,7 +26,7 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { dispatch(getLearningAssistantSummary(courseId)); }, [dispatch, courseId]); - // NOTE: This value can be used later on if/when we pass the enrollment mode to this componentn + // NOTE: This value can be used later on if/when we pass the enrollment mode to this component const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars const auditTrialExpired = (Date.now() - auditTrial.expirationDate) > 0; if (auditTrialExpired) { From 3625ed2e1caf023b56c3195c03af74dbaa346abf Mon Sep 17 00:00:00 2001 From: ilee2u Date: Wed, 4 Dec 2024 14:17:51 -0500 Subject: [PATCH 8/9] fix: convert stored datestring back to date + nits --- src/data/api.js | 4 ++-- src/data/api.test.js | 6 +++--- src/data/slice.js | 5 +---- src/data/thunks.js | 10 ++++------ src/data/thunks.test.js | 30 +++++++++++++++--------------- src/widgets/Xpert.jsx | 11 +++++------ src/widgets/Xpert.test.jsx | 6 +++--- 7 files changed, 33 insertions(+), 39 deletions(-) diff --git a/src/data/api.js b/src/data/api.js index 09f04b9b..0f90283f 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -23,7 +23,7 @@ async function fetchChatResponse(courseId, messageList, unitId, customQueryParam return data; } -async function fetchLearningAssistantSummary(courseId) { +async function fetchLearningAssistantChatSummary(courseId) { const url = new URL(`${getConfig().CHAT_RESPONSE_URL}/${courseId}/chat-summary`); const { data } = await getAuthenticatedHttpClient().get(url.href); @@ -32,5 +32,5 @@ async function fetchLearningAssistantSummary(courseId) { export { fetchChatResponse, - fetchLearningAssistantSummary, + fetchLearningAssistantChatSummary, }; diff --git a/src/data/api.test.js b/src/data/api.test.js index da61e169..d685150d 100644 --- a/src/data/api.test.js +++ b/src/data/api.test.js @@ -1,7 +1,7 @@ /* eslint-disable no-import-assign */ import * as auth from '@edx/frontend-platform/auth'; -import { fetchLearningAssistantSummary } from './api'; +import { fetchLearningAssistantChatSummary } from './api'; jest.mock('@edx/frontend-platform/auth'); @@ -16,7 +16,7 @@ describe('API', () => { jest.restoreAllMocks(); }); - describe('fetchLearningAssistantSummary()', () => { + describe('fetchLearningAssistantChatSummary()', () => { const courseId = 'course-v1:edx+test+23'; const apiPayload = { enabled: true, @@ -50,7 +50,7 @@ describe('API', () => { }); it('should call the endpoint and process the results', async () => { - const response = await fetchLearningAssistantSummary(courseId); + const response = await fetchLearningAssistantChatSummary(courseId); expect(response).toEqual(apiPayload); expect(mockGet).toHaveBeenCalledTimes(1); diff --git a/src/data/slice.js b/src/data/slice.js index 39e06ec7..91982422 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -11,10 +11,7 @@ export const initialState = { disclosureAcknowledged: false, sidebarIsOpen: false, isEnabled: false, - auditTrial: { - startDate: null, - expirationDate: null, - }, + auditTrial: {}, }; export const learningAssistantSlice = createSlice({ diff --git a/src/data/thunks.js b/src/data/thunks.js index 037be51e..fb84955c 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -4,7 +4,7 @@ import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import trackChatBotMessageOptimizely from '../utils/optimizelyExperiment'; import { fetchChatResponse, - fetchLearningAssistantSummary, + fetchLearningAssistantChatSummary, } from './api'; import { setCurrentMessage, @@ -101,12 +101,12 @@ export function updateSidebarIsOpen(isOpen) { }; } -export function getLearningAssistantSummary(courseId) { +export function getLearningAssistantChatSummary(courseId) { return async (dispatch) => { dispatch(setApiIsLoading(true)); try { - const data = await fetchLearningAssistantSummary(courseId); + const data = await fetchLearningAssistantChatSummary(courseId); // Enabled dispatch(setIsEnabled(data.enabled)); @@ -132,12 +132,10 @@ export function getLearningAssistantSummary(courseId) { const auditTrial = data.audit_trial; // If returned audit trial data is not empty - if (Object.keys(auditTrial).length !== 0) { // eslint-disable-line no-undef + if (Object.keys(auditTrial).length !== 0) { dispatch(setAuditTrial(auditTrial)); } } catch (error) { - // NOTE: When used to not show if fetching the messages failed - // But we do know since this endpoint is combined. dispatch(setApiError()); } dispatch(setApiIsLoading(false)); diff --git a/src/data/thunks.test.js b/src/data/thunks.test.js index 75d1d576..439b9e49 100644 --- a/src/data/thunks.test.js +++ b/src/data/thunks.test.js @@ -1,6 +1,6 @@ -import { fetchLearningAssistantSummary } from './api'; +import { fetchLearningAssistantChatSummary } from './api'; -import { getLearningAssistantSummary } from './thunks'; +import { getLearningAssistantChatSummary } from './thunks'; jest.mock('./api'); @@ -9,7 +9,7 @@ describe('Thunks unit tests', () => { afterEach(() => jest.resetAllMocks()); - describe('getLearningAssistantSummary()', () => { + describe('getLearningAssistantChatSummary()', () => { const courseId = 'course-v1:edx+test+23'; it('with message_history and audit_trial data returned, call all expected dispatches', async () => { @@ -33,16 +33,16 @@ describe('Thunks unit tests', () => { }, }; - fetchLearningAssistantSummary.mockResolvedValue(apiResponse); + fetchLearningAssistantChatSummary.mockResolvedValue(apiResponse); - await getLearningAssistantSummary(courseId)(dispatch); + await getLearningAssistantChatSummary(courseId)(dispatch); expect(dispatch).toHaveBeenNthCalledWith(1, { type: 'learning-assistant/setApiIsLoading', payload: true, }); - expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); + expect(fetchLearningAssistantChatSummary).toHaveBeenCalledWith(courseId); expect(dispatch).toHaveBeenNthCalledWith(2, { type: 'learning-assistant/setIsEnabled', @@ -88,16 +88,16 @@ describe('Thunks unit tests', () => { }, }; - fetchLearningAssistantSummary.mockResolvedValue(apiResponse); + fetchLearningAssistantChatSummary.mockResolvedValue(apiResponse); - await getLearningAssistantSummary(courseId)(dispatch); + await getLearningAssistantChatSummary(courseId)(dispatch); expect(dispatch).toHaveBeenNthCalledWith(1, { type: 'learning-assistant/setApiIsLoading', payload: true, }); - expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); + expect(fetchLearningAssistantChatSummary).toHaveBeenCalledWith(courseId); expect(dispatch).toHaveBeenNthCalledWith(2, { type: 'learning-assistant/setIsEnabled', @@ -144,16 +144,16 @@ describe('Thunks unit tests', () => { audit_trial: {}, }; - fetchLearningAssistantSummary.mockResolvedValue(apiResponse); + fetchLearningAssistantChatSummary.mockResolvedValue(apiResponse); - await getLearningAssistantSummary(courseId)(dispatch); + await getLearningAssistantChatSummary(courseId)(dispatch); expect(dispatch).toHaveBeenNthCalledWith(1, { type: 'learning-assistant/setApiIsLoading', payload: true, }); - expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); + expect(fetchLearningAssistantChatSummary).toHaveBeenCalledWith(courseId); expect(dispatch).toHaveBeenNthCalledWith(2, { type: 'learning-assistant/setIsEnabled', @@ -182,16 +182,16 @@ describe('Thunks unit tests', () => { }); it('when throwing on fetching, should set the loading state and throw error', async () => { - fetchLearningAssistantSummary.mockRejectedValue('Whoopsie!'); + fetchLearningAssistantChatSummary.mockRejectedValue('Whoopsie!'); - await getLearningAssistantSummary(courseId)(dispatch); + await getLearningAssistantChatSummary(courseId)(dispatch); expect(dispatch).toHaveBeenNthCalledWith(1, { type: 'learning-assistant/setApiIsLoading', payload: true, }); - expect(fetchLearningAssistantSummary).toHaveBeenCalledWith(courseId); + expect(fetchLearningAssistantChatSummary).toHaveBeenCalledWith(courseId); expect(dispatch).not.toHaveBeenCalledWith( expect.objectContaining({ type: 'learning-assistant/setMessageList' }), diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index b15853b6..19f41c5e 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -2,15 +2,13 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { updateSidebarIsOpen, getLearningAssistantSummary } from '../data/thunks'; +import { updateSidebarIsOpen, getLearningAssistantChatSummary } from '../data/thunks'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; import { ExperimentsProvider } from '../experiments'; -// import { getLearningAssistantData } from '../hooks'; const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const dispatch = useDispatch(); - // getLearningAssistantData(courseId); const { isEnabled, @@ -23,13 +21,14 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { }; useEffect(() => { - dispatch(getLearningAssistantSummary(courseId)); + dispatch(getLearningAssistantChatSummary(courseId)); }, [dispatch, courseId]); // NOTE: This value can be used later on if/when we pass the enrollment mode to this component const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars - const auditTrialExpired = (Date.now() - auditTrial.expirationDate) > 0; - if (auditTrialExpired) { + const auditTrialExpirationDate = new Date(auditTrial.expirationDate); + + if ((Date.now() - auditTrialExpirationDate) >= 0) { return true; } return false; diff --git a/src/widgets/Xpert.test.jsx b/src/widgets/Xpert.test.jsx index cb3a99af..24819f66 100644 --- a/src/widgets/Xpert.test.jsx +++ b/src/widgets/Xpert.test.jsx @@ -45,7 +45,7 @@ const assertSidebarElementsNotInDOM = () => { beforeEach(() => { const responseMessage = createRandomResponseForTesting(); - jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ + jest.spyOn(api, 'fetchLearningAssistantChatSummary').mockResolvedValue({ enabled: true, message_history: responseMessage, audit_trial: { @@ -63,7 +63,7 @@ beforeEach(() => { }); test('doesn\'t load if not enabled', async () => { - jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ enabled: false }); + jest.spyOn(api, 'fetchLearningAssistantChatSummary').mockResolvedValue({ enabled: false }); render(, { preloadedState: initialState }); @@ -316,7 +316,7 @@ test('popup modal should close and display CTA', async () => { expect(screen.queryByTestId('action-message')).toBeVisible(); }); test('survey monkey survey should appear after closing sidebar', async () => { - jest.spyOn(api, 'fetchLearningAssistantSummary').mockResolvedValue({ + jest.spyOn(api, 'fetchLearningAssistantChatSummary').mockResolvedValue({ enabled: true, message_history: [ // A length >= 2 is required to load the survey From 9199c92a64924817681020051340ffdfeb7a969a Mon Sep 17 00:00:00 2001 From: ilee2u Date: Wed, 4 Dec 2024 16:41:14 -0500 Subject: [PATCH 9/9] chore: use > not >= --- src/widgets/Xpert.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index 19f41c5e..75591e7f 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -28,7 +28,7 @@ const Xpert = ({ courseId, contentToolsEnabled, unitId }) => { const isAuditTrialNotExpired = () => { // eslint-disable-line no-unused-vars const auditTrialExpirationDate = new Date(auditTrial.expirationDate); - if ((Date.now() - auditTrialExpirationDate) >= 0) { + if ((Date.now() - auditTrialExpirationDate) > 0) { return true; } return false;